diff --git a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/EventSourceConfig.java b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/EventSourceConfig.java
index f8a8700a9d..78daf209b2 100644
--- a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/EventSourceConfig.java
+++ b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/EventSourceConfig.java
@@ -29,6 +29,7 @@
import java.util.Objects;
import org.apache.hc.client5.http.sse.impl.ExponentialJitterBackoff;
+import org.apache.hc.core5.util.Args;
/**
* Immutable configuration for {@link EventSource} behavior, primarily covering
@@ -43,6 +44,8 @@
* The strategy can also honor server hints (SSE {@code retry:} field or HTTP {@code Retry-After}).
*
{@link #maxReconnects}: maximum number of reconnect attempts before giving up.
* A value of {@code -1} means unlimited attempts.
+ * {@link #maxLineLength}: maximum length of a single line of the stream.
+ * {@link #maxEventSize}: maximum size of the data accumulated for a single event.
*
*
* Thread-safety
@@ -74,20 +77,55 @@ public final class EventSourceConfig {
*/
public final int maxReconnects;
+ /**
+ * Default value of {@link #maxLineLength}, 1 MiB.
+ *
+ * @since 5.7
+ */
+ public static final int DEFAULT_MAX_LINE_LENGTH = 1024 * 1024;
+
+ /**
+ * Default value of {@link #maxEventSize}, 8 MiB.
+ *
+ * @since 5.7
+ */
+ public static final int DEFAULT_MAX_EVENT_SIZE = 8 * 1024 * 1024;
+
+ /**
+ * Maximum length of a single line, excluding the line terminator. Counted in characters
+ * by the char parser and in bytes by the byte parser. A longer line fails the stream.
+ *
+ * @since 5.7
+ */
+ public final int maxLineLength;
+
+ /**
+ * Maximum size of the {@code data:} content accumulated for one event before the
+ * terminating blank line. Counted in characters. A larger event fails the stream.
+ *
+ * @since 5.7
+ */
+ public final int maxEventSize;
+
/**
* Default configuration:
*
* - {@link #backoff} = {@code new ExponentialJitterBackoff(1000, 30000, 2.0, 250)}
* - {@link #maxReconnects} = {@code -1} (unlimited)
+ * - {@link #maxLineLength} = 1 MiB
+ * - {@link #maxEventSize} = 8 MiB
*
*
* @since 5.7
*/
public static final EventSourceConfig DEFAULT = builder().build();
- private EventSourceConfig(final BackoffStrategy backoff, final int maxReconnects) {
+ private EventSourceConfig(final BackoffStrategy backoff, final int maxReconnects,
+ final int maxLineLength, final int maxEventSize) {
this.backoff = backoff;
this.maxReconnects = maxReconnects;
+ this.maxLineLength = maxLineLength;
+ this.maxEventSize = maxEventSize;
}
/**
@@ -111,6 +149,8 @@ public static final class Builder {
private BackoffStrategy backoff =
new ExponentialJitterBackoff(1_000L, 30_000L, 2.0, 250L);
private int maxReconnects = -1;
+ private int maxLineLength = DEFAULT_MAX_LINE_LENGTH;
+ private int maxEventSize = DEFAULT_MAX_EVENT_SIZE;
/**
* Sets the reconnect/backoff strategy.
@@ -139,6 +179,30 @@ public Builder maxReconnects(final int nmaxReconnects) {
return this;
}
+ /**
+ * Sets the maximum length of a single line.
+ *
+ * @param maxLineLength maximum line length, must be positive
+ * @return this builder
+ * @since 5.7
+ */
+ public Builder maxLineLength(final int maxLineLength) {
+ this.maxLineLength = Args.positive(maxLineLength, "Max line length");
+ return this;
+ }
+
+ /**
+ * Sets the maximum size of the data accumulated for a single event.
+ *
+ * @param maxEventSize maximum event size, must be positive
+ * @return this builder
+ * @since 5.7
+ */
+ public Builder maxEventSize(final int maxEventSize) {
+ this.maxEventSize = Args.positive(maxEventSize, "Max event size");
+ return this;
+ }
+
/**
* Builds an immutable {@link EventSourceConfig}.
*
@@ -146,7 +210,7 @@ public Builder maxReconnects(final int nmaxReconnects) {
* @since 5.7
*/
public EventSourceConfig build() {
- return new EventSourceConfig(backoff, maxReconnects);
+ return new EventSourceConfig(backoff, maxReconnects, maxLineLength, maxEventSize);
}
}
}
diff --git a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/ByteSseEntityConsumer.java b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/ByteSseEntityConsumer.java
index d346c84446..e415df2edc 100644
--- a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/ByteSseEntityConsumer.java
+++ b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/ByteSseEntityConsumer.java
@@ -30,10 +30,13 @@
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
+import org.apache.hc.client5.http.sse.EventSourceConfig;
import org.apache.hc.core5.annotation.Internal;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.HttpException;
+import org.apache.hc.core5.http.MessageConstraintException;
import org.apache.hc.core5.http.nio.entity.AbstractBinAsyncEntityConsumer;
+import org.apache.hc.core5.util.Args;
/**
* Low-allocation binary consumer for Server-Sent Events (SSE).
@@ -52,6 +55,8 @@
* Implements WHATWG SSE fields: {@code data}, {@code id}, {@code event}, {@code retry}.
* Unknown fields and malformed {@code retry} values are ignored.
* At end of stream, discards any incomplete line or event.
+ * Fails the stream with {@link MessageConstraintException} if a line exceeds the maximum
+ * line length in bytes or an event accumulates more than the maximum event size.
*
*
* Thread-safety
@@ -71,6 +76,8 @@ public final class ByteSseEntityConsumer extends AbstractBinAsyncEntityConsumer<
private static final byte SPACE = (byte) ' ';
private final SseCallbacks cb;
+ private final int maxLineLength;
+ private final int maxEventSize;
// line accumulator
private byte[] lineBuf = new byte[256];
@@ -88,7 +95,13 @@ public final class ByteSseEntityConsumer extends AbstractBinAsyncEntityConsumer<
private boolean bomDone = false; // once true, no further BOM detection
public ByteSseEntityConsumer(final SseCallbacks callbacks) {
+ this(callbacks, EventSourceConfig.DEFAULT_MAX_LINE_LENGTH, EventSourceConfig.DEFAULT_MAX_EVENT_SIZE);
+ }
+
+ public ByteSseEntityConsumer(final SseCallbacks callbacks, final int maxLineLength, final int maxEventSize) {
this.cb = callbacks;
+ this.maxLineLength = Args.positive(maxLineLength, "Max line length");
+ this.maxEventSize = Args.positive(maxEventSize, "Max event size");
}
@Override
@@ -101,7 +114,16 @@ public void streamStart(final ContentType contentType) throws HttpException, IOE
}
@Override
- protected void data(final ByteBuffer src, final boolean endOfStream) {
+ protected void data(final ByteBuffer src, final boolean endOfStream) throws IOException {
+ try {
+ consumeBytes(src, endOfStream);
+ } catch (final MessageConstraintException ex) {
+ releaseResources();
+ throw ex;
+ }
+ }
+
+ private void consumeBytes(final ByteBuffer src, final boolean endOfStream) throws IOException {
if (!bomDone) {
while (src.hasRemaining() && bomMatched < 3) {
final int expected = (bomMatched == 0) ? 0xEF : (bomMatched == 1 ? 0xBB : 0xBF);
@@ -142,7 +164,7 @@ protected void data(final ByteBuffer src, final boolean endOfStream) {
}
}
- private void processByte(final byte b) {
+ private void processByte(final byte b) throws IOException {
if (b == LF) {
if (pendingCr) {
// LF completing a CRLF pair; the line was already emitted on the CR.
@@ -168,7 +190,10 @@ private void flushEndOfStream() {
type = null;
}
- private void appendByte(final byte b) {
+ private void appendByte(final byte b) throws MessageConstraintException {
+ if (lineLen >= maxLineLength) {
+ throw new MessageConstraintException("Maximum line length exceeded: " + maxLineLength);
+ }
ensureCapacity(lineLen + 1);
lineBuf[lineLen++] = b;
}
@@ -189,13 +214,14 @@ public void releaseResources() {
lineLen = 0;
pendingCr = false;
data.setLength(0);
+ data.trimToSize();
id = null;
type = null;
bomMatched = 0;
bomDone = false;
}
- private void handleLine(final byte[] buf, final int len) {
+ private void handleLine(final byte[] buf, final int len) throws IOException {
if (len == 0) {
dispatch();
return;
@@ -222,6 +248,9 @@ private void handleLine(final byte[] buf, final int len) {
// Compare ASCII field name without allocations
if (fLen == 4 && buf[0] == 'd' && buf[1] == 'a' && buf[2] == 't' && buf[3] == 'a') {
final String v = new String(buf, vStart, len - vStart, StandardCharsets.UTF_8);
+ if ((long) data.length() + v.length() + 1 > maxEventSize) {
+ throw new MessageConstraintException("Maximum event size exceeded: " + maxEventSize);
+ }
data.append(v).append('\n');
} else if (fLen == 5 && buf[0] == 'e' && buf[1] == 'v' && buf[2] == 'e' && buf[3] == 'n' && buf[4] == 't') {
type = new String(buf, vStart, len - vStart, StandardCharsets.UTF_8);
diff --git a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/DefaultEventSource.java b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/DefaultEventSource.java
index 4a1f8b049a..a6152519dd 100644
--- a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/DefaultEventSource.java
+++ b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/DefaultEventSource.java
@@ -119,6 +119,8 @@ public final class DefaultEventSource implements EventSource {
private final Executor callbackExecutor;
private final BackoffStrategy backoff;
private final int maxReconnects;
+ private final int maxLineLength;
+ private final int maxEventSize;
private final SseParser parser;
private final AtomicBoolean started = new AtomicBoolean(false);
@@ -195,6 +197,8 @@ public DefaultEventSource(final CloseableHttpAsyncClient client,
final EventSourceConfig cfg = (config != null) ? config : EventSourceConfig.DEFAULT;
this.backoff = cfg.backoff;
this.maxReconnects = cfg.maxReconnects;
+ this.maxLineLength = cfg.maxLineLength;
+ this.maxEventSize = cfg.maxEventSize;
this.parser = parser != null ? parser : SseParser.CHAR;
}
@@ -414,8 +418,8 @@ public void onRetry(final long retryMs) {
};
final AsyncEntityConsumer entity =
- (parser == SseParser.BYTE) ? new ByteSseEntityConsumer(cbs)
- : new SseEntityConsumer(cbs);
+ (parser == SseParser.BYTE) ? new ByteSseEntityConsumer(cbs, maxLineLength, maxEventSize)
+ : new SseEntityConsumer(cbs, maxLineLength, maxEventSize);
return new SseResponseConsumer(entity, ms -> retryAfterHintMs = Math.max(0L, ms));
}
diff --git a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/ServerSentEventReader.java b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/ServerSentEventReader.java
index 3dd7b1ebb4..8f096c9a36 100644
--- a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/ServerSentEventReader.java
+++ b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/ServerSentEventReader.java
@@ -26,7 +26,12 @@
*/
package org.apache.hc.client5.http.sse.impl;
+import java.io.IOException;
+
+import org.apache.hc.client5.http.sse.EventSourceConfig;
import org.apache.hc.core5.annotation.Internal;
+import org.apache.hc.core5.http.MessageConstraintException;
+import org.apache.hc.core5.util.Args;
/**
* Minimal-allocation SSE line parser.
@@ -54,15 +59,27 @@ default void onLastEventId(final String id) {
}
private final Callback cb;
+ private final int maxEventSize;
private final StringBuilder data = new StringBuilder(128);
private String type; // defaults to "message"
private String id;
public ServerSentEventReader(final Callback cb) {
+ this(cb, EventSourceConfig.DEFAULT_MAX_EVENT_SIZE);
+ }
+
+ public ServerSentEventReader(final Callback cb, final int maxEventSize) {
this.cb = cb;
+ this.maxEventSize = Args.positive(maxEventSize, "Max event size");
}
- public void line(final String line) {
+ /**
+ * Processes one line without its terminator.
+ *
+ * @throws MessageConstraintException if the data accumulated for the current event
+ * exceeds the maximum event size
+ */
+ public void line(final String line) throws IOException {
// Trim possible trailing CR without reallocating
final int L0 = line.length();
int end = L0;
@@ -114,6 +131,11 @@ public void line(final String line) {
// data: (append newline; removed on dispatch)
if (vStart <= end) {
+ if ((long) data.length() + (end - vStart) + 1 > maxEventSize) {
+ data.setLength(0);
+ data.trimToSize();
+ throw new MessageConstraintException("Maximum event size exceeded: " + maxEventSize);
+ }
data.append(line, vStart, end).append('\n');
}
diff --git a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/SseEntityConsumer.java b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/SseEntityConsumer.java
index c1bc08764f..d6f33edaae 100644
--- a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/SseEntityConsumer.java
+++ b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/impl/SseEntityConsumer.java
@@ -32,10 +32,13 @@
import java.nio.CharBuffer;
import java.nio.charset.StandardCharsets;
+import org.apache.hc.client5.http.sse.EventSourceConfig;
import org.apache.hc.core5.annotation.Internal;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.HttpException;
+import org.apache.hc.core5.http.MessageConstraintException;
import org.apache.hc.core5.http.nio.entity.AbstractCharAsyncEntityConsumer;
+import org.apache.hc.core5.util.Args;
/**
* Internal char-level SSE consumer that converts decoded UTF-8 characters
@@ -53,6 +56,9 @@
* through {@link SseCallbacks}.
*
*
+ * A line longer than the maximum line length, or an event accumulating more than the
+ * maximum event size, fails the stream with {@link MessageConstraintException}.
+ *
* Thread-safety: Not thread-safe. One instance is expected to be
* used by a single decoding flow on an I/O thread.
*
@@ -63,13 +69,21 @@ public final class SseEntityConsumer extends AbstractCharAsyncEntityConsumer= maxLineLength) {
+ throw new MessageConstraintException("Maximum line length exceeded: " + maxLineLength);
+ }
partial.append(c);
}
}
- if (endOfStream) {
- partial.setLength(0);
- }
}
@Override
@@ -130,6 +156,7 @@ protected Void generateContent() {
@Override
public void releaseResources() {
partial.setLength(0);
+ partial.trimToSize();
pendingCr = false;
reader = null;
}
diff --git a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/package-info.java b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/package-info.java
index 6fa57894b2..cf6ed0a168 100644
--- a/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/package-info.java
+++ b/httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/package-info.java
@@ -89,6 +89,9 @@
* Backoff: Provide a {@link org.apache.hc.client5.http.sse.BackoffStrategy} via
* {@link org.apache.hc.client5.http.sse.EventSourceConfig}. Server {@code retry:} lines and
* HTTP {@code Retry-After} headers are honored when present.
+ * Limits: {@link org.apache.hc.client5.http.sse.EventSourceConfig} bounds the line length
+ * (default 1 MiB) and the data accumulated for one event (default 8 MiB). A stream that exceeds
+ * either limit is failed and the connection is closed.
* Headers: Add defaults on the {@link org.apache.hc.client5.http.sse.SseExecutorBuilder}
* or per-connection using {@link org.apache.hc.client5.http.sse.EventSource#setHeader(String, String)}.
* Parser: {@link org.apache.hc.client5.http.sse.impl.SseParser#CHAR} (default) is spec-compliant;
diff --git a/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/ByteSseEntityConsumerTest.java b/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/ByteSseEntityConsumerTest.java
index dd34c05dc4..7c189003fc 100644
--- a/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/ByteSseEntityConsumerTest.java
+++ b/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/ByteSseEntityConsumerTest.java
@@ -28,6 +28,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.nio.ByteBuffer;
@@ -36,6 +37,7 @@
import org.apache.hc.client5.http.sse.impl.ByteSseEntityConsumer;
import org.apache.hc.client5.http.sse.impl.SseCallbacks;
import org.apache.hc.core5.http.ContentType;
+import org.apache.hc.core5.http.MessageConstraintException;
import org.junit.jupiter.api.Test;
class ByteSseEntityConsumerTest {
@@ -212,4 +214,55 @@ void resetsLastEventIdWithoutDispatchingEvent() throws Exception {
assertEquals("", cb.lastEventId);
assertNull(cb.data);
}
+
+ private static ByteBuffer bytes(final String s) {
+ return ByteBuffer.wrap(s.getBytes(StandardCharsets.UTF_8));
+ }
+
+ @Test
+ void failsOnUnterminatedLineLongerThanLimit() throws Exception {
+ final Cb cb = new Cb();
+ final ByteSseEntityConsumer c = new ByteSseEntityConsumer(cb, 32, 1024);
+ c.streamStart(ContentType.parse("text/event-stream"));
+
+ c.consume(bytes("data: 0123456789012345678901234"));
+ assertThrows(MessageConstraintException.class, () -> c.consume(bytes("0123456789")));
+ assertNull(cb.data);
+ }
+
+ @Test
+ void failsOnManyDataLinesWithoutDispatch() throws Exception {
+ final Cb cb = new Cb();
+ final ByteSseEntityConsumer c = new ByteSseEntityConsumer(cb, 64, 100);
+ c.streamStart(ContentType.parse("text/event-stream"));
+
+ assertThrows(MessageConstraintException.class, () -> {
+ for (int i = 0; i < 1000; i++) {
+ c.consume(bytes("data: 0123456789\n"));
+ }
+ });
+ assertNull(cb.data);
+ }
+
+ @Test
+ void acceptsLineExactlyAtLimit() throws Exception {
+ final Cb cb = new Cb();
+ final ByteSseEntityConsumer c = new ByteSseEntityConsumer(cb, 16, 1024);
+ c.streamStart(ContentType.parse("text/event-stream"));
+
+ c.consume(bytes("data: 0123456789\n\n"));
+
+ assertEquals("0123456789", cb.data);
+ }
+
+ @Test
+ void acceptsMultiLineEventUnderLimits() throws Exception {
+ final Cb cb = new Cb();
+ final ByteSseEntityConsumer c = new ByteSseEntityConsumer(cb, 32, 64);
+ c.streamStart(ContentType.parse("text/event-stream"));
+
+ c.consume(bytes("data: line0\ndata: line1\ndata: line2\n\n"));
+
+ assertEquals("line0\nline1\nline2", cb.data);
+ }
}
diff --git a/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/ServerSentEventReaderTest.java b/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/ServerSentEventReaderTest.java
index 32360698ba..82c547cc3c 100644
--- a/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/ServerSentEventReaderTest.java
+++ b/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/ServerSentEventReaderTest.java
@@ -28,8 +28,10 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import org.apache.hc.client5.http.sse.impl.ServerSentEventReader;
+import org.apache.hc.core5.http.MessageConstraintException;
import org.junit.jupiter.api.Test;
class ServerSentEventReaderTest {
@@ -63,7 +65,7 @@ public void onRetryChange(final long retryMs) {
}
@Test
- void parsesMultiLineEventWithDefaults() {
+ void parsesMultiLineEventWithDefaults() throws Exception {
final Capt c = new Capt();
final ServerSentEventReader r = new ServerSentEventReader(c);
@@ -78,7 +80,7 @@ void parsesMultiLineEventWithDefaults() {
}
@Test
- void parsesEventTypeAndCommentAndRetry() {
+ void parsesEventTypeAndCommentAndRetry() throws Exception {
final Capt c = new Capt();
final ServerSentEventReader r = new ServerSentEventReader(c);
@@ -96,7 +98,7 @@ void parsesEventTypeAndCommentAndRetry() {
}
@Test
- void ignoresIdWithNul() {
+ void ignoresIdWithNul() throws Exception {
final Capt c = new Capt();
final ServerSentEventReader r = new ServerSentEventReader(c);
@@ -109,7 +111,7 @@ void ignoresIdWithNul() {
}
@Test
- void updatesLastEventIdOnBlankLineWithoutDispatchingEvent() {
+ void updatesLastEventIdOnBlankLineWithoutDispatchingEvent() throws Exception {
final Capt c = new Capt();
final ServerSentEventReader r = new ServerSentEventReader(c);
@@ -124,7 +126,7 @@ void updatesLastEventIdOnBlankLineWithoutDispatchingEvent() {
}
@Test
- void resetsLastEventIdOnBlankLineWithoutDispatchingEvent() {
+ void resetsLastEventIdOnBlankLineWithoutDispatchingEvent() throws Exception {
final Capt c = new Capt();
final ServerSentEventReader r = new ServerSentEventReader(c);
@@ -139,4 +141,26 @@ void resetsLastEventIdOnBlankLineWithoutDispatchingEvent() {
assertEquals("", c.lastEventId);
assertNull(c.data);
}
+
+ @Test
+ void failsWhenDataExceedsMaxEventSize() throws Exception {
+ final Capt c = new Capt();
+ final ServerSentEventReader r = new ServerSentEventReader(c, 16);
+
+ r.line("data: 01234567");
+ assertThrows(MessageConstraintException.class, () -> r.line("data: 01234567"));
+ assertNull(c.data);
+ }
+
+ @Test
+ void acceptsEventExactlyAtMaxEventSize() throws Exception {
+ final Capt c = new Capt();
+ final ServerSentEventReader r = new ServerSentEventReader(c, 16);
+
+ r.line("data: 0123456");
+ r.line("data: 012345");
+ r.line("");
+
+ assertEquals("0123456\n012345", c.data);
+ }
}
diff --git a/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/SseEntityConsumerTest.java b/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/SseEntityConsumerTest.java
index d8ecedb335..ec28042335 100644
--- a/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/SseEntityConsumerTest.java
+++ b/httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/SseEntityConsumerTest.java
@@ -28,6 +28,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@@ -36,6 +37,7 @@
import org.apache.hc.client5.http.sse.impl.SseCallbacks;
import org.apache.hc.client5.http.sse.impl.SseEntityConsumer;
import org.apache.hc.core5.http.ContentType;
+import org.apache.hc.core5.http.MessageConstraintException;
import org.junit.jupiter.api.Test;
class SseEntityConsumerTest {
@@ -176,4 +178,55 @@ void resetsLastEventIdWithoutDispatchingEvent() throws Exception {
assertEquals("", cb.lastEventId);
assertNull(cb.data);
}
+
+ @Test
+ void failsOnUnterminatedLineLongerThanLimit() throws Exception {
+ final Cb cb = new Cb();
+ final SseEntityConsumer c = new SseEntityConsumer(cb, 32, 1024);
+ c.streamStart(ContentType.parse("text/event-stream"));
+
+ c.data(CharBuffer.wrap("data: 0123456789012345678901234"), false);
+ assertThrows(MessageConstraintException.class,
+ () -> c.data(CharBuffer.wrap("0123456789"), false));
+ assertNull(cb.data);
+ }
+
+ @Test
+ void failsOnManyDataLinesWithoutDispatch() throws Exception {
+ final Cb cb = new Cb();
+ final SseEntityConsumer c = new SseEntityConsumer(cb, 64, 100);
+ c.streamStart(ContentType.parse("text/event-stream"));
+
+ assertThrows(MessageConstraintException.class, () -> {
+ for (int i = 0; i < 1000; i++) {
+ c.data(CharBuffer.wrap("data: 0123456789\n"), false);
+ }
+ });
+ assertNull(cb.data);
+ }
+
+ @Test
+ void acceptsLineExactlyAtLimit() throws Exception {
+ final Cb cb = new Cb();
+ final SseEntityConsumer c = new SseEntityConsumer(cb, 16, 1024);
+ c.streamStart(ContentType.parse("text/event-stream"));
+
+ c.data(CharBuffer.wrap("data: 0123456789\n\n"), false);
+
+ assertEquals("0123456789", cb.data);
+ }
+
+ @Test
+ void acceptsMultiLineEventUnderLimits() throws Exception {
+ final Cb cb = new Cb();
+ final SseEntityConsumer c = new SseEntityConsumer(cb, 32, 64);
+ c.streamStart(ContentType.parse("text/event-stream"));
+
+ for (int i = 0; i < 3; i++) {
+ c.data(CharBuffer.wrap("data: line" + i + "\n"), false);
+ }
+ c.data(CharBuffer.wrap("\n"), false);
+
+ assertEquals("line0\nline1\nline2", cb.data);
+ }
}
diff --git a/httpclient5-testing/pom.xml b/httpclient5-testing/pom.xml
index 97c1e96401..36062df988 100644
--- a/httpclient5-testing/pom.xml
+++ b/httpclient5-testing/pom.xml
@@ -126,7 +126,7 @@
org.apache.maven.plugins
maven-surefire-plugin
- slow,time-out,stale-conn
+ slow,time-out,stale-conn,cancellation
diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestHttp1Async.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestHttp1Async.java
index 700c631b36..cf5b0a477f 100644
--- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestHttp1Async.java
+++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestHttp1Async.java
@@ -46,6 +46,7 @@
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.URIScheme;
import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
@@ -139,6 +140,7 @@ void testSharedPool() throws Exception {
Assertions.assertEquals(2048, body3.length());
}
+ @Tag("cancellation")
@Test
void testRequestCancellation() throws Exception {
configureServer(bootstrap -> bootstrap.register("/random/*", AsyncRandomHandler::new));
diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java
index 17173a4253..0eb8de18fa 100644
--- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java
+++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java
@@ -65,7 +65,7 @@
import org.apache.hc.core5.net.URIBuilder;
import org.apache.hc.core5.util.TimeValue;
import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
/**
@@ -316,7 +316,8 @@ void testAbsoluteRequestURIWithFragment() throws Exception {
Assertions.assertEquals(uri, location);
}
- @Test @Disabled("Fails intermittently with GitHub Actions")
+ @Tag("cancellation")
+ @Test
void testRequestCancellation() throws Exception {
startServer();
final HttpHost target = startServer();
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
index 706793e98b..3a7de2326a 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
@@ -217,7 +217,8 @@ public AsyncDataConsumer handleResponse(
if (needAuthentication(
targetAuthExchange,
proxyAuthExchange,
- proxy != null ? proxy : target,
+ // A 407 received through a tunnel comes from the origin, not from the proxy
+ route.isTunnelled() ? null : proxy,
target,
pathPrefix,
response,
@@ -319,11 +320,11 @@ private boolean needAuthentication(
}
}
- final boolean proxyAuthRequested = authenticator.isChallenged(
+ final boolean proxyAuthRequested = proxy != null && authenticator.isChallenged(
proxy, ChallengeType.PROXY, response, proxyAuthExchange, context);
- final boolean proxyMutualAuthRequired = authenticator.isChallengeExpected(proxyAuthExchange);
+ final boolean proxyMutualAuthRequired = proxy != null && authenticator.isChallengeExpected(proxyAuthExchange);
- if (authCacheKeeper != null) {
+ if (proxy != null && authCacheKeeper != null) {
if (proxyAuthRequested) {
authCacheKeeper.updateOnChallenge(proxy, null, proxyAuthExchange, context);
} else {
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
index 4514ddfe94..cc73b224e5 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
@@ -207,7 +207,8 @@ public ClassicHttpResponse execute(
if (needAuthentication(
targetAuthExchange,
proxyAuthExchange,
- proxy != null ? proxy : target,
+ // A 407 received through a tunnel comes from the origin, not from the proxy
+ route.isTunnelled() ? null : proxy,
target,
pathPrefix,
response,
@@ -280,11 +281,11 @@ private boolean needAuthentication(
}
}
- final boolean proxyAuthRequested = authenticator.isChallenged(
+ final boolean proxyAuthRequested = proxy != null && authenticator.isChallenged(
proxy, ChallengeType.PROXY, response, proxyAuthExchange, context);
- final boolean proxyMutualAuthRequired = authenticator.isChallengeExpected(proxyAuthExchange);
+ final boolean proxyMutualAuthRequired = proxy != null && authenticator.isChallengeExpected(proxyAuthExchange);
- if (authCacheKeeper != null) {
+ if (proxy != null && authCacheKeeper != null) {
if (proxyAuthRequested) {
authCacheKeeper.updateOnChallenge(proxy, null, proxyAuthExchange, context);
} else {
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java
index 78e9476b1e..49cef36be2 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java
@@ -491,7 +491,8 @@ public void connect(final ConnectionEndpoint endpoint, final TimeValue timeout,
this.connectionOperator.connect(
connection,
firstHop,
- route.getTargetName(),
+ // The target name applies to the target only, never to an intermediate proxy
+ route.getProxyHost() != null ? null : route.getTargetName(),
route.getLocalSocketAddress(),
connectTimeout,
socketConfig,
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java
index 2945e8ed7d..6d670bf3e6 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java
@@ -565,7 +565,8 @@ public void connect(final ConnectionEndpoint endpoint, final TimeValue timeout,
this.connectionOperator.connect(
conn,
firstHop,
- route.getTargetName(),
+ // The target name applies to the target only, never to an intermediate proxy
+ route.getProxyHost() != null ? null : route.getTargetName(),
unixDomainSocket,
route.getLocalSocketAddress(),
connectTimeout,
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java
index c5215a4156..d030b9b3f3 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java
@@ -523,7 +523,8 @@ public Future connect(
connectionInitiator,
firstHop,
unixDomainSocket,
- route.getTargetName(),
+ // The target name applies to the target only, never to an intermediate proxy
+ route.getProxyHost() != null ? null : route.getTargetName(),
route.getLocalSocketAddress(),
connectTimeout,
connectAttachment,
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java
index d403ff18df..0aba6a75ae 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java
@@ -99,6 +99,10 @@ abstract class AbstractClientTlsStrategy implements TlsStrategy, TlsSocketStrate
this.sslBufferManagement = sslBufferManagement != null ? sslBufferManagement : SSLBufferMode.STATIC;
this.hostnameVerificationPolicy = hostnameVerificationPolicy != null ? hostnameVerificationPolicy :
(hostnameVerifier != null ? HostnameVerificationPolicy.BOTH : HostnameVerificationPolicy.BUILTIN);
+ Args.check(this.hostnameVerificationPolicy != HostnameVerificationPolicy.CLIENT
+ && this.hostnameVerificationPolicy != HostnameVerificationPolicy.BOTH
+ || hostnameVerifier != null,
+ "Hostname verifier is required for hostname verification policy " + this.hostnameVerificationPolicy);
this.hostnameVerifier = hostnameVerifier;
}
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/ConscryptClientTlsStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/ConscryptClientTlsStrategy.java
index bcf9bfd344..d635866f73 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/ConscryptClientTlsStrategy.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/ConscryptClientTlsStrategy.java
@@ -69,7 +69,8 @@ public ConscryptClientTlsStrategy(
final String[] supportedCipherSuites,
final SSLBufferMode sslBufferManagement,
final HostnameVerifier hostnameVerifier) {
- this(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement, HostnameVerificationPolicy.CLIENT, hostnameVerifier);
+ this(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement,
+ hostnameVerifier != null ? HostnameVerificationPolicy.CLIENT : HostnameVerificationPolicy.BUILTIN, hostnameVerifier);
}
/**
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java
index c52997473f..2792df98f0 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java
@@ -101,7 +101,8 @@ public DefaultClientTlsStrategy(
final SSLBufferMode sslBufferManagement,
final HostnameVerifier hostnameVerifier,
final Factory tlsDetailsFactory) {
- super(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement, HostnameVerificationPolicy.CLIENT, hostnameVerifier);
+ super(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement,
+ hostnameVerifier != null ? HostnameVerificationPolicy.CLIENT : HostnameVerificationPolicy.BUILTIN, hostnameVerifier);
this.tlsDetailsFactory = tlsDetailsFactory;
}
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/async/TestAsyncProtocolExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/async/TestAsyncProtocolExec.java
index dff99e94e6..7fcae912dc 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/async/TestAsyncProtocolExec.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/async/TestAsyncProtocolExec.java
@@ -27,11 +27,14 @@
package org.apache.hc.client5.http.impl.async;
import java.io.IOException;
+import java.util.ArrayList;
import java.util.Collections;
+import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.hc.client5.http.AuthenticationStrategy;
import org.apache.hc.client5.http.HttpRoute;
+import org.apache.hc.client5.http.RouteInfo;
import org.apache.hc.client5.http.async.AsyncExecCallback;
import org.apache.hc.client5.http.async.AsyncExecChain;
import org.apache.hc.client5.http.async.AsyncExecRuntime;
@@ -39,6 +42,7 @@
import org.apache.hc.client5.http.auth.AuthScope;
import org.apache.hc.client5.http.auth.ChallengeType;
import org.apache.hc.client5.http.auth.StandardAuthScheme;
+import org.apache.hc.client5.http.impl.auth.BasicAuthCache;
import org.apache.hc.client5.http.impl.auth.BasicScheme;
import org.apache.hc.client5.http.impl.auth.CredentialsProviderBuilder;
import org.apache.hc.client5.http.protocol.HttpClientContext;
@@ -157,4 +161,111 @@ public void failed(final Exception cause) {
final AuthExchange authExchange = context.getAuthExchange(target);
Assertions.assertEquals("/blah/", authExchange.getPathPrefix());
}
+
+ private static final class RecordingCallback implements AsyncExecCallback {
+
+ private final List codes = new ArrayList<>();
+ private final AtomicInteger completed = new AtomicInteger(0);
+
+ @Override
+ public AsyncDataConsumer handleResponse(final HttpResponse response, final EntityDetails entityDetails) {
+ codes.add(response.getCode());
+ return Mockito.mock(AsyncDataConsumer.class);
+ }
+
+ @Override
+ public void handleInformationResponse(final HttpResponse response) {
+ }
+
+ @Override
+ public void completed() {
+ completed.incrementAndGet();
+ }
+
+ @Override
+ public void failed(final Exception cause) {
+ Assertions.fail(cause);
+ }
+
+ }
+
+ private void stubChain(final AtomicInteger proceedCount, final List requests, final int... codes)
+ throws Exception {
+ Mockito.doAnswer(invocation -> {
+ requests.add(invocation.getArgument(0, HttpRequest.class));
+ final AsyncExecCallback cb = invocation.getArgument(3, AsyncExecCallback.class);
+ final int code = codes[Math.min(proceedCount.getAndIncrement(), codes.length - 1)];
+ final HttpResponse resp = new BasicHttpResponse(code, "Huh?");
+ if (code == 407) {
+ resp.setHeader(HttpHeaders.PROXY_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test");
+ }
+ cb.handleResponse(resp, (EntityDetails) null);
+ cb.completed();
+ return null;
+ }).when(chain).proceed(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
+ }
+
+ @Test
+ void testProxyChallengeThroughTunnelWithAuthCacheIsReturnedToCaller() throws Exception {
+ final AsyncProtocolExec cachingExec = new AsyncProtocolExec(targetAuthStrategy, proxyAuthStrategy, null, false);
+ final HttpHost httpsTarget = new HttpHost("https", "foo", 443);
+ final HttpHost proxy = new HttpHost("http", "proxy", 8080);
+ final HttpRoute route = new HttpRoute(httpsTarget, null, proxy, true,
+ RouteInfo.TunnelType.TUNNELLED, RouteInfo.LayerType.LAYERED);
+ final BasicAuthCache authCache = new BasicAuthCache();
+ final HttpClientContext context = HttpClientContext.create();
+ context.setAuthCache(authCache);
+ context.setCredentialsProvider(CredentialsProviderBuilder.create()
+ .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray())
+ .build());
+ Mockito.when(execRuntime.isEndpointConnected()).thenReturn(true);
+ final AtomicInteger proceedCount = new AtomicInteger(0);
+ final List requests = new ArrayList<>();
+ stubChain(proceedCount, requests, 407);
+
+ final HttpRequest request = BasicRequestBuilder.get("https://foo/test").build();
+ final AsyncExecChain.Scope scope = new AsyncExecChain.Scope("test", route, request,
+ Mockito.mock(CancellableDependency.class), context, execRuntime, null, new AtomicInteger(1));
+ final RecordingCallback callback = new RecordingCallback();
+
+ cachingExec.execute(request, (AsyncEntityProducer) null, scope, chain, callback);
+
+ Assertions.assertEquals(Collections.singletonList(407), callback.codes);
+ Assertions.assertEquals(1, callback.completed.get());
+ Assertions.assertEquals(1, proceedCount.get());
+ Assertions.assertNull(requests.get(0).getFirstHeader(HttpHeaders.PROXY_AUTHORIZATION));
+ Mockito.verifyNoInteractions(proxyAuthStrategy);
+ Assertions.assertEquals(AuthExchange.State.UNCHALLENGED, context.getAuthExchange(proxy).getState());
+ Assertions.assertNull(authCache.get(proxy));
+ }
+
+ @Test
+ void testProxyChallengeOnPlainRouteStillAuthenticates() throws Exception {
+ final HttpHost proxy = new HttpHost("http", "proxy", 8080);
+ final HttpRoute route = new HttpRoute(target, null, proxy, false);
+ final HttpClientContext context = HttpClientContext.create();
+ context.setCredentialsProvider(CredentialsProviderBuilder.create()
+ .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray())
+ .build());
+ Mockito.when(proxyAuthStrategy.select(
+ Mockito.eq(ChallengeType.PROXY),
+ Mockito.any(),
+ Mockito.any()))
+ .thenReturn(Collections.singletonList(new BasicScheme()));
+ Mockito.when(execRuntime.isEndpointConnected()).thenReturn(true);
+ final AtomicInteger proceedCount = new AtomicInteger(0);
+ final List requests = new ArrayList<>();
+ stubChain(proceedCount, requests, 407, 200);
+
+ final HttpRequest request = BasicRequestBuilder.get("http://foo/test").build();
+ final AsyncExecChain.Scope scope = new AsyncExecChain.Scope("test", route, request,
+ Mockito.mock(CancellableDependency.class), context, execRuntime, null, new AtomicInteger(1));
+ final RecordingCallback callback = new RecordingCallback();
+
+ protocolExec.execute(request, (AsyncEntityProducer) null, scope, chain, callback);
+
+ Assertions.assertEquals(2, proceedCount.get());
+ Assertions.assertEquals(Collections.singletonList(200), callback.codes);
+ Assertions.assertNotNull(requests.get(1).getFirstHeader(HttpHeaders.PROXY_AUTHORIZATION));
+ }
}
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java
index 3417e66ff5..9588a42972 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java
@@ -34,6 +34,7 @@
import org.apache.hc.client5.http.AuthenticationStrategy;
import org.apache.hc.client5.http.HttpRoute;
+import org.apache.hc.client5.http.RouteInfo;
import org.apache.hc.client5.http.auth.AuthExchange;
import org.apache.hc.client5.http.auth.AuthScope;
import org.apache.hc.client5.http.auth.ChallengeType;
@@ -43,6 +44,7 @@
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.classic.methods.HttpPost;
import org.apache.hc.client5.http.entity.EntityBuilder;
+import org.apache.hc.client5.http.impl.auth.BasicAuthCache;
import org.apache.hc.client5.http.impl.auth.BasicScheme;
import org.apache.hc.client5.http.impl.auth.CredentialsProviderBuilder;
import org.apache.hc.client5.http.protocol.HttpClientContext;
@@ -58,6 +60,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
@@ -295,4 +298,105 @@ void testAuthExchangePathPrefixRestoredAfterChallenge() throws Exception {
final AuthExchange authExchange = context.getAuthExchange(target);
Assertions.assertEquals("/blah/", authExchange.getPathPrefix());
}
+
+ @Test
+ void testProxyChallengeThroughTunnelIsReturnedToCaller() throws Exception {
+ final HttpHost httpsTarget = new HttpHost("https", "foo", 443);
+ final HttpHost proxy = new HttpHost("http", "proxy", 8080);
+ final HttpRoute route = new HttpRoute(httpsTarget, null, proxy, true,
+ RouteInfo.TunnelType.TUNNELLED, RouteInfo.LayerType.LAYERED);
+ final HttpClientContext context = HttpClientContext.create();
+ context.setCredentialsProvider(CredentialsProviderBuilder.create()
+ .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray())
+ .build());
+ final ClassicHttpRequest request = new HttpGet("https://foo/test");
+ final ClassicHttpResponse response = new BasicClassicHttpResponse(407, "Huh?");
+ response.setHeader(HttpHeaders.PROXY_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test");
+
+ Mockito.when(chain.proceed(Mockito.same(request), Mockito.any())).thenReturn(response);
+
+ final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context);
+ final ClassicHttpResponse finalResponse = protocolExec.execute(request, scope, chain);
+ Assertions.assertEquals(407, finalResponse.getCode());
+ Mockito.verify(chain, Mockito.times(1)).proceed(request, scope);
+ Mockito.verifyNoInteractions(proxyAuthStrategy);
+ Assertions.assertNull(context.getAuthExchange(proxy).getAuthScheme());
+ }
+
+ @Test
+ void testProxyChallengeWithoutProxyIsReturnedToCaller() throws Exception {
+ final HttpRoute route = new HttpRoute(target);
+ final HttpClientContext context = HttpClientContext.create();
+ context.setCredentialsProvider(CredentialsProviderBuilder.create()
+ .add(new AuthScope(target), "user", "pass".toCharArray())
+ .build());
+ final ClassicHttpRequest request = new HttpGet("http://foo/test");
+ final ClassicHttpResponse response = new BasicClassicHttpResponse(407, "Huh?");
+ response.setHeader(HttpHeaders.PROXY_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test");
+
+ Mockito.when(chain.proceed(Mockito.same(request), Mockito.any())).thenReturn(response);
+
+ final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context);
+ final ClassicHttpResponse finalResponse = protocolExec.execute(request, scope, chain);
+ Assertions.assertEquals(407, finalResponse.getCode());
+ Mockito.verify(chain, Mockito.times(1)).proceed(request, scope);
+ Mockito.verifyNoInteractions(proxyAuthStrategy);
+ }
+
+ @Test
+ void testProxyChallengeThroughTunnelWithAuthCacheIsReturnedToCaller() throws Exception {
+ final ProtocolExec cachingExec = new ProtocolExec(targetAuthStrategy, proxyAuthStrategy, null, false);
+ final HttpHost httpsTarget = new HttpHost("https", "foo", 443);
+ final HttpHost proxy = new HttpHost("http", "proxy", 8080);
+ final HttpRoute route = new HttpRoute(httpsTarget, null, proxy, true,
+ RouteInfo.TunnelType.TUNNELLED, RouteInfo.LayerType.LAYERED);
+ final BasicAuthCache authCache = new BasicAuthCache();
+ final HttpClientContext context = HttpClientContext.create();
+ context.setAuthCache(authCache);
+ context.setCredentialsProvider(CredentialsProviderBuilder.create()
+ .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray())
+ .build());
+ final ClassicHttpRequest request = new HttpGet("https://foo/test");
+ final ClassicHttpResponse response = new BasicClassicHttpResponse(407, "Huh?");
+ response.setHeader(HttpHeaders.PROXY_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test");
+
+ Mockito.when(chain.proceed(Mockito.same(request), Mockito.any())).thenReturn(response);
+
+ final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context);
+ final ClassicHttpResponse finalResponse = cachingExec.execute(request, scope, chain);
+ Assertions.assertSame(response, finalResponse);
+ Mockito.verify(chain, Mockito.times(1)).proceed(request, scope);
+ Mockito.verifyNoInteractions(proxyAuthStrategy);
+ Assertions.assertNull(request.getFirstHeader(HttpHeaders.PROXY_AUTHORIZATION));
+ Assertions.assertEquals(AuthExchange.State.UNCHALLENGED, context.getAuthExchange(proxy).getState());
+ Assertions.assertNull(authCache.get(proxy));
+ }
+
+ @Test
+ void testProxyChallengeOnPlainRouteStillAuthenticates() throws Exception {
+ final HttpHost proxy = new HttpHost("http", "proxy", 8080);
+ final HttpRoute route = new HttpRoute(target, null, proxy, false);
+ final HttpClientContext context = HttpClientContext.create();
+ context.setCredentialsProvider(CredentialsProviderBuilder.create()
+ .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray())
+ .build());
+ final ClassicHttpRequest request = new HttpGet("http://foo/test");
+ final ClassicHttpResponse response1 = new BasicClassicHttpResponse(407, "Huh?");
+ response1.setHeader(HttpHeaders.PROXY_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test");
+ final ClassicHttpResponse response2 = new BasicClassicHttpResponse(200, "OK");
+
+ Mockito.when(chain.proceed(Mockito.any(), Mockito.any())).thenReturn(response1, response2);
+ Mockito.when(proxyAuthStrategy.select(
+ Mockito.eq(ChallengeType.PROXY),
+ Mockito.any(),
+ Mockito.any())).thenReturn(Collections.singletonList(new BasicScheme()));
+ Mockito.when(execRuntime.isConnectionReusable()).thenReturn(true);
+
+ final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context);
+ final ClassicHttpResponse finalResponse = protocolExec.execute(request, scope, chain);
+ Assertions.assertEquals(200, finalResponse.getCode());
+ final ArgumentCaptor captor = ArgumentCaptor.forClass(ClassicHttpRequest.class);
+ Mockito.verify(chain, Mockito.times(2)).proceed(captor.capture(), Mockito.same(scope));
+ Assertions.assertNotNull(captor.getAllValues().get(1).getFirstHeader(HttpHeaders.PROXY_AUTHORIZATION));
+ }
}
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java
index b045626750..e5f8416018 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java
@@ -53,6 +53,7 @@
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.config.Lookup;
import org.apache.hc.core5.http.io.SocketConfig;
+import org.apache.hc.core5.net.URIAuthority;
import org.apache.hc.core5.http2.HttpVersionPolicy;
import org.apache.hc.core5.pool.PoolEntry;
import org.apache.hc.core5.pool.StrictConnPool;
@@ -407,5 +408,52 @@ void testConcurrentShutdown() throws InterruptedException {
Assertions.assertTrue(mgr.isClosed(), "Connection manager should be shutdown after concurrent calls to shutdown.");
}
+ @Test
+ void testHttpsProxyVerifiedAgainstProxyNotTargetName() throws Exception {
+ final HttpHost target = new HttpHost("https", "somehost", 443);
+ final URIAuthority targetName = new URIAuthority("virtualhost", 443);
+ final HttpHost proxy = new HttpHost("https", "someproxy", 8443);
+ final InetAddress remote = InetAddress.getByAddress(new byte[] {10, 0, 0, 1});
+ final HttpRoute route = new HttpRoute(target, targetName, null, proxy, true);
+ Assertions.assertTrue(route.isTunnelled());
+
+ final PoolEntry entry = new PoolEntry<>(route, TimeValue.NEG_ONE_MILLISECOND);
+ entry.assignConnection(conn);
+
+ Mockito.when(conn.isOpen()).thenReturn(false);
+ Mockito.when(future.get(1, TimeUnit.SECONDS)).thenReturn(entry);
+ Mockito.when(pool.lease(
+ Mockito.eq(route),
+ Mockito.eq(null),
+ Mockito.any(),
+ Mockito.eq(null)))
+ .thenReturn(future);
+
+ final LeaseRequest connRequest = mgr.lease("some-id", route, null);
+ final ConnectionEndpoint endpoint = connRequest.get(Timeout.ofSeconds(1));
+ Assertions.assertNotNull(endpoint);
+
+ final HttpClientContext context = HttpClientContext.create();
+ final TlsConfig tlsConfig = TlsConfig.custom().build();
+ mgr.setDefaultTlsConfig(tlsConfig);
+
+ Mockito.when(dnsResolver.resolve("someproxy", 8443)).thenReturn(Collections.singletonList(new InetSocketAddress(remote, 8443)));
+ Mockito.when(schemePortResolver.resolve(proxy.getSchemeName(), proxy)).thenReturn(8443);
+ Mockito.when(tlsSocketStrategyLookup.lookup("https")).thenReturn(tlsSocketStrategy);
+ Mockito.when(detachedSocketFactory.create(Mockito.any(), Mockito.any())).thenReturn(socket);
+ Mockito.when(tlsSocketStrategy.upgrade(
+ Mockito.any(), Mockito.any(), Mockito.anyInt(), Mockito.any(), Mockito.any())).thenReturn(upgradedSocket);
+
+ mgr.connect(endpoint, null, context);
+
+ Mockito.verify(tlsSocketStrategy).upgrade(socket, "someproxy", 8443, null, context);
+
+ Mockito.when(conn.isOpen()).thenReturn(true);
+ Mockito.when(conn.getSocket()).thenReturn(socket);
+
+ mgr.upgrade(endpoint, context);
+
+ Mockito.verify(tlsSocketStrategy).upgrade(socket, "virtualhost", 443, tlsConfig, context);
+ }
}
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java
index 20a2bbd407..a052ca16e7 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java
@@ -27,17 +27,21 @@
package org.apache.hc.client5.http.ssl;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.security.cert.X509Certificate;
+import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLSession;
import javax.security.auth.x500.X500Principal;
+import org.apache.hc.core5.function.Factory;
import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
import org.apache.hc.core5.reactor.ssl.TlsDetails;
import org.apache.hc.core5.ssl.SSLContexts;
@@ -110,5 +114,31 @@ TlsDetails createTlsDetails(final SSLEngine sslEngine) {
}
+ @Test
+ void testClientPolicyRequiresHostnameVerifier() {
+ assertThrows(IllegalArgumentException.class, () -> new DefaultClientTlsStrategy(
+ SSLContexts.createDefault(), HostnameVerificationPolicy.CLIENT, null));
+ assertThrows(IllegalArgumentException.class, () -> new DefaultClientTlsStrategy(
+ SSLContexts.createDefault(), HostnameVerificationPolicy.BOTH, null));
+ }
+
+ @Test
+ void testBuilderClientPolicyRequiresHostnameVerifier() {
+ final ClientTlsStrategyBuilder builder = ClientTlsStrategyBuilder.create()
+ .setSslContext(SSLContexts.createDefault())
+ .setHostVerificationPolicy(HostnameVerificationPolicy.CLIENT);
+ assertThrows(IllegalArgumentException.class, builder::buildClassic);
+ }
+
+ @Test
+ @SuppressWarnings("deprecation")
+ void testNullHostnameVerifierDefaultsToBuiltinPolicy() {
+ assertDoesNotThrow(() -> new DefaultClientTlsStrategy(
+ SSLContexts.createDefault(), null, null, SSLBufferMode.STATIC,
+ (HostnameVerifier) null, (Factory) null));
+ assertDoesNotThrow(() -> new DefaultClientTlsStrategy(SSLContexts.createDefault()));
+ assertDoesNotThrow(() -> new ConscryptClientTlsStrategy(SSLContexts.createDefault()));
+ }
+
}
\ No newline at end of file
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestNullHostnameVerifierPolicy.java b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestNullHostnameVerifierPolicy.java
new file mode 100644
index 0000000000..02b9ee22b0
--- /dev/null
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestNullHostnameVerifierPolicy.java
@@ -0,0 +1,147 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation. For more
+ * information on the Apache Software Foundation, please see
+ * .
+ *
+ */
+
+
+package org.apache.hc.client5.http.ssl;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.Socket;
+
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLException;
+import javax.net.ssl.SSLServerSocket;
+import javax.net.ssl.SSLSocket;
+
+import org.apache.hc.core5.function.Factory;
+import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
+import org.apache.hc.core5.reactor.ssl.TlsDetails;
+import org.apache.hc.core5.ssl.SSLContexts;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Handshakes against a server whose certificate is issued for {@code localhost} to make sure
+ * that a strategy created without a {@link javax.net.ssl.HostnameVerifier} still verifies the host name.
+ */
+class TestNullHostnameVerifierPolicy {
+
+ private static final char[] PASSWORD = "nopassword".toCharArray();
+
+ private SSLContext sslContext;
+ private SSLServerSocket serverSocket;
+ private Thread serverThread;
+
+ @BeforeEach
+ void setUp() throws Exception {
+ sslContext = SSLContexts.custom()
+ .loadTrustMaterial(getClass().getResource("/test.keystore"), PASSWORD)
+ .loadKeyMaterial(getClass().getResource("/test.keystore"), PASSWORD, PASSWORD)
+ .build();
+ serverSocket = (SSLServerSocket) sslContext.getServerSocketFactory()
+ .createServerSocket(0, 1, InetAddress.getLoopbackAddress());
+ serverThread = new Thread(() -> {
+ try (final SSLSocket socket = (SSLSocket) serverSocket.accept()) {
+ socket.startHandshake();
+ } catch (final IOException ignore) {
+ // the client may abort the handshake
+ }
+ });
+ serverThread.setDaemon(true);
+ serverThread.start();
+ }
+
+ @AfterEach
+ void tearDown() throws Exception {
+ serverSocket.close();
+ serverThread.join(5000);
+ }
+
+ private SSLSocket upgrade(final AbstractClientTlsStrategy strategy, final String target) throws IOException {
+ final Socket socket = new Socket(InetAddress.getLoopbackAddress(), serverSocket.getLocalPort());
+ try {
+ return strategy.upgrade(socket, target, serverSocket.getLocalPort(), null, null);
+ } catch (final IOException | RuntimeException ex) {
+ socket.close();
+ throw ex;
+ }
+ }
+
+ @SuppressWarnings("deprecation")
+ private AbstractClientTlsStrategy defaultStrategy() {
+ return new DefaultClientTlsStrategy(
+ sslContext, null, null, SSLBufferMode.STATIC,
+ (HostnameVerifier) null, (Factory) null);
+ }
+
+ @SuppressWarnings("deprecation")
+ private AbstractClientTlsStrategy conscryptStrategy() {
+ return new ConscryptClientTlsStrategy(
+ sslContext, null, null, SSLBufferMode.STATIC, (HostnameVerifier) null);
+ }
+
+ @Test
+ void testDefaultStrategyRejectsMismatchingHost() {
+ assertThrows(SSLException.class, () -> upgrade(defaultStrategy(), "example.invalid"));
+ }
+
+ @Test
+ void testDefaultStrategyAcceptsMatchingHost() throws Exception {
+ try (final SSLSocket socket = upgrade(defaultStrategy(), "localhost")) {
+ assertNotNull(socket.getSession().getPeerCertificates());
+ }
+ }
+
+ @Test
+ void testConscryptStrategyRejectsMismatchingHost() {
+ assertThrows(SSLException.class, () -> upgrade(conscryptStrategy(), "example.invalid"));
+ }
+
+ @Test
+ void testConscryptStrategyAcceptsMatchingHost() throws Exception {
+ try (final SSLSocket socket = upgrade(conscryptStrategy(), "localhost")) {
+ assertNotNull(socket.getSession().getPeerCertificates());
+ }
+ }
+
+ @Test
+ void testConscryptDefaultConstructorRejectsMismatchingHost() {
+ assertThrows(SSLException.class, () -> upgrade(new ConscryptClientTlsStrategy(sslContext), "example.invalid"));
+ }
+
+ @Test
+ void testDefaultConstructorRejectsMismatchingHost() {
+ assertThrows(SSLException.class, () -> upgrade(new DefaultClientTlsStrategy(sslContext), "example.invalid"));
+ }
+
+}
diff --git a/httpclient5/src/test/resources/test.keystore b/httpclient5/src/test/resources/test.keystore
new file mode 100644
index 0000000000..f8d5ace1ad
Binary files /dev/null and b/httpclient5/src/test/resources/test.keystore differ
diff --git a/pom.xml b/pom.xml
index 36db97227e..ac7e99dc0e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -60,7 +60,7 @@
- 8
+ 17
17
${javaVersion}
${javaVersion}