From 8511c9294eb713a78c045c1f1f06e9763132236f Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Tue, 6 Apr 2021 15:40:21 -0700 Subject: [PATCH 01/13] Fix StreamBufferingEncoder GOAWAY bug --- .../codec/http2/StreamBufferingEncoder.java | 6 ++- .../http2/StreamBufferingEncoderTest.java | 37 ++++++++++++++++++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java index ac754edd440..da016a041a5 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java @@ -149,10 +149,14 @@ public ChannelFuture writeHeaders(ChannelHandlerContext ctx, int streamId, Http2 if (closed) { return promise.setFailure(new Http2ChannelClosedException()); } - if (isExistingStream(streamId) || connection().goAwayReceived()) { + if (isExistingStream(streamId)) { return super.writeHeaders(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream, promise); } + if (connection().goAwayReceived()) { + promise.setFailure(new Http2Exception(Http2Error.NO_ERROR, "GOAWAY received")); + return promise; + } if (canCreateStream()) { return super.writeHeaders(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream, promise); diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java index 2ad6068ca61..e5c7a62e67d 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java @@ -111,6 +111,11 @@ public void setup() throws Exception { when(writer.writeGoAway(any(ChannelHandlerContext.class), anyInt(), anyLong(), any(ByteBuf.class), any(ChannelPromise.class))) .thenAnswer(successAnswer()); + when(writer.writeHeaders(any(ChannelHandlerContext.class), anyInt(), any(Http2Headers.class), + anyInt(), anyBoolean(), any(ChannelPromise.class))).thenAnswer(noopAnswer()); + when(writer.writeHeaders(any(ChannelHandlerContext.class), anyInt(), any(Http2Headers.class), + anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean(), any(ChannelPromise.class))) + .thenAnswer(noopAnswer()); connection = new DefaultHttp2Connection(false); connection.remote().flowController(new DefaultHttp2RemoteFlowController(connection)); @@ -167,7 +172,7 @@ public void multipleWritesToActiveStream() { encoder.writeData(ctx, 3, data(), 0, false, newPromise()); encoderWriteHeaders(3, newPromise()); - writeVerifyWriteHeaders(times(2), 3); + writeVerifyWriteHeaders(times(1), 3); // Contiguous data writes are coalesced ArgumentCaptor bufCaptor = ArgumentCaptor.forClass(ByteBuf.class); verify(writer, times(1)) @@ -253,10 +258,24 @@ public void receivingGoAwayFailsBufferedStreams() throws Http2Exception { int failCount = 0; for (ChannelFuture f : futures) { if (f.cause() != null) { + assertTrue(f.cause() instanceof Http2Exception); + Http2Exception e = (Http2Exception) f.cause(); + assertEquals(Http2Error.STREAM_CLOSED, e.error()); failCount++; } } - assertEquals(9, failCount); + } + + @Test + public void receivingGoAwayFailsNewStreams() throws Http2Exception { + encoder.writeSettingsAck(ctx, newPromise()); + connection.goAwayReceived(11, 8, EMPTY_BUFFER); + ChannelFuture f = encoderWriteHeaders(3, newPromise()); + + assertTrue(f.cause() instanceof Http2Exception); + Http2Exception e = (Http2Exception) f.cause(); + assertEquals(Http2Error.NO_ERROR, e.error()); + assertEquals("GOAWAY received", e.getMessage()); assertEquals(0, encoder.numBufferedStreams()); } @@ -533,6 +552,20 @@ public ChannelFuture answer(InvocationOnMock invocation) throws Throwable { }; } + private Answer noopAnswer() { + return new Answer() { + @Override + public ChannelFuture answer(InvocationOnMock invocation) throws Throwable { + for (Object a : invocation.getArguments()) { + if (a instanceof ChannelPromise) { + return (ChannelFuture) a; + } + } + return newPromise(); + } + }; + } + private ChannelPromise newPromise() { return new DefaultChannelPromise(channel, ImmediateEventExecutor.INSTANCE); } From f3e66cd61bbc5c558eb91c5ddebfcd52a941da76 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Fri, 9 Apr 2021 17:05:06 -0700 Subject: [PATCH 02/13] reorder goAway check to keep legal old behavior --- codec-http2/pom.xml | 12 ++++++++++++ .../handler/codec/http2/StreamBufferingEncoder.java | 6 +----- .../codec/http2/StreamBufferingEncoderTest.java | 11 ++++++----- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/codec-http2/pom.xml b/codec-http2/pom.xml index 313a3536696..9eb7190c0af 100644 --- a/codec-http2/pom.xml +++ b/codec-http2/pom.xml @@ -17,6 +17,18 @@ 4.0.0 + + + + org.apache.maven.plugins + maven-compiler-plugin + + 7 + 7 + + + + io.netty netty-parent diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java index da016a041a5..a81da6c0bf1 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java @@ -149,7 +149,7 @@ public ChannelFuture writeHeaders(ChannelHandlerContext ctx, int streamId, Http2 if (closed) { return promise.setFailure(new Http2ChannelClosedException()); } - if (isExistingStream(streamId)) { + if (isExistingStream(streamId) || canCreateStream()) { return super.writeHeaders(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream, promise); } @@ -157,10 +157,6 @@ public ChannelFuture writeHeaders(ChannelHandlerContext ctx, int streamId, Http2 promise.setFailure(new Http2Exception(Http2Error.NO_ERROR, "GOAWAY received")); return promise; } - if (canCreateStream()) { - return super.writeHeaders(ctx, streamId, headers, streamDependency, weight, - exclusive, padding, endOfStream, promise); - } PendingStream pendingStream = pendingStreams.get(streamId); if (pendingStream == null) { pendingStream = new PendingStream(ctx, streamId); diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java index e5c7a62e67d..71a83cfba95 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java @@ -250,11 +250,9 @@ public void receivingGoAwayFailsBufferedStreams() throws Http2Exception { futures.add(encoderWriteHeaders(streamId, newPromise())); streamId += 2; } + assertEquals(5, connection.numActiveStreams()); assertEquals(4, encoder.numBufferedStreams()); - connection.goAwayReceived(11, 8, EMPTY_BUFFER); - - assertEquals(5, connection.numActiveStreams()); int failCount = 0; for (ChannelFuture f : futures) { if (f.cause() != null) { @@ -264,13 +262,16 @@ public void receivingGoAwayFailsBufferedStreams() throws Http2Exception { failCount++; } } + assertEquals(4, failCount); } @Test - public void receivingGoAwayFailsNewStreams() throws Http2Exception { + public void receivingGoAwayFailsNewStreamIfMaxConcurrentStreamsReached() throws Http2Exception { encoder.writeSettingsAck(ctx, newPromise()); + setMaxConcurrentStreams(1); + encoderWriteHeaders(3, newPromise()); connection.goAwayReceived(11, 8, EMPTY_BUFFER); - ChannelFuture f = encoderWriteHeaders(3, newPromise()); + ChannelFuture f = encoderWriteHeaders(5, newPromise()); assertTrue(f.cause() instanceof Http2Exception); Http2Exception e = (Http2Exception) f.cause(); From 7751c79d778e1941fc5e6988968a221f68fb0f61 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Fri, 9 Apr 2021 17:10:27 -0700 Subject: [PATCH 03/13] revert pom.xml changed by IDE --- codec-http2/pom.xml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/codec-http2/pom.xml b/codec-http2/pom.xml index 9eb7190c0af..313a3536696 100644 --- a/codec-http2/pom.xml +++ b/codec-http2/pom.xml @@ -17,18 +17,6 @@ 4.0.0 - - - - org.apache.maven.plugins - maven-compiler-plugin - - 7 - 7 - - - - io.netty netty-parent From b610b183fc4f612793b021f1cd4c72fd02bbe211 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Fri, 9 Apr 2021 17:23:10 -0700 Subject: [PATCH 04/13] show more clear picture in test --- .../netty/handler/codec/http2/StreamBufferingEncoderTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java index 71a83cfba95..f5d6d980252 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java @@ -252,7 +252,10 @@ public void receivingGoAwayFailsBufferedStreams() throws Http2Exception { } assertEquals(5, connection.numActiveStreams()); assertEquals(4, encoder.numBufferedStreams()); + connection.goAwayReceived(11, 8, EMPTY_BUFFER); + assertEquals(5, connection.numActiveStreams()); + assertEquals(0, encoder.numBufferedStreams()); int failCount = 0; for (ChannelFuture f : futures) { if (f.cause() != null) { From 5329184a58f58a8fa4468fd81c54ad2169f3475b Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Mon, 12 Apr 2021 11:19:35 -0700 Subject: [PATCH 05/13] save goAwayException --- .../codec/http2/StreamBufferingEncoder.java | 14 ++++++++------ .../codec/http2/StreamBufferingEncoderTest.java | 11 ++++------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java index a81da6c0bf1..87cde3e7314 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java @@ -106,6 +106,7 @@ public byte[] debugData() { private final TreeMap pendingStreams = new TreeMap(); private int maxConcurrentStreams; private boolean closed; + private Http2GoAwayException goAwayException; public StreamBufferingEncoder(Http2ConnectionEncoder delegate) { this(delegate, SMALLEST_MAX_CONCURRENT_STREAMS); @@ -118,7 +119,9 @@ public StreamBufferingEncoder(Http2ConnectionEncoder delegate, int initialMaxCon @Override public void onGoAwayReceived(int lastStreamId, long errorCode, ByteBuf debugData) { - cancelGoAwayStreams(lastStreamId, errorCode, debugData); + goAwayException = new Http2GoAwayException( + lastStreamId, errorCode, ByteBufUtil.getBytes(debugData)); + cancelGoAwayStreams(lastStreamId); } @Override @@ -153,8 +156,8 @@ public ChannelFuture writeHeaders(ChannelHandlerContext ctx, int streamId, Http2 return super.writeHeaders(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream, promise); } - if (connection().goAwayReceived()) { - promise.setFailure(new Http2Exception(Http2Error.NO_ERROR, "GOAWAY received")); + if (goAwayException != null) { + promise.setFailure(goAwayException); return promise; } PendingStream pendingStream = pendingStreams.get(streamId); @@ -248,14 +251,13 @@ private void tryCreatePendingStreams() { } } - private void cancelGoAwayStreams(int lastStreamId, long errorCode, ByteBuf debugData) { + private void cancelGoAwayStreams(int lastStreamId) { Iterator iter = pendingStreams.values().iterator(); - Exception e = new Http2GoAwayException(lastStreamId, errorCode, ByteBufUtil.getBytes(debugData)); while (iter.hasNext()) { PendingStream stream = iter.next(); if (stream.streamId > lastStreamId) { iter.remove(); - stream.close(e); + stream.close(goAwayException); } } } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java index f5d6d980252..26c1714315a 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/StreamBufferingEncoderTest.java @@ -49,6 +49,7 @@ import io.netty.channel.ChannelPromise; import io.netty.channel.DefaultChannelPromise; import io.netty.channel.DefaultMessageSizeEstimator; +import io.netty.handler.codec.http2.StreamBufferingEncoder.Http2GoAwayException; import io.netty.util.ReferenceCountUtil; import io.netty.util.concurrent.EventExecutor; import io.netty.util.concurrent.ImmediateEventExecutor; @@ -254,14 +255,13 @@ public void receivingGoAwayFailsBufferedStreams() throws Http2Exception { assertEquals(4, encoder.numBufferedStreams()); connection.goAwayReceived(11, 8, EMPTY_BUFFER); + assertEquals(5, connection.numActiveStreams()); assertEquals(0, encoder.numBufferedStreams()); int failCount = 0; for (ChannelFuture f : futures) { if (f.cause() != null) { - assertTrue(f.cause() instanceof Http2Exception); - Http2Exception e = (Http2Exception) f.cause(); - assertEquals(Http2Error.STREAM_CLOSED, e.error()); + assertTrue(f.cause() instanceof Http2GoAwayException); failCount++; } } @@ -276,10 +276,7 @@ public void receivingGoAwayFailsNewStreamIfMaxConcurrentStreamsReached() throws connection.goAwayReceived(11, 8, EMPTY_BUFFER); ChannelFuture f = encoderWriteHeaders(5, newPromise()); - assertTrue(f.cause() instanceof Http2Exception); - Http2Exception e = (Http2Exception) f.cause(); - assertEquals(Http2Error.NO_ERROR, e.error()); - assertEquals("GOAWAY received", e.getMessage()); + assertTrue(f.cause() instanceof Http2GoAwayException); assertEquals(0, encoder.numBufferedStreams()); } From 4ce08983a2fe9b96857de5cfbc7baf2abee3897a Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Mon, 12 Apr 2021 11:47:51 -0700 Subject: [PATCH 06/13] save data, not exeptioin --- .../codec/http2/StreamBufferingEncoder.java | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java index 87cde3e7314..c618fcb3801 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java @@ -106,7 +106,9 @@ public byte[] debugData() { private final TreeMap pendingStreams = new TreeMap(); private int maxConcurrentStreams; private boolean closed; - private Http2GoAwayException goAwayException; + private Integer goAwayLastStreamId; + private long goAwayErrorCode; + private ByteBuf goAwayDebugData; public StreamBufferingEncoder(Http2ConnectionEncoder delegate) { this(delegate, SMALLEST_MAX_CONCURRENT_STREAMS); @@ -119,9 +121,10 @@ public StreamBufferingEncoder(Http2ConnectionEncoder delegate, int initialMaxCon @Override public void onGoAwayReceived(int lastStreamId, long errorCode, ByteBuf debugData) { - goAwayException = new Http2GoAwayException( - lastStreamId, errorCode, ByteBufUtil.getBytes(debugData)); - cancelGoAwayStreams(lastStreamId); + goAwayLastStreamId = lastStreamId; + goAwayErrorCode = errorCode; + goAwayDebugData = debugData; + cancelGoAwayStreams(); } @Override @@ -156,8 +159,9 @@ public ChannelFuture writeHeaders(ChannelHandlerContext ctx, int streamId, Http2 return super.writeHeaders(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream, promise); } - if (goAwayException != null) { - promise.setFailure(goAwayException); + if (goAwayLastStreamId != null) { + promise.setFailure(new Http2GoAwayException( + goAwayLastStreamId, goAwayErrorCode, ByteBufUtil.getBytes(goAwayDebugData))); return promise; } PendingStream pendingStream = pendingStreams.get(streamId); @@ -251,13 +255,15 @@ private void tryCreatePendingStreams() { } } - private void cancelGoAwayStreams(int lastStreamId) { + private void cancelGoAwayStreams() { Iterator iter = pendingStreams.values().iterator(); + Exception e = new Http2GoAwayException( + goAwayLastStreamId, goAwayErrorCode, ByteBufUtil.getBytes(goAwayDebugData)); while (iter.hasNext()) { PendingStream stream = iter.next(); - if (stream.streamId > lastStreamId) { + if (stream.streamId > goAwayLastStreamId) { iter.remove(); - stream.close(goAwayException); + stream.close(e); } } } From fa6615822709c0659b6c611dd8a5297423fcbbd1 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Tue, 13 Apr 2021 17:21:50 -0700 Subject: [PATCH 07/13] use byte array --- .../codec/http2/StreamBufferingEncoder.java | 52 +++++++++++-------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java index c618fcb3801..078569970ff 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java @@ -69,33 +69,45 @@ public Http2ChannelClosedException() { } } + private static final class GoAwayDetail { + private final int lastStreamId; + private final long errorCode; + private final byte[] debugData; + + GoAwayDetail(int lastStreamId, long errorCode, byte[] debugData) { + this.lastStreamId = lastStreamId; + this.errorCode = errorCode; + this.debugData = debugData.clone(); + } + } + /** * Thrown by {@link StreamBufferingEncoder} if buffered streams are terminated due to * receipt of a {@code GOAWAY}. */ public static final class Http2GoAwayException extends Http2Exception { private static final long serialVersionUID = 1326785622777291198L; - private final int lastStreamId; - private final long errorCode; - private final byte[] debugData; + private GoAwayDetail goAwayDetail; public Http2GoAwayException(int lastStreamId, long errorCode, byte[] debugData) { + this(new GoAwayDetail(lastStreamId, errorCode, debugData)); + } + + private Http2GoAwayException(GoAwayDetail goAwayDetail) { super(Http2Error.STREAM_CLOSED); - this.lastStreamId = lastStreamId; - this.errorCode = errorCode; - this.debugData = debugData; + this.goAwayDetail = goAwayDetail; } public int lastStreamId() { - return lastStreamId; + return goAwayDetail.lastStreamId; } public long errorCode() { - return errorCode; + return goAwayDetail.errorCode; } public byte[] debugData() { - return debugData; + return goAwayDetail.debugData; } } @@ -106,9 +118,7 @@ public byte[] debugData() { private final TreeMap pendingStreams = new TreeMap(); private int maxConcurrentStreams; private boolean closed; - private Integer goAwayLastStreamId; - private long goAwayErrorCode; - private ByteBuf goAwayDebugData; + private GoAwayDetail goAwayDetail; public StreamBufferingEncoder(Http2ConnectionEncoder delegate) { this(delegate, SMALLEST_MAX_CONCURRENT_STREAMS); @@ -121,10 +131,9 @@ public StreamBufferingEncoder(Http2ConnectionEncoder delegate, int initialMaxCon @Override public void onGoAwayReceived(int lastStreamId, long errorCode, ByteBuf debugData) { - goAwayLastStreamId = lastStreamId; - goAwayErrorCode = errorCode; - goAwayDebugData = debugData; - cancelGoAwayStreams(); + goAwayDetail = new GoAwayDetail( + lastStreamId, errorCode, ByteBufUtil.getBytes(debugData)); + cancelGoAwayStreams(goAwayDetail); } @Override @@ -159,9 +168,9 @@ public ChannelFuture writeHeaders(ChannelHandlerContext ctx, int streamId, Http2 return super.writeHeaders(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream, promise); } - if (goAwayLastStreamId != null) { + if (goAwayDetail != null) { promise.setFailure(new Http2GoAwayException( - goAwayLastStreamId, goAwayErrorCode, ByteBufUtil.getBytes(goAwayDebugData))); + goAwayDetail.lastStreamId, goAwayDetail.lastStreamId, goAwayDetail.debugData)); return promise; } PendingStream pendingStream = pendingStreams.get(streamId); @@ -255,13 +264,12 @@ private void tryCreatePendingStreams() { } } - private void cancelGoAwayStreams() { + private void cancelGoAwayStreams(GoAwayDetail goAwayDetail) { Iterator iter = pendingStreams.values().iterator(); - Exception e = new Http2GoAwayException( - goAwayLastStreamId, goAwayErrorCode, ByteBufUtil.getBytes(goAwayDebugData)); + Exception e = new Http2GoAwayException(goAwayDetail); while (iter.hasNext()) { PendingStream stream = iter.next(); - if (stream.streamId > goAwayLastStreamId) { + if (stream.streamId > goAwayDetail.lastStreamId) { iter.remove(); stream.close(e); } From 8f78a158e822d329c794498213aeabdde73a0210 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Thu, 15 Apr 2021 17:49:58 -0700 Subject: [PATCH 08/13] address comments --- .../netty/handler/codec/http2/StreamBufferingEncoder.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java index 078569970ff..a7887886352 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java @@ -93,7 +93,7 @@ public Http2GoAwayException(int lastStreamId, long errorCode, byte[] debugData) this(new GoAwayDetail(lastStreamId, errorCode, debugData)); } - private Http2GoAwayException(GoAwayDetail goAwayDetail) { + Http2GoAwayException(GoAwayDetail goAwayDetail) { super(Http2Error.STREAM_CLOSED); this.goAwayDetail = goAwayDetail; } @@ -107,7 +107,7 @@ public long errorCode() { } public byte[] debugData() { - return goAwayDetail.debugData; + return goAwayDetail.debugData.clone(); } } @@ -169,8 +169,7 @@ public ChannelFuture writeHeaders(ChannelHandlerContext ctx, int streamId, Http2 exclusive, padding, endOfStream, promise); } if (goAwayDetail != null) { - promise.setFailure(new Http2GoAwayException( - goAwayDetail.lastStreamId, goAwayDetail.lastStreamId, goAwayDetail.debugData)); + promise.setFailure(new Http2GoAwayException(goAwayDetail)); return promise; } PendingStream pendingStream = pendingStreams.get(streamId); From d456d5b72bcb970ba404ba883f9c44c2467c07de Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 19 Apr 2021 12:46:48 +0200 Subject: [PATCH 09/13] Update codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java --- .../io/netty/handler/codec/http2/StreamBufferingEncoder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java index a7887886352..175e37ce571 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java @@ -132,7 +132,8 @@ public StreamBufferingEncoder(Http2ConnectionEncoder delegate, int initialMaxCon @Override public void onGoAwayReceived(int lastStreamId, long errorCode, ByteBuf debugData) { goAwayDetail = new GoAwayDetail( - lastStreamId, errorCode, ByteBufUtil.getBytes(debugData)); + // Using getBytes(..., false) is safe here as GoAwayDetail(...) will clone the byte[]. + lastStreamId, errorCode, ByteBufUtil.getBytes(debugData, debugData.readerIndex(), debugData.readableBytes(), false)); cancelGoAwayStreams(goAwayDetail); } From 3c507c23b5637651b53c20d9bba4ce2eed6d0e9c Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 19 Apr 2021 12:46:55 +0200 Subject: [PATCH 10/13] Update codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java --- .../io/netty/handler/codec/http2/StreamBufferingEncoder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java index 175e37ce571..752e8e0b562 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java @@ -87,7 +87,7 @@ private static final class GoAwayDetail { */ public static final class Http2GoAwayException extends Http2Exception { private static final long serialVersionUID = 1326785622777291198L; - private GoAwayDetail goAwayDetail; + private final GoAwayDetail goAwayDetail; public Http2GoAwayException(int lastStreamId, long errorCode, byte[] debugData) { this(new GoAwayDetail(lastStreamId, errorCode, debugData)); From 9fb5c0063e6ea7f58a88abae10097f38622d4e29 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 19 Apr 2021 12:47:34 +0200 Subject: [PATCH 11/13] Apply suggestions from code review --- .../io/netty/handler/codec/http2/StreamBufferingEncoder.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java index 752e8e0b562..e6b4e3b1569 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java @@ -170,8 +170,7 @@ public ChannelFuture writeHeaders(ChannelHandlerContext ctx, int streamId, Http2 exclusive, padding, endOfStream, promise); } if (goAwayDetail != null) { - promise.setFailure(new Http2GoAwayException(goAwayDetail)); - return promise; + return promise.setFailure(new Http2GoAwayException(goAwayDetail)); } PendingStream pendingStream = pendingStreams.get(streamId); if (pendingStream == null) { From e3a96d5b791834c90fb6d8b149ef9f3291b11ed2 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 19 Apr 2021 12:50:30 +0200 Subject: [PATCH 12/13] Update codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java --- .../io/netty/handler/codec/http2/StreamBufferingEncoder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java index e6b4e3b1569..1e3f61839e0 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java @@ -133,7 +133,8 @@ public StreamBufferingEncoder(Http2ConnectionEncoder delegate, int initialMaxCon public void onGoAwayReceived(int lastStreamId, long errorCode, ByteBuf debugData) { goAwayDetail = new GoAwayDetail( // Using getBytes(..., false) is safe here as GoAwayDetail(...) will clone the byte[]. - lastStreamId, errorCode, ByteBufUtil.getBytes(debugData, debugData.readerIndex(), debugData.readableBytes(), false)); + lastStreamId, errorCode, + ByteBufUtil.getBytes(debugData, debugData.readerIndex(), debugData.readableBytes(), false)); cancelGoAwayStreams(goAwayDetail); } From 8eb084b73f96fc095682af6a52c643921d2ff728 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 19 Apr 2021 14:05:50 +0200 Subject: [PATCH 13/13] Update codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java --- .../io/netty/handler/codec/http2/StreamBufferingEncoder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java index 1e3f61839e0..c3c340bc82b 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java @@ -133,7 +133,7 @@ public StreamBufferingEncoder(Http2ConnectionEncoder delegate, int initialMaxCon public void onGoAwayReceived(int lastStreamId, long errorCode, ByteBuf debugData) { goAwayDetail = new GoAwayDetail( // Using getBytes(..., false) is safe here as GoAwayDetail(...) will clone the byte[]. - lastStreamId, errorCode, + lastStreamId, errorCode, ByteBufUtil.getBytes(debugData, debugData.readerIndex(), debugData.readableBytes(), false)); cancelGoAwayStreams(goAwayDetail); }