Skip to content

Commit

Permalink
Cleanup of relative redirect handling #11014
Browse files Browse the repository at this point in the history
+ Handle request relative redirects
+ Moved to Response
+ Changed default to allow relative
  • Loading branch information
gregw committed Dec 4, 2023
1 parent 5120c48 commit 8a5ca86
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,16 @@ private void onBadMessage(HttpException x)
// TODO
}

@Override
public void willRead()
{
if (_expects100Continue)
{
_expects100Continue = false;
send(_requestMetaData, HttpGenerator.CONTINUE_100_INFO, false, null, Callback.NOOP);
}
}

@Override
public Content.Chunk read()
{
Expand Down Expand Up @@ -218,11 +228,7 @@ else if (!_demand)
}
else if (demand)
{
if (_expects100Continue)
{
_expects100Continue = false;
send(_requestMetaData, HttpGenerator.CONTINUE_100_INFO, false, null, Callback.NOOP);
}
willRead();
_stream.demand();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,16 @@ private void onBadMessage(HttpException x)
// TODO
}

@Override
public void willRead()
{
if (expects100Continue)
{
expects100Continue = false;
send(requestMetaData, HttpGenerator.CONTINUE_100_INFO, false, null, Callback.NOOP);
}
}

@Override
public Content.Chunk read()
{
Expand Down Expand Up @@ -186,11 +196,7 @@ public void demand()
}
else
{
if (expects100Continue)
{
expects100Continue = false;
send(requestMetaData, HttpGenerator.CONTINUE_100_INFO, false, null, Callback.NOOP);
}
willRead();
stream.demand();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,22 @@ public interface HttpStream extends Callback
*/
String getId();

/**
* Signal the intention to read the request content for the purposes of 100-Continues processing as follows: <ul>
* <li>If a {@code 100-Continue} response is expected, then calling this method will send that response and the expectation
* is satisfied. Otherwise this method is a {@code noop}.</li>
* <li>There is an implied call to {@code willRead()} when {@link #demand()} is called.</li>
* <li>If a {@link #read()} is attempted before calling this method and some content is available, then
* any 100-Continues expectation is cancelled.</li>
* <li>If a {@link #send(MetaData.Request, MetaData.Response, boolean, ByteBuffer, Callback)} is done prior to
* a call to {@code willRead()}, then any expectation is cancelled and the connection put into a protocol dependent
* state to not expect the request content to be sent and to ignore any further content sent on the channel</li>
* </ul>
*/
default void willRead()
{
}

/**
* <p>Reads a chunk of content, with the same semantic as {@link Content.Source#read()}.</p>
* <p>This method is called from the implementation of {@link Request#read()}.</p>
Expand Down Expand Up @@ -155,6 +171,12 @@ public final String getId()
return getWrapped().getId();
}

@Override
public void willRead()
{
getWrapped().willRead();
}

@Override
public Content.Chunk read()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,14 @@ static long getTimeStamp(Request request)
// TODO: see above.
boolean isSecure();

/**
* Signal the intention to read the request content for the purposes of 100-Continues processing.
* @see HttpStream#willRead()
*/
default void willRead()
{
}

/**
* {@inheritDoc}
* <p>In addition, the returned {@link Content.Chunk} may be a
Expand Down Expand Up @@ -834,6 +842,12 @@ public long getLength()
return getWrapped().getLength();
}

@Override
public void willRead()
{
getWrapped().willRead();
}

@Override
public Content.Chunk read()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,19 @@ public long getLength()
return _metaData.getContentLength();
}

@Override
public void willRead()
{
HttpStream stream;
try (AutoLock ignored = _lock.lock())
{
HttpChannelState httpChannel = lockedGetHttpChannelState();
stream = httpChannel._stream;
}

stream.willRead();
}

@Override
public Content.Chunk read()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1331,6 +1331,16 @@ public String getId()
return Long.toString(_id);
}

@Override
public void willRead()
{
if (_expects100Continue)
{
_expects100Continue = false;
send(_request, HttpGenerator.CONTINUE_100_INFO, false, null, Callback.NOOP);
}
}

@Override
public Content.Chunk read()
{
Expand Down Expand Up @@ -1373,11 +1383,7 @@ public void demand()
return;
}

if (_expects100Continue)
{
_expects100Continue = false;
send(_request, HttpGenerator.CONTINUE_100_INFO, false, null, Callback.NOOP);
}
willRead();

tryFillInterested(_demandContentCallback);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,7 @@ public ServletInputStream getInputStream() throws IOException
if (_inputState != ServletContextRequest.INPUT_NONE && _inputState != ServletContextRequest.INPUT_STREAM)
throw new IllegalStateException("READER");
_inputState = ServletContextRequest.INPUT_STREAM;
_servletChannel.getRequest().willRead();
return getServletRequestInfo().getHttpInput();
}

Expand Down Expand Up @@ -1070,7 +1071,11 @@ public String toString()
};
}

if (_reader == null || !charset.equals(_readerCharset))
if (_reader != null && charset.equals(_readerCharset))
{
_servletChannel.getRequest().willRead();
}
else
{
ServletInputStream in = getInputStream();
_readerCharset = charset;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,10 @@ public void testExpect100ContinueWithChunkedContentRespond100Continue(Transport
@Override
protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException
{
// read/write first byte to send 100-Continue
// Send 100-Continue and copy the content back
ServletInputStream input = request.getInputStream();
response.getOutputStream().write(input.read());

// Commit response so that length is not known and chunking or equivalent must be used.
// Make sure we chunk the response too
response.flushBuffer();

// Copy the content.
IO.copy(input, response.getOutputStream());
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,7 @@ public ServletInputStream getInputStream() throws IOException
if (_inputState != INPUT_NONE && _inputState != INPUT_STREAM)
throw new IllegalStateException("READER");
_inputState = INPUT_STREAM;
_channel.getCoreRequest().willRead();
return _input;
}

Expand Down Expand Up @@ -1046,7 +1047,11 @@ public BufferedReader getReader() throws IOException
if (encoding == null)
encoding = MimeTypes.ISO_8859_1;

if (_reader == null || !encoding.equalsIgnoreCase(_readerEncoding))
if (_reader != null && encoding.equalsIgnoreCase(_readerEncoding))
{
_channel.getCoreRequest().willRead();
}
else
{
ServletInputStream in = getInputStream();
_readerEncoding = encoding;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,8 @@ public void testExpect100ContinueWithChunkedContentRespond100Continue(Transport
@Override
protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException
{
// read/write first byte to send 100-Continue
// Send 100-Continue and copy the content back
ServletInputStream input = request.getInputStream();
response.getOutputStream().write(input.read());

// Make sure we chunk the response too
response.flushBuffer();
IO.copy(input, response.getOutputStream());
Expand Down

0 comments on commit 8a5ca86

Please sign in to comment.