Skip to content

Commit

Permalink
Issue #4967 - Possible buffer corruption in HTTP/2 session failures
Browse files Browse the repository at this point in the history 
Improved logging.

Signed-off-by: Simone Bordet <[email protected]>
  • Loading branch information
@sbordet
sbordet committed Jun 25, 2020
1 parent 1b59672 commit 04ebc00
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 17 deletions.
38 changes: 23 additions & 15 deletions jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public void onData(DataFrame frame)
public void onData(final DataFrame frame, Callback callback)
{
if (LOG.isDebugEnabled())
LOG.debug("Received {}", frame);
LOG.debug("Received {} on {}", frame, this);

int streamId = frame.getStreamId();
IStream stream = getStream(streamId);
Expand All @@ -252,7 +252,7 @@ public void onData(final DataFrame frame, Callback callback)
else
{
if (LOG.isDebugEnabled())
LOG.debug("Stream #{} not found", streamId);
LOG.debug("Stream #{} not found on {}", streamId, this);
// We must enlarge the session flow control window,
// otherwise other requests will be stalled.
flowControl.onDataConsumed(this, null, flowControlLength);
Expand Down Expand Up @@ -290,14 +290,14 @@ protected boolean isRemoteStreamClosed(int streamId)
public void onPriority(PriorityFrame frame)
{
if (LOG.isDebugEnabled())
LOG.debug("Received {}", frame);
LOG.debug("Received {} on {}", frame, this);
}

@Override
public void onReset(ResetFrame frame)
{
if (LOG.isDebugEnabled())
LOG.debug("Received {}", frame);
LOG.debug("Received {} on {}", frame, this);

int streamId = frame.getStreamId();
IStream stream = getStream(streamId);
Expand Down Expand Up @@ -329,7 +329,7 @@ public void onSettings(SettingsFrame frame)
public void onSettings(SettingsFrame frame, boolean reply)
{
if (LOG.isDebugEnabled())
LOG.debug("Received {}", frame);
LOG.debug("Received {} on {}", frame, this);

if (frame.isReply())
return;
Expand Down Expand Up @@ -405,7 +405,7 @@ public void onSettings(SettingsFrame frame, boolean reply)
public void onPing(PingFrame frame)
{
if (LOG.isDebugEnabled())
LOG.debug("Received {}", frame);
LOG.debug("Received {} on {}", frame, this);

if (frame.isReply())
{
Expand Down Expand Up @@ -439,7 +439,7 @@ public void onPing(PingFrame frame)
public void onGoAway(final GoAwayFrame frame)
{
if (LOG.isDebugEnabled())
LOG.debug("Received {}", frame);
LOG.debug("Received {} on {}", frame, this);

while (true)
{
Expand Down Expand Up @@ -472,7 +472,7 @@ public void onGoAway(final GoAwayFrame frame)
public void onWindowUpdate(WindowUpdateFrame frame)
{
if (LOG.isDebugEnabled())
LOG.debug("Received {}", frame);
LOG.debug("Received {} on {}", frame, this);

int streamId = frame.getStreamId();
int windowDelta = frame.getWindowDelta();
Expand Down Expand Up @@ -512,7 +512,9 @@ public void onWindowUpdate(WindowUpdateFrame frame)
public void onStreamFailure(int streamId, int error, String reason)
{
Callback callback = new ResetCallback(streamId, error, Callback.NOOP);
Throwable failure = toFailure("Stream failure", error, reason);
Throwable failure = toFailure(error, reason);
if (LOG.isDebugEnabled())
LOG.debug("Stream #{} failure {}", streamId, this, failure);
onStreamFailure(streamId, error, reason, failure, callback);
}

Expand All @@ -533,12 +535,16 @@ public void onConnectionFailure(int error, String reason)

protected void onConnectionFailure(int error, String reason, Callback callback)
{
Throwable failure = toFailure("Session failure", error, reason);
Throwable failure = toFailure(error, reason);
if (LOG.isDebugEnabled())
LOG.debug("Session failure {}", this, failure);
onFailure(error, reason, failure, new CloseCallback(error, reason, callback));
}

protected void abort(Throwable failure)
{
if (LOG.isDebugEnabled())
LOG.debug("Session abort {}", this, failure);
onFailure(ErrorCode.NO_ERROR.code, null, failure, new TerminateCallback(failure));
}

Expand All @@ -558,7 +564,9 @@ private void onClose(GoAwayFrame frame, Callback callback)
{
int error = frame.getError();
String reason = frame.tryConvertPayload();
Throwable failure = toFailure("Session close", error, reason);
Throwable failure = toFailure(error, reason);
if (LOG.isDebugEnabled())
LOG.debug("Session close {}", this, failure);
Collection<Stream> streams = getStreams();
int count = streams.size();
Callback countCallback = new CountingCallback(callback, count + 1);
Expand All @@ -569,9 +577,9 @@ private void onClose(GoAwayFrame frame, Callback callback)
notifyClose(this, frame, countCallback);
}

private Throwable toFailure(String message, int error, String reason)
private Throwable toFailure(int error, String reason)
{
return new IOException(String.format("%s %s/%s", message, ErrorCode.toString(error, null), reason));
return new IOException(String.format("%s/%s", ErrorCode.toString(error, null), reason));
}

@Override
Expand Down Expand Up @@ -721,7 +729,7 @@ public void data(IStream stream, Callback callback, DataFrame frame)
private void frame(HTTP2Flusher.Entry entry, boolean flush)
{
if (LOG.isDebugEnabled())
LOG.debug("{} {}", flush ? "Sending" : "Queueing", entry.frame);
LOG.debug("{} {} on {}", flush ? "Sending" : "Queueing", entry.frame, this);
// Ping frames are prepended to process them as soon as possible.
boolean queued = entry.frame.getType() == FrameType.PING ? flusher.prepend(entry) : flusher.append(entry);
if (queued && flush)
Expand Down Expand Up @@ -822,7 +830,7 @@ public void removeStream(IStream stream)
onStreamClosed(stream);
flowControl.onStreamDestroyed(stream);
if (LOG.isDebugEnabled())
LOG.debug("Removed {} {}", stream.isLocal() ? "local" : "remote", stream);
LOG.debug("Removed {} {} from {}", stream.isLocal() ? "local" : "remote", stream, this);
}
}

Expand Down
4 changes: 2 additions & 2 deletions jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ protected boolean parseHeader(ByteBuffer buffer)
return false;

if (LOG.isDebugEnabled())
LOG.debug("Parsed {} frame header from {}", headerParser, buffer);
LOG.debug("Parsed {} frame header from {}@{}", headerParser, buffer, Integer.toHexString(buffer.hashCode()));

if (headerParser.getLength() > getMaxFrameLength())
return connectionFailure(buffer, ErrorCode.FRAME_SIZE_ERROR, "invalid_frame_length");
Expand Down Expand Up @@ -199,7 +199,7 @@ protected boolean parseBody(ByteBuffer buffer)
return false;
}
if (LOG.isDebugEnabled())
LOG.debug("Parsed {} frame body from {}", FrameType.from(type), buffer);
LOG.debug("Parsed {} frame body from {}@{}", FrameType.from(type), buffer, Integer.toHexString(buffer.hashCode()));
reset();
return true;
}
Expand Down

0 comments on commit 04ebc00

Please sign in to comment.