Skip to content

Commit

Permalink
Jetty 9.4.x 3038 ssl connection leak (#3121)
Browse files Browse the repository at this point in the history
Issue #3038 - SSL connection leak.

Fixed SSL spin caused when fill had NEED_WRAP, but a flush/wrap
produced 0 bytes and stayed in NEED_WRAP

Removed check of isInputShutdown prior to filling that allowed EOF to
overtake data already read.

Fix for leak by shutting down output in HttpConnection if
filled -1 and the HttpChannelState was no longer processing
current request.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
  • Loading branch information
gregw committed Nov 20, 2018
1 parent 10622f3 commit 8c4ee84
Show file tree
Hide file tree
Showing 9 changed files with 507 additions and 168 deletions.
Expand Up @@ -20,6 +20,7 @@

import java.io.EOFException;
import java.nio.ByteBuffer;
import java.util.concurrent.atomic.AtomicReference;

import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpExchange;
Expand All @@ -39,6 +40,7 @@

public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.ResponseHandler
{
private final AtomicReference<ContentState> handlingContent = new AtomicReference<>(ContentState.IDLE);
private final HttpParser parser;
private ByteBuffer buffer;
private boolean shutdown;
Expand Down Expand Up @@ -263,8 +265,18 @@ public boolean content(ByteBuffer buffer)
if (exchange == null)
return false;

handlingContent.set(ContentState.CONTENT);
CompletableCallback callback = new CompletableCallback()
{
@Override
public void succeeded()
{
boolean messageComplete = !handlingContent.compareAndSet(ContentState.CONTENT, ContentState.IDLE);
super.succeeded();
if (messageComplete)
messageComplete();
}

@Override
public void resume()
{
Expand Down Expand Up @@ -304,6 +316,9 @@ public void parsedTrailer(HttpField trailer)
@Override
public boolean messageComplete()
{
if (handlingContent.compareAndSet(ContentState.CONTENT, ContentState.COMPLETE))
return false;

HttpExchange exchange = getHttpExchange();
if (exchange == null)
return false;
Expand Down Expand Up @@ -375,4 +390,6 @@ public String toString()
{
return String.format("%s[%s]", super.toString(), parser);
}

private enum ContentState { IDLE, CONTENT, COMPLETE }
}
21 changes: 11 additions & 10 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
Expand Up @@ -767,7 +767,7 @@ private boolean parseLine(ByteBuffer buffer)

case LF:
setState(State.HEADER);
handle=_responseHandler.startResponse(_version, _responseStatus, null)||handle;
handle |= _responseHandler.startResponse(_version, _responseStatus, null);
break;

default:
Expand All @@ -789,7 +789,7 @@ private boolean parseLine(ByteBuffer buffer)
handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
setState(State.END);
BufferUtil.clear(buffer);
handle= handleHeaderContentMessage() || handle;
handle |= handleHeaderContentMessage();
break;

case ALPHA:
Expand Down Expand Up @@ -865,7 +865,7 @@ else if (n==HttpTokens.LINE_FEED)
if (_responseHandler!=null)
{
setState(State.HEADER);
handle=_responseHandler.startResponse(_version, _responseStatus, null)||handle;
handle |= _responseHandler.startResponse(_version, _responseStatus, null);
}
else
{
Expand All @@ -876,7 +876,7 @@ else if (n==HttpTokens.LINE_FEED)
handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
setState(State.END);
BufferUtil.clear(buffer);
handle= handleHeaderContentMessage() || handle;
handle |= handleHeaderContentMessage();
}
break;

Expand Down Expand Up @@ -905,7 +905,7 @@ else if (n==HttpTokens.LINE_FEED)

setState(State.HEADER);

handle=_requestHandler.startRequest(_methodString,_uri.toString(), _version)||handle;
handle |= _requestHandler.startRequest(_methodString,_uri.toString(), _version);
continue;

case ALPHA:
Expand All @@ -927,7 +927,7 @@ else if (n==HttpTokens.LINE_FEED)
case LF:
String reason=takeString();
setState(State.HEADER);
handle=_responseHandler.startResponse(_version, _responseStatus, reason)||handle;
handle |= _responseHandler.startResponse(_version, _responseStatus, reason);
continue;

case ALPHA:
Expand Down Expand Up @@ -1660,14 +1660,16 @@ protected boolean parseContent(ByteBuffer buffer)
_contentPosition += _contentChunk.remaining();
buffer.position(buffer.position()+_contentChunk.remaining());

if (_handler.content(_contentChunk))
return true;
boolean handle = _handler.content(_contentChunk);

if(_contentPosition == _contentLength)
{
setState(State.END);
return handleContentMessage();
boolean handleContent = handleContentMessage();
return handle || handleContent;
}
else if (handle)
return true;
}
break;
}
Expand Down Expand Up @@ -1808,7 +1810,6 @@ protected boolean parseContent(ByteBuffer buffer)

/* ------------------------------------------------------------------------------- */
public boolean isAtEOF()

{
return _eof;
}
Expand Down
26 changes: 20 additions & 6 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java
Expand Up @@ -27,7 +27,6 @@
import java.nio.charset.StandardCharsets;

import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
Expand Down Expand Up @@ -110,13 +109,28 @@ public static Response parseResponse(ByteBuffer response)

public static Response parseResponse(InputStream responseStream) throws IOException
{
ByteArrayOutputStream contentStream = new ByteArrayOutputStream();
IO.copy(responseStream, contentStream);

Response r=new Response();
HttpParser parser =new HttpParser(r);
parser.parseNext(ByteBuffer.wrap(contentStream.toByteArray()));
return r;

// Read and parse a character at a time so we never can read more than we should.
byte[] array = new byte[1];
ByteBuffer buffer = ByteBuffer.wrap(array);
buffer.limit(1);

while(true)
{
buffer.position(1);
int l = responseStream.read(array);
if (l<0)
parser.atEOF();
else
buffer.position(0);

if (parser.parseNext(buffer))
return r;
else if (l<0)
return null;
}
}

public abstract static class Input
Expand Down

0 comments on commit 8c4ee84

Please sign in to comment.