You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2025/01/14 19:22:52 UTC
(httpcomponents-core) 01/01: HTTPCORE-775: The SSLIOSession::write does not handle SSLEngineResult#BUFFER_OVERFLOW
This is an automated email from the ASF dual-hosted git repository.
olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git
commit a4ae27d404346c393cd5139128aee081e72ed167
Author: Andriy Redko <dr...@gmail.com>
AuthorDate: Sun Jan 12 15:31:29 2025 -0500
HTTPCORE-775: The SSLIOSession::write does not handle SSLEngineResult#BUFFER_OVERFLOW
Signed-off-by: Andriy Redko <dr...@gmail.com>
---
.../apache/hc/core5/testing/SSLTestContexts.java | 11 ++++-
.../nio/H2CoreTransportMultiplexingTest.java | 24 ++++++++++
.../hc/core5/testing/nio/H2CoreTransportTest.java | 8 +++-
.../core5/testing/nio/Http1CoreTransportTest.java | 8 +++-
.../core5/testing/nio/HttpCoreTransportTest.java | 24 ++++++++++
.../hc/core5/testing/nio/HttpIntegrationTests.java | 20 +++++++++
.../apache/hc/core5/reactor/ssl/SSLIOSession.java | 22 +++++++---
.../hc/core5/reactor/ssl/SSLManagedBuffer.java | 51 +++++++++++++++++++++-
8 files changed, 156 insertions(+), 12 deletions(-)
diff --git a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/SSLTestContexts.java b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/SSLTestContexts.java
index 9cbec8048..785392785 100644
--- a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/SSLTestContexts.java
+++ b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/SSLTestContexts.java
@@ -40,8 +40,11 @@ import javax.net.ssl.SSLContext;
import org.apache.hc.core5.ssl.SSLContextBuilder;
public final class SSLTestContexts {
-
public static SSLContext createServerSSLContext() {
+ return createServerSSLContext(null);
+ }
+
+ public static SSLContext createServerSSLContext(final String protocol) {
final URL keyStoreURL = SSLTestContexts.class.getResource("/test.p12");
final String storePassword = "nopassword";
try {
@@ -49,6 +52,7 @@ public final class SSLTestContexts {
.setKeyStoreType("pkcs12")
.loadTrustMaterial(keyStoreURL, storePassword.toCharArray())
.loadKeyMaterial(keyStoreURL, storePassword.toCharArray(), storePassword.toCharArray())
+ .setProtocol(protocol)
.build();
} catch (final NoSuchAlgorithmException | KeyManagementException | KeyStoreException | CertificateException |
UnrecoverableKeyException | IOException ex) {
@@ -57,12 +61,17 @@ public final class SSLTestContexts {
}
public static SSLContext createClientSSLContext() {
+ return createClientSSLContext(null);
+ }
+
+ public static SSLContext createClientSSLContext(final String protocol) {
final URL keyStoreURL = SSLTestContexts.class.getResource("/test.p12");
final String storePassword = "nopassword";
try {
return SSLContextBuilder.create()
.setKeyStoreType("pkcs12")
.loadTrustMaterial(keyStoreURL, storePassword.toCharArray())
+ .setProtocol(protocol)
.build();
} catch (final NoSuchAlgorithmException | KeyManagementException | KeyStoreException | CertificateException |
IOException ex) {
diff --git a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2CoreTransportMultiplexingTest.java b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2CoreTransportMultiplexingTest.java
index 3a8ff8028..408ff8d7e 100644
--- a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2CoreTransportMultiplexingTest.java
+++ b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2CoreTransportMultiplexingTest.java
@@ -34,6 +34,8 @@ import java.util.LinkedList;
import java.util.Queue;
import java.util.Random;
import java.util.concurrent.Future;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
import org.apache.hc.core5.concurrent.Cancellable;
import org.apache.hc.core5.concurrent.CountDownLatchFutureCallback;
@@ -48,6 +50,7 @@ import org.apache.hc.core5.http.URIScheme;
import org.apache.hc.core5.http.impl.bootstrap.HttpAsyncServer;
import org.apache.hc.core5.http.impl.routing.RequestRouter;
import org.apache.hc.core5.http.nio.AsyncServerExchangeHandler;
+import org.apache.hc.core5.http.nio.entity.AsyncEntityProducers;
import org.apache.hc.core5.http.nio.entity.StringAsyncEntityConsumer;
import org.apache.hc.core5.http.nio.entity.StringAsyncEntityProducer;
import org.apache.hc.core5.http.nio.support.BasicClientExchangeHandler;
@@ -146,6 +149,27 @@ abstract class H2CoreTransportMultiplexingTest {
assertThat(body3, CoreMatchers.equalTo("some more stuff"));
}
+ @Test
+ void testLargeRequest() throws Exception {
+ final HttpAsyncServer server = serverResource.start();
+ final Future<ListenerEndpoint> future = server.listen(new InetSocketAddress(0), scheme);
+ final ListenerEndpoint listener = future.get();
+ final InetSocketAddress address = (InetSocketAddress) listener.getAddress();
+ final H2MultiplexingRequester requester = clientResource.start();
+
+ final HttpHost target = new HttpHost(scheme.id, "localhost", address.getPort());
+ final String content = IntStream.range(0, 1000).mapToObj(i -> "a lot of stuff").collect(Collectors.joining(" "));
+ final Future<Message<HttpResponse, String>> resultFuture = requester.execute(
+ new BasicRequestProducer(Method.POST, target, "/a-lot-of-stuff", AsyncEntityProducers.create(content, ContentType.TEXT_PLAIN)),
+ new BasicResponseConsumer<>(new StringAsyncEntityConsumer()), TIMEOUT, null);
+ final Message<HttpResponse, String> message = resultFuture.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit());
+ assertThat(message, CoreMatchers.notNullValue());
+ final HttpResponse response = message.getHead();
+ assertThat(response.getCode(), CoreMatchers.equalTo(HttpStatus.SC_OK));
+ final String body = message.getBody();
+ assertThat(body, CoreMatchers.equalTo(content));
+ }
+
@Test
void testMultiplexedRequests() throws Exception {
final HttpAsyncServer server = serverResource.start();
diff --git a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2CoreTransportTest.java b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2CoreTransportTest.java
index 2738936c2..c683e9524 100644
--- a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2CoreTransportTest.java
+++ b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2CoreTransportTest.java
@@ -55,11 +55,15 @@ abstract class H2CoreTransportTest extends HttpCoreTransportTest {
private final H2AsyncRequesterResource clientResource;
public H2CoreTransportTest(final URIScheme scheme) {
+ this(scheme, null);
+ }
+
+ public H2CoreTransportTest(final URIScheme scheme, final String tlsProtocol) {
super(scheme);
this.serverResource = new H2AsyncServerResource();
this.serverResource.configure(bootstrap -> bootstrap
.setVersionPolicy(HttpVersionPolicy.NEGOTIATE)
- .setTlsStrategy(new H2ServerTlsStrategy(SSLTestContexts.createServerSSLContext()))
+ .setTlsStrategy(new H2ServerTlsStrategy(SSLTestContexts.createServerSSLContext(tlsProtocol)))
.setIOReactorConfig(
IOReactorConfig.custom()
.setSoTimeout(TIMEOUT)
@@ -72,7 +76,7 @@ abstract class H2CoreTransportTest extends HttpCoreTransportTest {
this.clientResource = new H2AsyncRequesterResource();
this.clientResource.configure(bootstrap -> bootstrap
.setVersionPolicy(HttpVersionPolicy.NEGOTIATE)
- .setTlsStrategy(new H2ClientTlsStrategy(SSLTestContexts.createClientSSLContext()))
+ .setTlsStrategy(new H2ClientTlsStrategy(SSLTestContexts.createClientSSLContext(tlsProtocol)))
.setIOReactorConfig(IOReactorConfig.custom()
.setSoTimeout(TIMEOUT)
.build())
diff --git a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/Http1CoreTransportTest.java b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/Http1CoreTransportTest.java
index 22376331b..5dca585de 100644
--- a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/Http1CoreTransportTest.java
+++ b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/Http1CoreTransportTest.java
@@ -79,10 +79,14 @@ abstract class Http1CoreTransportTest extends HttpCoreTransportTest {
private final HttpAsyncRequesterResource clientResource;
public Http1CoreTransportTest(final URIScheme scheme) {
+ this(scheme, null);
+ }
+
+ public Http1CoreTransportTest(final URIScheme scheme, final String tlsProtocol) {
super(scheme);
this.serverResource = new HttpAsyncServerResource();
this.serverResource.configure(bootstrap -> bootstrap
- .setTlsStrategy(new H2ServerTlsStrategy(SSLTestContexts.createServerSSLContext()))
+ .setTlsStrategy(new H2ServerTlsStrategy(SSLTestContexts.createServerSSLContext(tlsProtocol)))
.setIOReactorConfig(
IOReactorConfig.custom()
.setSoTimeout(TIMEOUT)
@@ -121,7 +125,7 @@ abstract class Http1CoreTransportTest extends HttpCoreTransportTest {
);
this.clientResource = new HttpAsyncRequesterResource();
this.clientResource.configure(bootstrap -> bootstrap
- .setTlsStrategy(new H2ClientTlsStrategy(SSLTestContexts.createClientSSLContext()))
+ .setTlsStrategy(new H2ClientTlsStrategy(SSLTestContexts.createClientSSLContext(tlsProtocol)))
.setIOReactorConfig(IOReactorConfig.custom()
.setSoTimeout(TIMEOUT)
.build())
diff --git a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpCoreTransportTest.java b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpCoreTransportTest.java
index a870ec209..a9049b2e9 100644
--- a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpCoreTransportTest.java
+++ b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpCoreTransportTest.java
@@ -34,6 +34,8 @@ import java.net.InetSocketAddress;
import java.util.LinkedList;
import java.util.Queue;
import java.util.concurrent.Future;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.HttpHost;
@@ -47,6 +49,7 @@ import org.apache.hc.core5.http.impl.bootstrap.HttpAsyncRequester;
import org.apache.hc.core5.http.impl.bootstrap.HttpAsyncServer;
import org.apache.hc.core5.http.message.BasicHttpRequest;
import org.apache.hc.core5.http.nio.AsyncClientEndpoint;
+import org.apache.hc.core5.http.nio.entity.AsyncEntityProducers;
import org.apache.hc.core5.http.nio.entity.StringAsyncEntityConsumer;
import org.apache.hc.core5.http.nio.entity.StringAsyncEntityProducer;
import org.apache.hc.core5.http.nio.support.BasicRequestProducer;
@@ -113,6 +116,27 @@ abstract class HttpCoreTransportTest {
assertThat(body3, CoreMatchers.equalTo("some more stuff"));
}
+ @Test
+ void testLargeRequest() throws Exception {
+ final HttpAsyncServer server = serverStart();
+ final Future<ListenerEndpoint> future = server.listen(new InetSocketAddress(0), scheme);
+ final ListenerEndpoint listener = future.get();
+ final InetSocketAddress address = (InetSocketAddress) listener.getAddress();
+ final HttpAsyncRequester requester = clientStart();
+
+ final HttpHost target = new HttpHost(scheme.id, "localhost", address.getPort());
+ final String content = IntStream.range(0, 1000).mapToObj(i -> "a lot of stuff").collect(Collectors.joining(" "));
+ final Future<Message<HttpResponse, String>> resultFuture = requester.execute(
+ new BasicRequestProducer(Method.POST, target, "/a-lot-of-stuff", AsyncEntityProducers.create(content, ContentType.TEXT_PLAIN)),
+ new BasicResponseConsumer<>(new StringAsyncEntityConsumer()), TIMEOUT, null);
+ final Message<HttpResponse, String> message = resultFuture.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit());
+ assertThat(message, CoreMatchers.notNullValue());
+ final HttpResponse response = message.getHead();
+ assertThat(response.getCode(), CoreMatchers.equalTo(HttpStatus.SC_OK));
+ final String body = message.getBody();
+ assertThat(body, CoreMatchers.equalTo(content));
+ }
+
@Test
void testSequentialRequestsNonPersistentConnection() throws Exception {
final HttpAsyncServer server = serverStart();
diff --git a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpIntegrationTests.java b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpIntegrationTests.java
index a81e8749e..aaff7e275 100644
--- a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpIntegrationTests.java
+++ b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpIntegrationTests.java
@@ -53,6 +53,16 @@ class HttpIntegrationTests {
}
+ @Nested
+ @DisplayName("Core transport (HTTP/1.1, TLSv1.3)")
+ class CoreTransportTls13 extends Http1CoreTransportTest {
+
+ public CoreTransportTls13() {
+ super(URIScheme.HTTPS, "TLSv1.3");
+ }
+
+ }
+
@Nested
@DisplayName("Core transport (H2)")
class CoreTransportH2 extends H2CoreTransportTest {
@@ -73,6 +83,16 @@ class HttpIntegrationTests {
}
+ @Nested
+ @DisplayName("Core transport (H2, TLSv1.3)")
+ class CoreTransportH2Tls13 extends H2CoreTransportTest {
+
+ public CoreTransportH2Tls13() {
+ super(URIScheme.HTTPS, "TLSv1.3");
+ }
+
+ }
+
@Nested
@DisplayName("Core transport (H2, multiplexing)")
class CoreTransportH2Multiplexing extends H2CoreTransportMultiplexingTest {
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
index b636c0e19..36755e7d7 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
@@ -93,6 +93,7 @@ public class SSLIOSession implements IOSession {
private final AtomicInteger outboundClosedCount;
private final AtomicReference<TLSHandShakeState> handshakeStateRef;
private final IOEventHandler internalEventHandler;
+ private final int packetBufferSize;
private int appEventMask;
@@ -178,9 +179,9 @@ public class SSLIOSession implements IOSession {
final SSLSession sslSession = this.sslEngine.getSession();
// Allocate buffers for network (encrypted) data
- final int netBufferSize = sslSession.getPacketBufferSize();
- this.inEncrypted = SSLManagedBuffer.create(sslBufferMode, netBufferSize);
- this.outEncrypted = SSLManagedBuffer.create(sslBufferMode, netBufferSize);
+ this.packetBufferSize = sslSession.getPacketBufferSize();
+ this.inEncrypted = SSLManagedBuffer.create(sslBufferMode, packetBufferSize);
+ this.outEncrypted = SSLManagedBuffer.create(sslBufferMode, packetBufferSize);
// Allocate buffers for application (unencrypted) data
final int appBufferSize = sslSession.getApplicationBufferSize();
@@ -668,9 +669,18 @@ public class SSLIOSession implements IOSession {
if (this.handshakeStateRef.get() == TLSHandShakeState.READY) {
return 0;
}
- final ByteBuffer outEncryptedBuf = this.outEncrypted.acquire();
- final SSLEngineResult result = doWrap(src, outEncryptedBuf);
- return result.bytesConsumed();
+
+ for (;;) {
+ final ByteBuffer outEncryptedBuf = this.outEncrypted.acquire();
+ final SSLEngineResult result = doWrap(src, outEncryptedBuf);
+ if (result.getStatus() == SSLEngineResult.Status.BUFFER_OVERFLOW) {
+ // We don't release the buffer here, it will be expanded (if needed)
+ // and returned by the next attempt of SSLManagedBuffer#acquire() call.
+ this.outEncrypted.ensureWriteable(packetBufferSize);
+ } else {
+ return result.bytesConsumed();
+ }
+ }
} finally {
this.session.getLock().unlock();
}
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLManagedBuffer.java b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLManagedBuffer.java
index e3d21738a..1a184a7b8 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLManagedBuffer.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLManagedBuffer.java
@@ -57,13 +57,54 @@ abstract class SSLManagedBuffer {
*/
abstract boolean hasData();
+ /**
+ * Expands the underlying buffer's to make sure it has enough write capacity to accommodate
+ * the required amount of bytes. This method has no side effect if the buffer has enough writeable
+ * capacity left.
+ * @param size the required write capacity
+ */
+ abstract void ensureWriteable(final int size);
+
+ /**
+ * Helper method to ensure additional writeable capacity with respect to the source buffer. It
+ * allocates a new buffer and copies all the data if needed, returning the new buffer. This method
+ * has no side effect if the source buffer has enough writeable capacity left.
+ * @param src source buffer
+ * @param size the required write capacity
+ * @return new buffer (or the source buffer of it has enough writeable capacity left)
+ */
+ ByteBuffer ensureWriteable(final ByteBuffer src, final int size) {
+ if (src == null) {
+ // Nothing to do, the buffer is not allocated
+ return null;
+ }
+
+ // There is not enough capacity left, we need to expand
+ if (src.remaining() < size) {
+ final int additionalCapacityNeeded = size - src.remaining();
+ final ByteBuffer expanded = ByteBuffer.allocate(src.capacity() + additionalCapacityNeeded);
+
+ // use a duplicated buffer so we don't disrupt the limit of the original buffer
+ final ByteBuffer tmp = src.duplicate();
+ tmp.flip();
+
+ // Copy to expanded buffer
+ expanded.put(tmp);
+
+ // Use a new buffer
+ return expanded;
+ } else {
+ return src;
+ }
+ }
+
static SSLManagedBuffer create(final SSLBufferMode mode, final int size) {
return mode == SSLBufferMode.DYNAMIC ? new DynamicBuffer(size) : new StaticBuffer(size);
}
static final class StaticBuffer extends SSLManagedBuffer {
- private final ByteBuffer buffer;
+ private ByteBuffer buffer;
public StaticBuffer(final int size) {
Args.positive(size, "size");
@@ -90,6 +131,10 @@ abstract class SSLManagedBuffer {
return buffer.position() > 0;
}
+ @Override
+ void ensureWriteable(final int size) {
+ buffer = ensureWriteable(buffer, size);
+ }
}
static final class DynamicBuffer extends SSLManagedBuffer {
@@ -126,6 +171,10 @@ abstract class SSLManagedBuffer {
return wrapped != null && wrapped.position() > 0;
}
+ @Override
+ void ensureWriteable(final int size) {
+ wrapped = ensureWriteable(wrapped, size);
+ }
}
}