update quiche, update to netty 4.1.105
parent
dde0ad8a11
commit
ada04aef58
@ -0,0 +1 @@
|
||||
apply plugin: 'org.xbib.gradle.plugin.cmake'
|
||||
@ -1,40 +0,0 @@
|
||||
cmake_minimum_required(VERSION 3.13 FATAL_ERROR)
|
||||
|
||||
project(netty-channel-unix-native VERSION 4.1.104 LANGUAGES CXX)
|
||||
|
||||
set(CMAKE_CXX_STANDARD 17)
|
||||
set(CMAKE_CXX_STANDARD_REQUIRED ON)
|
||||
set(CMAKE_CXX_EXTENSIONS OFF)
|
||||
|
||||
include(GNUInstallDirs)
|
||||
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR})
|
||||
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR})
|
||||
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_BINDIR})
|
||||
|
||||
# jvm
|
||||
find_package(Java REQUIRED)
|
||||
# https://stackoverflow.com/questions/51047978/cmake-could-not-find-jni
|
||||
set(JAVA_AWT_LIBRARY NotNeeded)
|
||||
set(JAVA_JVM_LIBRARY NotNeeded)
|
||||
find_package(JNI REQUIRED)
|
||||
|
||||
include_directories(${JNI_INCLUDE_DIRS})
|
||||
|
||||
# force off-tree build
|
||||
if(${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_BINARY_DIR})
|
||||
message(FATAL_ERROR "CMake generation is not allowed within the source directory!
|
||||
Remove the CMakeCache.txt file and try again from another folder, e.g.:
|
||||
mkdir build && cd build
|
||||
cmake ..
|
||||
")
|
||||
endif()
|
||||
|
||||
# default to Release build
|
||||
if(NOT CMAKE_BUILD_TYPE)
|
||||
set(CMAKE_BUILD_TYPE Release CACHE STRING
|
||||
"Choose the type of build, options are: None Debug Release RelWithDebInfo MinSizeRel."
|
||||
FORCE)
|
||||
endif()
|
||||
|
||||
include_directories(src/main/include)
|
||||
add_subdirectory(src/main/c)
|
||||
@ -1,10 +1,3 @@
|
||||
/* currently we do not build our C code natively, but we provide copies of the binaries in META-INF/native */
|
||||
|
||||
tasks.register("compile", org.xbib.gradle.plugin.c.tasks.ExtendedCCompile) {
|
||||
group = "build"
|
||||
source(layout.projectDirectory.files("src/main/c").asFileTree)
|
||||
includes(layout.projectDirectory.files("src/main/headers"))
|
||||
toolChain.set(c.localTool("12.3.1", "/usr/bin/gcc", ".o"))
|
||||
compilerArgs.addAll('-v', '-O3', '-Werror', '-Wno-attributes', '-fPIC', '-fno-omit-frame-pointer', '-Wunused-variable', '-fvisibility=hidden')
|
||||
targetPlatform.set(c.platform())
|
||||
objectFileDir.set(layout.buildDirectory.dir("libs"))
|
||||
}
|
||||
/* the static library is included in other native builds, so nothing is provided here */
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@ -1,66 +0,0 @@
|
||||
/*
|
||||
* Copyright (c) 1996, 2020, Oracle and/or its affiliates. All rights reserved.
|
||||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
|
||||
*
|
||||
* This code is free software; you can redistribute it and/or modify it
|
||||
* under the terms of the GNU General Public License version 2 only, as
|
||||
* published by the Free Software Foundation. Oracle designates this
|
||||
* particular file as subject to the "Classpath" exception as provided
|
||||
* by Oracle in the LICENSE file that accompanied this code.
|
||||
*
|
||||
* This code is distributed in the hope that it will be useful, but WITHOUT
|
||||
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
|
||||
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
|
||||
* version 2 for more details (a copy is included in the LICENSE file that
|
||||
* accompanied this code).
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License version
|
||||
* 2 along with this work; if not, write to the Free Software Foundation,
|
||||
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
|
||||
*
|
||||
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
|
||||
* or visit www.oracle.com if you need additional information or have any
|
||||
* questions.
|
||||
*/
|
||||
|
||||
#ifndef _JAVASOFT_JNI_MD_H_
|
||||
#define _JAVASOFT_JNI_MD_H_
|
||||
|
||||
#ifndef __has_attribute
|
||||
#define __has_attribute(x) 0
|
||||
#endif
|
||||
|
||||
#ifndef JNIEXPORT
|
||||
#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) && (__GNUC_MINOR__ > 2))) || __has_attribute(visibility)
|
||||
#ifdef ARM
|
||||
#define JNIEXPORT __attribute__((externally_visible,visibility("default")))
|
||||
#else
|
||||
#define JNIEXPORT __attribute__((visibility("default")))
|
||||
#endif
|
||||
#else
|
||||
#define JNIEXPORT
|
||||
#endif
|
||||
#endif
|
||||
|
||||
#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) && (__GNUC_MINOR__ > 2))) || __has_attribute(visibility)
|
||||
#ifdef ARM
|
||||
#define JNIIMPORT __attribute__((externally_visible,visibility("default")))
|
||||
#else
|
||||
#define JNIIMPORT __attribute__((visibility("default")))
|
||||
#endif
|
||||
#else
|
||||
#define JNIIMPORT
|
||||
#endif
|
||||
|
||||
#define JNICALL
|
||||
|
||||
typedef int jint;
|
||||
#ifdef _LP64
|
||||
typedef long jlong;
|
||||
#else
|
||||
typedef long long jlong;
|
||||
#endif
|
||||
|
||||
typedef signed char jbyte;
|
||||
|
||||
#endif /* !_JAVASOFT_JNI_MD_H_ */
|
||||
@ -0,0 +1,18 @@
|
||||
#!/bin/bash
|
||||
|
||||
VERSION=0.0.56.Final
|
||||
|
||||
wget "https://repo1.maven.org/maven2/io/netty/incubator/netty-incubator-codec-native-quic/$VERSION/netty-incubator-codec-native-quic-$VERSION-linux-x86_64.jar"
|
||||
jar xf netty-incubator-codec-native-quic-$VERSION-linux-x86_64.jar META-INF/native/libnetty_quiche_linux_x86_64.so
|
||||
|
||||
wget "https://repo1.maven.org/maven2/io/netty/incubator/netty-incubator-codec-native-quic/$VERSION/netty-incubator-codec-native-quic-$VERSION-linux-aarch_64.jar"
|
||||
jar xf netty-incubator-codec-native-quic-$VERSION-linux-aarch_64.jar META-INF/native/libnetty_quiche_linux_aarch_64.so
|
||||
|
||||
wget "https://repo1.maven.org/maven2/io/netty/incubator/netty-incubator-codec-native-quic/$VERSION/netty-incubator-codec-native-quic-$VERSION-osx-aarch_64.jar"
|
||||
jar xf netty-incubator-codec-native-quic-$VERSION-osx-aarch_64.jar META-INF/native/libnetty_quiche_osx_aarch_64.jnilib
|
||||
|
||||
wget "https://repo1.maven.org/maven2/io/netty/incubator/netty-incubator-codec-native-quic/$VERSION/netty-incubator-codec-native-quic-$VERSION-osx-x86_64.jar"
|
||||
jar xf netty-incubator-codec-native-quic-$VERSION-osx-x86_64.jar META-INF/native/libnetty_quiche_osx_x86_64.jnilib
|
||||
|
||||
wget "https://repo1.maven.org/maven2/io/netty/incubator/netty-incubator-codec-native-quic/$VERSION/netty-incubator-codec-native-quic-$VERSION-windows-x86_64.jar"
|
||||
jar xf netty-incubator-codec-native-quic-$VERSION-windows-x86_64.jar META-INF/native/netty_quiche_windows_x86_64.dll
|
||||
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
@ -0,0 +1,55 @@
|
||||
From 49952a6302c10628155d8344f57473ceb355c9be Mon Sep 17 00:00:00 2001
|
||||
From: Francesco Nigro <nigro.fra@gmail.com>
|
||||
Date: Tue, 16 Jan 2024 13:48:59 +0100
|
||||
Subject: [PATCH] Short-circuit ByteBuf::release
|
||||
|
||||
Motivation:
|
||||
|
||||
ReferenceCountUtil::safeRelease can both hit interface virtual calls and requires checking for an interface type (ReferenceCounted)
|
||||
|
||||
Modifications:
|
||||
|
||||
Perform a class check to save both.
|
||||
|
||||
Result:
|
||||
|
||||
Faster buffers release
|
||||
---
|
||||
.../io/netty/channel/ChannelOutboundBuffer.java | 16 ++++++++++++++--
|
||||
1 file changed, 14 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/transport/src/main/java/io/netty/channel/ChannelOutboundBuffer.java b/transport/src/main/java/io/netty/channel/ChannelOutboundBuffer.java
|
||||
index 7008ef558ccc..27119e8fbf47 100644
|
||||
--- a/transport/src/main/java/io/netty/channel/ChannelOutboundBuffer.java
|
||||
+++ b/transport/src/main/java/io/netty/channel/ChannelOutboundBuffer.java
|
||||
@@ -15,6 +15,7 @@
|
||||
*/
|
||||
package io.netty.channel;
|
||||
|
||||
+import io.netty.buffer.AbstractReferenceCountedByteBuf;
|
||||
import io.netty.buffer.ByteBuf;
|
||||
import io.netty.buffer.ByteBufHolder;
|
||||
import io.netty.buffer.Unpooled;
|
||||
@@ -275,9 +276,20 @@ public boolean remove() {
|
||||
|
||||
removeEntry(e);
|
||||
|
||||
+ // only release message, notify and decrement if it was not canceled before.
|
||||
if (!e.cancelled) {
|
||||
- // only release message, notify and decrement if it was not canceled before.
|
||||
- ReferenceCountUtil.safeRelease(msg);
|
||||
+ // this save both checking against the ReferenceCounted interface
|
||||
+ // and makes better use of virtual calls vs interface ones
|
||||
+ if (msg instanceof AbstractReferenceCountedByteBuf) {
|
||||
+ try {
|
||||
+ // release now as it is flushed.
|
||||
+ ((AbstractReferenceCountedByteBuf) msg).release();
|
||||
+ } catch (Throwable t) {
|
||||
+ logger.warn("Failed to release a ByteBuf: {}", msg, t);
|
||||
+ }
|
||||
+ } else {
|
||||
+ ReferenceCountUtil.safeRelease(msg);
|
||||
+ }
|
||||
safeSuccess(promise);
|
||||
decrementPendingOutboundBytes(size, false, true);
|
||||
}
|
||||
@ -0,0 +1,110 @@
|
||||
From 60441afc56decf21808b8b2767d6769c787f82a5 Mon Sep 17 00:00:00 2001
|
||||
From: Norman Maurer <norman_maurer@apple.com>
|
||||
Date: Wed, 17 Jan 2024 13:12:19 +0100
|
||||
Subject: [PATCH] DnsNameResolver: Fail query if id space is exhausted
|
||||
|
||||
Motivation:
|
||||
|
||||
When we try to execute a query we will try to select / generate an id that is used. When we are not able to find one we throw an IllegalStateException. In this case we also need to ensure we fail the original promise as otherwise the user might never be notified of the problem.
|
||||
|
||||
Modifications:
|
||||
|
||||
- Move throw code out of the DnsQueryContextManager to make it easier to reason about
|
||||
- Fail query with IllegalStateException
|
||||
|
||||
Result:
|
||||
|
||||
Query will be correctly failed in the case of id space exhausting
|
||||
---
|
||||
.../netty/resolver/dns/DnsQueryContext.java | 13 ++++++---
|
||||
.../resolver/dns/DnsQueryContextManager.java | 27 +++++++++++++++++--
|
||||
2 files changed, 35 insertions(+), 5 deletions(-)
|
||||
|
||||
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java
|
||||
index da1091b1d160..348d15b30bd0 100644
|
||||
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java
|
||||
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java
|
||||
@@ -81,7 +81,7 @@ abstract class DnsQueryContext {
|
||||
|
||||
private volatile Future<?> timeoutFuture;
|
||||
|
||||
- private int id = -1;
|
||||
+ private int id = Integer.MIN_VALUE;
|
||||
|
||||
DnsQueryContext(Channel channel,
|
||||
Future<? extends Channel> channelReadyFuture,
|
||||
@@ -172,8 +172,15 @@ final DnsQuestion question() {
|
||||
* @return the {@link ChannelFuture} that is notified once once the write completes.
|
||||
*/
|
||||
final ChannelFuture writeQuery(boolean flush) {
|
||||
- assert id == -1 : this.getClass().getSimpleName() + ".writeQuery(...) can only be executed once.";
|
||||
- id = queryContextManager.add(nameServerAddr, this);
|
||||
+ assert id == Integer.MIN_VALUE : this.getClass().getSimpleName() +
|
||||
+ ".writeQuery(...) can only be executed once.";
|
||||
+
|
||||
+ if ((id = queryContextManager.add(nameServerAddr, this)) == -1) {
|
||||
+ // We did exhaust the id space, fail the query
|
||||
+ IllegalStateException e = new IllegalStateException("query ID space exhausted: " + question());
|
||||
+ finishFailure("failed to send a query via " + protocol(), e, false);
|
||||
+ return channel.newFailedFuture(e);
|
||||
+ }
|
||||
|
||||
// Ensure we remove the id from the QueryContextManager once the query completes.
|
||||
promise.addListener(new FutureListener<AddressedEnvelope<DnsResponse, InetSocketAddress>>() {
|
||||
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContextManager.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContextManager.java
|
||||
index f82648c43dd2..773eecca0aad 100644
|
||||
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContextManager.java
|
||||
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContextManager.java
|
||||
@@ -38,11 +38,27 @@ final class DnsQueryContextManager {
|
||||
private final Map<InetSocketAddress, DnsQueryContextMap> map =
|
||||
new HashMap<InetSocketAddress, DnsQueryContextMap>();
|
||||
|
||||
+ /**
|
||||
+ * Add {@link DnsQueryContext} to the context manager and return the ID that should be used for the query.
|
||||
+ * This method will return {@code -1} if an ID could not be generated and the context was not stored.
|
||||
+ *
|
||||
+ * @param nameServerAddr The {@link InetSocketAddress} of the nameserver to query.
|
||||
+ * @param qCtx The {@link {@link DnsQueryContext} to store.
|
||||
+ * @return the ID that should be used or {@code -1} if none could be generated.
|
||||
+ */
|
||||
int add(InetSocketAddress nameServerAddr, DnsQueryContext qCtx) {
|
||||
final DnsQueryContextMap contexts = getOrCreateContextMap(nameServerAddr);
|
||||
return contexts.add(qCtx);
|
||||
}
|
||||
|
||||
+ /**
|
||||
+ * Return the {@link DnsQueryContext} for the given {@link InetSocketAddress} and id or {@code null} if
|
||||
+ * none could be found.
|
||||
+ *
|
||||
+ * @param nameServerAddr The {@link InetSocketAddress} of the nameserver.
|
||||
+ * @param id The id that identifies the {@link DnsQueryContext} and was used for the query.
|
||||
+ * @return The context or {@code null} if none could be found.
|
||||
+ */
|
||||
DnsQueryContext get(InetSocketAddress nameServerAddr, int id) {
|
||||
final DnsQueryContextMap contexts = getContextMap(nameServerAddr);
|
||||
if (contexts == null) {
|
||||
@@ -51,6 +67,14 @@ DnsQueryContext get(InetSocketAddress nameServerAddr, int id) {
|
||||
return contexts.get(id);
|
||||
}
|
||||
|
||||
+ /**
|
||||
+ * Remove the {@link DnsQueryContext} for the given {@link InetSocketAddress} and id or {@code null} if
|
||||
+ * none could be found.
|
||||
+ *
|
||||
+ * @param nameServerAddr The {@link InetSocketAddress} of the nameserver.
|
||||
+ * @param id The id that identifies the {@link DnsQueryContext} and was used for the query.
|
||||
+ * @return The context or {@code null} if none could be removed.
|
||||
+ */
|
||||
DnsQueryContext remove(InetSocketAddress nameServerAddr, int id) {
|
||||
final DnsQueryContextMap contexts = getContextMap(nameServerAddr);
|
||||
if (contexts == null) {
|
||||
@@ -150,8 +174,7 @@ synchronized int add(DnsQueryContext ctx) {
|
||||
|
||||
id = id + 1 & 0xFFFF;
|
||||
if (++tries >= MAX_TRIES) {
|
||||
- throw new IllegalStateException(
|
||||
- "query ID space exhausted after " + MAX_TRIES + ": " + ctx.question());
|
||||
+ return -1;
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,113 @@
|
||||
From 284ad8e5ce3ec87964dea6ec8a611dfb4df62a29 Mon Sep 17 00:00:00 2001
|
||||
From: songmw725 <songmw725@gmail.com>
|
||||
Date: Thu, 18 Jan 2024 00:11:34 +0900
|
||||
Subject: [PATCH] Prevent sharing the index of the continuation frame header
|
||||
ByteBuf. Motivation: The current implementation uses the `byteBuf` for a
|
||||
continuation frame header multiple times if the header length exceeds `3 *
|
||||
maxFrameLength`. However, it fails to slice the `byteBuf` during usage.
|
||||
[Reference](https://github.com/netty/netty/blob/d027ba7320d430743992d613e52596b0182ca854/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java#L570)
|
||||
|
||||
Modification:
|
||||
- Introduce `ByteBuf.retainedSlice()` for a continuation frame header when it's used to prevent sharing the index.
|
||||
|
||||
Result:
|
||||
- Correctly send continuation frame headers to the remote peer, addressing the issue of reusing the index of the ByteBuf.
|
||||
---
|
||||
.../codec/http2/DefaultHttp2FrameWriter.java | 2 +-
|
||||
.../http2/DefaultHttp2FrameWriterTest.java | 49 ++++++++++++++-----
|
||||
2 files changed, 39 insertions(+), 12 deletions(-)
|
||||
|
||||
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java
|
||||
index 9b608921c28b..8540e0c2ed6e 100644
|
||||
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java
|
||||
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java
|
||||
@@ -567,7 +567,7 @@ private ChannelFuture writeContinuationFrames(ChannelHandlerContext ctx, int str
|
||||
ByteBuf fragment = headerBlock.readRetainedSlice(fragmentReadableBytes);
|
||||
|
||||
if (headerBlock.isReadable()) {
|
||||
- ctx.write(buf.retain(), promiseAggregator.newPromise());
|
||||
+ ctx.write(buf.retainedSlice(), promiseAggregator.newPromise());
|
||||
} else {
|
||||
// The frame header is different for the last frame, so re-allocate and release the old buffer
|
||||
flags = flags.endOfHeaders(true);
|
||||
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriterTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriterTest.java
|
||||
index e134fb8194df..8311a20823ef 100644
|
||||
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriterTest.java
|
||||
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriterTest.java
|
||||
@@ -189,7 +189,7 @@ public void writeLargeHeaders() throws Exception {
|
||||
int streamId = 1;
|
||||
Http2Headers headers = new DefaultHttp2Headers()
|
||||
.method("GET").path("/").authority("foo.com").scheme("https");
|
||||
- headers = dummyHeaders(headers, 20);
|
||||
+ headers = dummyHeaders(headers, 60);
|
||||
|
||||
http2HeadersEncoder.configuration().maxHeaderListSize(Integer.MAX_VALUE);
|
||||
frameWriter.headersConfiguration().maxHeaderListSize(Integer.MAX_VALUE);
|
||||
@@ -198,7 +198,7 @@ public void writeLargeHeaders() throws Exception {
|
||||
|
||||
byte[] expectedPayload = headerPayload(streamId, headers);
|
||||
|
||||
- // First frame: HEADER(length=0x4000, flags=0x01)
|
||||
+ // First frame: HEADER(length=0x4000, type=0x01, flags=0x01)
|
||||
assertEquals(Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND,
|
||||
outbound.readUnsignedMedium());
|
||||
assertEquals(0x01, outbound.readByte());
|
||||
@@ -207,22 +207,49 @@ public void writeLargeHeaders() throws Exception {
|
||||
|
||||
byte[] firstPayload = new byte[Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND];
|
||||
outbound.readBytes(firstPayload);
|
||||
+ int index = 0;
|
||||
+ assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + firstPayload.length),
|
||||
+ firstPayload);
|
||||
+ index += firstPayload.length;
|
||||
|
||||
- int remainPayloadLength = expectedPayload.length - Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND;
|
||||
- // Second frame: CONTINUATION(length=remainPayloadLength, flags=0x04)
|
||||
+ // Second frame: HEADER(length=0x4000, type=0x09, flags=0x00)
|
||||
+ assertEquals(Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND,
|
||||
+ outbound.readUnsignedMedium());
|
||||
+ assertEquals(0x09, outbound.readByte());
|
||||
+ assertEquals(0x00, outbound.readByte());
|
||||
+ assertEquals(streamId, outbound.readInt());
|
||||
+
|
||||
+ byte[] secondPayload = new byte[Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND];
|
||||
+ outbound.readBytes(secondPayload);
|
||||
+ assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + secondPayload.length),
|
||||
+ secondPayload);
|
||||
+ index += secondPayload.length;
|
||||
+
|
||||
+ // third frame: HEADER(length=0x4000, type=0x09, flags=0x00)
|
||||
+ assertEquals(Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND,
|
||||
+ outbound.readUnsignedMedium());
|
||||
+ assertEquals(0x09, outbound.readByte());
|
||||
+ assertEquals(0x00, outbound.readByte());
|
||||
+ assertEquals(streamId, outbound.readInt());
|
||||
+
|
||||
+ byte[] thirdPayload = new byte[Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND];
|
||||
+ outbound.readBytes(thirdPayload);
|
||||
+ assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + thirdPayload.length),
|
||||
+ thirdPayload);
|
||||
+ index += thirdPayload.length;
|
||||
+
|
||||
+ int remainPayloadLength = expectedPayload.length - index;
|
||||
+ // Second frame: CONTINUATION(length=remainPayloadLength, type=0x09, flags=0x04)
|
||||
assertEquals(remainPayloadLength, outbound.readUnsignedMedium());
|
||||
assertEquals(0x09, outbound.readByte());
|
||||
assertEquals(0x04, outbound.readByte());
|
||||
assertEquals(streamId, outbound.readInt());
|
||||
|
||||
- byte[] secondPayload = new byte[remainPayloadLength];
|
||||
- outbound.readBytes(secondPayload);
|
||||
+ byte[] fourthPayload = new byte[remainPayloadLength];
|
||||
+ outbound.readBytes(fourthPayload);
|
||||
|
||||
- assertArrayEquals(Arrays.copyOfRange(expectedPayload, 0, firstPayload.length),
|
||||
- firstPayload);
|
||||
- assertArrayEquals(Arrays.copyOfRange(expectedPayload, firstPayload.length,
|
||||
- expectedPayload.length),
|
||||
- secondPayload);
|
||||
+ assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + fourthPayload.length),
|
||||
+ fourthPayload);
|
||||
}
|
||||
|
||||
@Test
|
||||
@ -0,0 +1,306 @@
|
||||
From c2fdcf7d9785ae386562f37174d8378b96a58c5d Mon Sep 17 00:00:00 2001
|
||||
From: Norman Maurer <norman_maurer@apple.com>
|
||||
Date: Tue, 16 Jan 2024 08:59:49 +0100
|
||||
Subject: [PATCH 1/2] Ensure QuicStreamChannel.shutdownOutput() is only called
|
||||
once all previous writes were processed.
|
||||
|
||||
Motivation:
|
||||
We need to ensure QuicStreamChannel.shutdownOutput() is only called once all previous writes were processed. This is necessary as otherwise shutdownOutput() might be called while some writes are still queued (due flowcontrol).
|
||||
|
||||
Modifications:
|
||||
- Always do the shutdownOutput() via a ChannelFutureListener
|
||||
- Adjust tests
|
||||
|
||||
Result:
|
||||
Always drain write queue first before shutdown the output.
|
||||
---
|
||||
.../http3/Http3FrameToHttpObjectCodec.java | 26 +++---
|
||||
.../Http3FrameToHttpObjectCodecTest.java | 85 ++++++++++++-------
|
||||
2 files changed, 68 insertions(+), 43 deletions(-)
|
||||
|
||||
diff --git a/src/main/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodec.java b/src/main/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodec.java
|
||||
index 5d22891..aa2deb3 100644
|
||||
--- a/src/main/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodec.java
|
||||
+++ b/src/main/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodec.java
|
||||
@@ -50,7 +50,7 @@
|
||||
* and back. It can be used as an adapter in conjunction with {@link
|
||||
* Http3ServerConnectionHandler} or {@link Http3ClientConnectionHandler} to make http/3 connections
|
||||
* backward-compatible with {@link ChannelHandler}s expecting {@link HttpObject}.
|
||||
- *
|
||||
+ * <p>
|
||||
* For simplicity, it converts to chunked encoding unless the entire stream
|
||||
* is a single header.
|
||||
*/
|
||||
@@ -148,7 +148,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
|
||||
return;
|
||||
} else {
|
||||
throw new EncoderException(
|
||||
- HttpResponseStatus.CONTINUE.toString() + " must be a FullHttpResponse");
|
||||
+ HttpResponseStatus.CONTINUE + " must be a FullHttpResponse");
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -187,18 +187,20 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
|
||||
Http3Headers headers = HttpConversionUtil.toHttp3Headers(last.trailingHeaders(), validateHeaders);
|
||||
promise = writeWithOptionalCombiner(ctx,
|
||||
new DefaultHttp3HeadersFrame(headers), promise, combiner, true);
|
||||
- }
|
||||
- if (!readable) {
|
||||
+ } else if (!readable) {
|
||||
+ // Release the data and just use EMPTY_BUFFER. This might allow us to give back memory to the allocator
|
||||
+ // faster.
|
||||
last.release();
|
||||
+ if (combiner == null) {
|
||||
+ // We only need to write something if there was no write before.
|
||||
+ promise = writeWithOptionalCombiner(ctx,
|
||||
+ new DefaultHttp3DataFrame(Unpooled.EMPTY_BUFFER), promise, combiner, true);
|
||||
+ }
|
||||
}
|
||||
-
|
||||
- if (!readable && !hasTrailers && combiner == null) {
|
||||
- // we had to write nothing. happy days!
|
||||
- ((QuicStreamChannel) ctx.channel()).shutdownOutput();
|
||||
- promise.trySuccess();
|
||||
- } else {
|
||||
- promise.addListener(QuicStreamChannel.SHUTDOWN_OUTPUT);
|
||||
- }
|
||||
+ // The shutdown is always done via the listener to ensure previous written data is correctly drained
|
||||
+ // before QuicStreamChannel.shutdownOutput() is called. Missing to do so might cause previous queued data
|
||||
+ // to be failed with a ClosedChannelException.
|
||||
+ promise.addListener(QuicStreamChannel.SHUTDOWN_OUTPUT);
|
||||
} else if (msg instanceof HttpContent) {
|
||||
promise = writeWithOptionalCombiner(ctx,
|
||||
new DefaultHttp3DataFrame(((HttpContent) msg).content()), promise, combiner, false);
|
||||
diff --git a/src/test/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodecTest.java b/src/test/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodecTest.java
|
||||
index 8646bfb..d061f20 100644
|
||||
--- a/src/test/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodecTest.java
|
||||
+++ b/src/test/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodecTest.java
|
||||
@@ -61,6 +61,11 @@
|
||||
import io.netty.incubator.codec.quic.QuicStreamChannel;
|
||||
import io.netty.util.CharsetUtil;
|
||||
import org.junit.jupiter.api.Test;
|
||||
+import org.junit.jupiter.api.extension.ExtensionContext;
|
||||
+import org.junit.jupiter.params.ParameterizedTest;
|
||||
+import org.junit.jupiter.params.provider.Arguments;
|
||||
+import org.junit.jupiter.params.provider.ArgumentsProvider;
|
||||
+import org.junit.jupiter.params.provider.ArgumentsSource;
|
||||
|
||||
import java.nio.CharBuffer;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
@@ -71,6 +76,7 @@
|
||||
import java.util.concurrent.ExecutionException;
|
||||
import java.util.concurrent.LinkedBlockingQueue;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
+import java.util.stream.Stream;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.is;
|
||||
import static org.hamcrest.CoreMatchers.nullValue;
|
||||
@@ -221,6 +227,13 @@ public void testUpgradeEmptyEnd() {
|
||||
ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
|
||||
|
||||
assertTrue(ch.isOutputShutdown());
|
||||
+ Http3DataFrame dataFrame = ch.readOutbound();
|
||||
+ try {
|
||||
+ assertThat(dataFrame.content().readableBytes(), is(0));
|
||||
+ } finally {
|
||||
+ dataFrame.release();
|
||||
+ }
|
||||
+
|
||||
assertFalse(ch.finish());
|
||||
}
|
||||
|
||||
@@ -510,6 +523,13 @@ public void testEncodeEmptyEndAsClient() {
|
||||
ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
|
||||
|
||||
assertTrue(ch.isOutputShutdown());
|
||||
+ Http3DataFrame dataFrame = ch.readOutbound();
|
||||
+ try {
|
||||
+ assertThat(dataFrame.content().readableBytes(), is(0));
|
||||
+ } finally {
|
||||
+ dataFrame.release();
|
||||
+ }
|
||||
+
|
||||
assertFalse(ch.finish());
|
||||
}
|
||||
|
||||
@@ -606,6 +626,13 @@ public void testEncodeEmptyLastPromiseCompletes() {
|
||||
assertThat(headers.path().toString(), is("/hello/world"));
|
||||
assertTrue(ch.isOutputShutdown());
|
||||
|
||||
+ Http3DataFrame dataFrame = ch.readOutbound();
|
||||
+ try {
|
||||
+ assertThat(dataFrame.content().readableBytes(), is(0));
|
||||
+ } finally {
|
||||
+ dataFrame.release();
|
||||
+ }
|
||||
+
|
||||
assertFalse(ch.finish());
|
||||
}
|
||||
|
||||
@@ -684,31 +711,30 @@ public void testEncodeVoidPromise() {
|
||||
assertFalse(ch.finish());
|
||||
}
|
||||
|
||||
- @Test
|
||||
- public void testEncodeCombinations() {
|
||||
- // this test goes through all the branches of Http3FrameToHttpObjectCodec and ensures right functionality
|
||||
-
|
||||
- for (boolean headers : new boolean[]{false, true}) {
|
||||
- for (boolean last : new boolean[]{false, true}) {
|
||||
- for (boolean nonEmptyContent : new boolean[]{false, true}) {
|
||||
- for (boolean hasTrailers : new boolean[]{false, true}) {
|
||||
- for (boolean voidPromise : new boolean[]{false, true}) {
|
||||
- testEncodeCombination(headers, last, nonEmptyContent, hasTrailers, voidPromise);
|
||||
+ private static final class EncodeCombinationsArgumentsProvider implements ArgumentsProvider {
|
||||
+ @Override
|
||||
+ public Stream<? extends Arguments> provideArguments(ExtensionContext extensionContext) {
|
||||
+ List<Arguments> arguments = new ArrayList<>();
|
||||
+ for (boolean headers : new boolean[]{false, true}) {
|
||||
+ for (boolean last : new boolean[]{false, true}) {
|
||||
+ for (boolean nonEmptyContent : new boolean[]{false, true}) {
|
||||
+ for (boolean hasTrailers : new boolean[]{false, true}) {
|
||||
+ for (boolean voidPromise : new boolean[]{false, true}) {
|
||||
+ // this test goes through all the branches of Http3FrameToHttpObjectCodec
|
||||
+ // and ensures right functionality
|
||||
+ arguments.add(Arguments.of(headers, last, nonEmptyContent, hasTrailers, voidPromise));
|
||||
+ }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
+ return arguments.stream();
|
||||
}
|
||||
}
|
||||
|
||||
- /**
|
||||
- * @param headers Should this be an initial message, with headers ({@link HttpRequest})?
|
||||
- * @param last Should this be a last message ({@link LastHttpContent})?
|
||||
- * @param nonEmptyContent Should this message have non-empty content?
|
||||
- * @param hasTrailers Should this {@code last} message have trailers?
|
||||
- * @param voidPromise Should the write operation use a void promise?
|
||||
- */
|
||||
- private static void testEncodeCombination(
|
||||
+ @ParameterizedTest(name = "headers: {0}, last: {1}, nonEmptyContent: {2}, hasTrailers: {3}, voidPromise: {4}")
|
||||
+ @ArgumentsSource(value = EncodeCombinationsArgumentsProvider.class)
|
||||
+ public void testEncodeCombination(
|
||||
boolean headers,
|
||||
boolean last,
|
||||
boolean nonEmptyContent,
|
||||
@@ -772,31 +798,28 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
|
||||
Http3DataFrame dataFrame = ch.readOutbound();
|
||||
assertThat(dataFrame.content().readableBytes(), is(1));
|
||||
dataFrame.release();
|
||||
- } else if (!headers && !hasTrailers && !last) {
|
||||
- ch.<Http3DataFrame>readOutbound().release();
|
||||
}
|
||||
if (hasTrailers) {
|
||||
Http3HeadersFrame trailersFrame = ch.readOutbound();
|
||||
assertThat(trailersFrame.headers().get("foo"), is("bar"));
|
||||
+ } else if (!nonEmptyContent && !headers) {
|
||||
+ Http3DataFrame dataFrame = ch.readOutbound();
|
||||
+ assertThat(dataFrame.content().readableBytes(), is(0));
|
||||
+ dataFrame.release();
|
||||
}
|
||||
- // empty LastHttpContent has no data written and will complete the promise immediately
|
||||
- boolean anyData = hasTrailers || nonEmptyContent || headers || !last;
|
||||
+
|
||||
if (!voidPromise) {
|
||||
- if (anyData) {
|
||||
- assertFalse(fullPromise.isDone());
|
||||
- } else {
|
||||
- // nothing to write, immediately complete
|
||||
- assertTrue(fullPromise.isDone());
|
||||
- }
|
||||
- }
|
||||
- if (!last || anyData) {
|
||||
- assertFalse(ch.isOutputShutdown());
|
||||
+ assertFalse(fullPromise.isDone());
|
||||
}
|
||||
+
|
||||
+ assertFalse(ch.isOutputShutdown());
|
||||
for (ChannelPromise framePromise : framePromises) {
|
||||
framePromise.trySuccess();
|
||||
}
|
||||
if (last) {
|
||||
assertTrue(ch.isOutputShutdown());
|
||||
+ } else {
|
||||
+ assertFalse(ch.isOutputShutdown());
|
||||
}
|
||||
if (!voidPromise) {
|
||||
assertTrue(fullPromise.isDone());
|
||||
|
||||
From edb7763561589ff53a20df848270cb5144b70f95 Mon Sep 17 00:00:00 2001
|
||||
From: Norman Maurer <norman_maurer@apple.com>
|
||||
Date: Tue, 16 Jan 2024 10:20:32 +0100
|
||||
Subject: [PATCH 2/2] Always correctly release
|
||||
|
||||
---
|
||||
.../http3/Http3FrameToHttpObjectCodec.java | 50 ++++++++++---------
|
||||
1 file changed, 26 insertions(+), 24 deletions(-)
|
||||
|
||||
diff --git a/src/main/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodec.java b/src/main/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodec.java
|
||||
index aa2deb3..c7b5058 100644
|
||||
--- a/src/main/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodec.java
|
||||
+++ b/src/main/java/io/netty/incubator/codec/http3/Http3FrameToHttpObjectCodec.java
|
||||
@@ -172,35 +172,37 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
|
||||
|
||||
if (isLast) {
|
||||
LastHttpContent last = (LastHttpContent) msg;
|
||||
- boolean readable = last.content().isReadable();
|
||||
- boolean hasTrailers = !last.trailingHeaders().isEmpty();
|
||||
+ try {
|
||||
+ boolean readable = last.content().isReadable();
|
||||
+ boolean hasTrailers = !last.trailingHeaders().isEmpty();
|
||||
|
||||
- if (combiner == null && readable && hasTrailers && !promise.isVoid()) {
|
||||
- combiner = new PromiseCombiner(ctx.executor());
|
||||
- }
|
||||
+ if (combiner == null && readable && hasTrailers && !promise.isVoid()) {
|
||||
+ combiner = new PromiseCombiner(ctx.executor());
|
||||
+ }
|
||||
|
||||
- if (readable) {
|
||||
- promise = writeWithOptionalCombiner(ctx,
|
||||
- new DefaultHttp3DataFrame(last.content()), promise, combiner, true);
|
||||
- }
|
||||
- if (hasTrailers) {
|
||||
- Http3Headers headers = HttpConversionUtil.toHttp3Headers(last.trailingHeaders(), validateHeaders);
|
||||
- promise = writeWithOptionalCombiner(ctx,
|
||||
- new DefaultHttp3HeadersFrame(headers), promise, combiner, true);
|
||||
- } else if (!readable) {
|
||||
- // Release the data and just use EMPTY_BUFFER. This might allow us to give back memory to the allocator
|
||||
- // faster.
|
||||
- last.release();
|
||||
- if (combiner == null) {
|
||||
- // We only need to write something if there was no write before.
|
||||
+ if (readable) {
|
||||
+ promise = writeWithOptionalCombiner(
|
||||
+ ctx, new DefaultHttp3DataFrame(last.content().retain()), promise, combiner, true);
|
||||
+ }
|
||||
+ if (hasTrailers) {
|
||||
+ Http3Headers headers = HttpConversionUtil.toHttp3Headers(last.trailingHeaders(), validateHeaders);
|
||||
promise = writeWithOptionalCombiner(ctx,
|
||||
- new DefaultHttp3DataFrame(Unpooled.EMPTY_BUFFER), promise, combiner, true);
|
||||
+ new DefaultHttp3HeadersFrame(headers), promise, combiner, true);
|
||||
+ } else if (!readable) {
|
||||
+ if (combiner == null) {
|
||||
+ // We only need to write something if there was no write before.
|
||||
+ promise = writeWithOptionalCombiner(
|
||||
+ ctx, new DefaultHttp3DataFrame(last.content().retain()), promise, combiner, true);
|
||||
+ }
|
||||
}
|
||||
+ // The shutdown is always done via the listener to ensure previous written data is correctly drained
|
||||
+ // before QuicStreamChannel.shutdownOutput() is called. Missing to do so might cause previous queued
|
||||
+ // data to be failed with a ClosedChannelException.
|
||||
+ promise = promise.unvoid().addListener(QuicStreamChannel.SHUTDOWN_OUTPUT);
|
||||
+ } finally {
|
||||
+ // Release LastHttpContent, we retain the content if we need it.
|
||||
+ last.release();
|
||||
}
|
||||
- // The shutdown is always done via the listener to ensure previous written data is correctly drained
|
||||
- // before QuicStreamChannel.shutdownOutput() is called. Missing to do so might cause previous queued data
|
||||
- // to be failed with a ClosedChannelException.
|
||||
- promise.addListener(QuicStreamChannel.SHUTDOWN_OUTPUT);
|
||||
} else if (msg instanceof HttpContent) {
|
||||
promise = writeWithOptionalCombiner(ctx,
|
||||
new DefaultHttp3DataFrame(((HttpContent) msg).content()), promise, combiner, false);
|
||||
Loading…
Reference in New Issue