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 2017/04/15 16:32:37 UTC
svn commit: r1791521 - in /httpcomponents/httpcore/trunk:
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/
httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/
Author: olegk
Date: Sat Apr 15 16:32:37 2017
New Revision: 1791521
URL: http://svn.apache.org/viewvc?rev=1791521&view=rev
Log:
Improved handling of invalid requests by the HTTP/2 client protocol handler; invalid requests should never get sent to the opposite endpoint
Modified:
httpcomponents/httpcore/trunk/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractHttp2StreamMultiplexer.java
httpcomponents/httpcore/trunk/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientHttp2StreamHandler.java
httpcomponents/httpcore/trunk/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/Http2IntegrationTest.java
Modified: httpcomponents/httpcore/trunk/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractHttp2StreamMultiplexer.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractHttp2StreamMultiplexer.java?rev=1791521&r1=1791520&r2=1791521&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractHttp2StreamMultiplexer.java (original)
+++ httpcomponents/httpcore/trunk/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractHttp2StreamMultiplexer.java Sat Apr 15 16:32:37 2017
@@ -586,6 +586,7 @@ abstract class AbstractHttp2StreamMultip
final int streamId = generateStreamId();
final Http2StreamChannelImpl channel = new Http2StreamChannelImpl(
streamId,
+ true,
localConfig.getInitialWindowSize(),
remoteConfig.getInitialWindowSize());
final AsyncClientExchangeHandler exchangeHandler = executionCommand.getExchangeHandler();
@@ -725,6 +726,7 @@ abstract class AbstractHttp2StreamMultip
final Http2StreamChannelImpl channel = new Http2StreamChannelImpl(
streamId,
+ false,
localConfig.getInitialWindowSize(),
remoteConfig.getInitialWindowSize());
final Http2StreamHandler streamHandler = createRemotelyInitiatedStream(
@@ -903,6 +905,7 @@ abstract class AbstractHttp2StreamMultip
final Http2StreamChannelImpl channel = new Http2StreamChannelImpl(
promisedStreamId,
+ false,
localConfig.getInitialWindowSize(),
remoteConfig.getInitialWindowSize());
final Http2StreamHandler streamHandler = createRemotelyInitiatedStream(
@@ -1252,13 +1255,15 @@ abstract class AbstractHttp2StreamMultip
private final AtomicInteger inputWindow;
private final AtomicInteger outputWindow;
+ private volatile boolean idle;
private volatile boolean remoteEndStream;
private volatile boolean localEndStream;
private volatile long deadline;
- Http2StreamChannelImpl(final int id, final int initialInputWindowSize, final int initialOutputWindowSize) {
+ Http2StreamChannelImpl(final int id, final boolean idle, final int initialInputWindowSize, final int initialOutputWindowSize) {
this.id = id;
+ this.idle = idle;
this.inputWindow = new AtomicInteger(initialInputWindowSize);
this.outputWindow = new AtomicInteger(initialOutputWindowSize);
}
@@ -1285,6 +1290,7 @@ abstract class AbstractHttp2StreamMultip
if (localEndStream) {
return;
}
+ idle = false;
commitHeaders(id, headers, endStream);
if (endStream) {
localEndStream = true;
@@ -1302,6 +1308,7 @@ abstract class AbstractHttp2StreamMultip
final int promisedStreamId = generateStreamId();
final Http2StreamChannelImpl channel = new Http2StreamChannelImpl(
promisedStreamId,
+ true,
localConfig.getInitialWindowSize(),
remoteConfig.getInitialWindowSize());
final HttpCoreContext context = HttpCoreContext.create();
@@ -1318,6 +1325,7 @@ abstract class AbstractHttp2StreamMultip
return;
}
commitPushPromise(id, promisedStreamId, headers);
+ idle = false;
} finally {
outputLock.unlock();
}
@@ -1400,13 +1408,15 @@ abstract class AbstractHttp2StreamMultip
void localReset(final int code) throws IOException {
deadline = System.currentTimeMillis() + LINGER_TIME;
- outputLock.lock();
- try {
- close();
- final RawFrame resetStream = frameFactory.createResetStream(id, code);
- commitFrameInternal(resetStream);
- } finally {
- outputLock.unlock();
+ close();
+ if (!idle) {
+ outputLock.lock();
+ try {
+ final RawFrame resetStream = frameFactory.createResetStream(id, code);
+ commitFrameInternal(resetStream);
+ } finally {
+ outputLock.unlock();
+ }
}
}
Modified: httpcomponents/httpcore/trunk/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientHttp2StreamHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientHttp2StreamHandler.java?rev=1791521&r1=1791520&r2=1791521&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientHttp2StreamHandler.java (original)
+++ httpcomponents/httpcore/trunk/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientHttp2StreamHandler.java Sat Apr 15 16:32:37 2017
@@ -132,10 +132,10 @@ class ClientHttp2StreamHandler implement
}
httpProcessor.process(request, entityDetails, context);
- connMetrics.incrementRequestCount();
final List<Header> headers = DefaultH2RequestConverter.INSTANCE.convert(request);
outputChannel.submit(headers, entityDetails == null);
+ connMetrics.incrementRequestCount();
if (entityDetails == null) {
requestState = MessageState.COMPLETE;
Modified: httpcomponents/httpcore/trunk/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/Http2IntegrationTest.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/Http2IntegrationTest.java?rev=1791521&r1=1791520&r2=1791521&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/Http2IntegrationTest.java (original)
+++ httpcomponents/httpcore/trunk/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/Http2IntegrationTest.java Sat Apr 15 16:32:37 2017
@@ -52,6 +52,7 @@ import java.util.Queue;
import java.util.StringTokenizer;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.LinkedBlockingDeque;
@@ -62,14 +63,19 @@ import java.util.concurrent.atomic.Atomi
import org.apache.hc.core5.function.Callback;
import org.apache.hc.core5.function.Supplier;
import org.apache.hc.core5.http.ContentType;
+import org.apache.hc.core5.http.EndpointDetails;
import org.apache.hc.core5.http.EntityDetails;
import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.HeaderElements;
+import org.apache.hc.core5.http.HttpConnection;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.Message;
+import org.apache.hc.core5.http.ProtocolException;
import org.apache.hc.core5.http.impl.nio.AbstractClassicServerExchangeHandler;
import org.apache.hc.core5.http.impl.nio.entity.AbstractClassicEntityConsumer;
import org.apache.hc.core5.http.impl.nio.entity.AbstractClassicEntityProducer;
@@ -102,9 +108,12 @@ import org.apache.hc.core5.http2.config.
import org.apache.hc.core5.http2.nio.command.PingCommand;
import org.apache.hc.core5.http2.nio.support.BasicPingHandler;
import org.apache.hc.core5.reactor.IOReactorConfig;
+import org.apache.hc.core5.reactor.IOSession;
+import org.apache.hc.core5.reactor.SessionRequest;
import org.apache.hc.core5.testing.ProtocolScheme;
import org.apache.hc.core5.util.TextUtils;
import org.apache.hc.core5.util.TimeValue;
+import org.hamcrest.CoreMatchers;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
@@ -948,4 +957,41 @@ public class Http2IntegrationTest extend
Assert.assertEquals(n, count.get());
}
+ @Test
+ public void testRequestWithInvalidConnectionHeader() throws Exception {
+ server.register("/hello", new Supplier<AsyncServerExchangeHandler>() {
+
+ @Override
+ public AsyncServerExchangeHandler get() {
+ return new SingleLineResponseHandler("Hi there");
+ }
+
+ });
+ final InetSocketAddress serverEndpoint = server.start();
+
+ client.start();
+
+ final SessionRequest sessionRequest = client.requestSession(new HttpHost("localhost", serverEndpoint.getPort()), TIMEOUT, null);
+ sessionRequest.setConnectTimeout(TIMEOUT.toMillisIntBound());
+ sessionRequest.waitFor();
+ final IOSession session = sessionRequest.getSession();
+ final ClientSessionEndpoint streamEndpoint = new ClientSessionEndpoint(session);
+
+ final HttpRequest request = new BasicHttpRequest("GET", createRequestURI(serverEndpoint, "/hello"));
+ request.addHeader(HttpHeaders.CONNECTION, HeaderElements.CLOSE);
+ final Future<Message<HttpResponse, String>> future = streamEndpoint.execute(
+ new BasicRequestProducer(request, null),
+ new BasicResponseConsumer<>(new StringAsyncEntityConsumer()), null);
+ try {
+ future.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit());
+ Assert.fail("ExecutionException is expected");
+ } catch (final ExecutionException ex) {
+ Assert.assertThat(ex.getCause(), CoreMatchers.instanceOf(ProtocolException.class));
+ }
+
+ final HttpConnection eventHandler = (HttpConnection) session.getHandler();
+ final EndpointDetails endpointDetails = eventHandler.getEndpointDetails();
+ Assert.assertThat(endpointDetails.getRequestCount(), CoreMatchers.equalTo(0L));
+ }
+
}