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 2020/06/03 13:11:24 UTC

[httpcomponents-core] branch master updated: Improved handling of early response messages by the classic client protocol handler

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


The following commit(s) were added to refs/heads/master by this push:
     new 6637591  Improved handling of early response messages by the classic client protocol handler
6637591 is described below

commit 6637591213221b21a5a473e7092fa2e65d69715c
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Thu May 28 10:28:56 2020 +0200

    Improved handling of early response messages by the classic client protocol handler
---
 .../hc/core5/http/impl/io/BHttpConnectionBase.java |  2 +-
 .../http/impl/io/DefaultBHttpClientConnection.java | 68 +++++++++++++++++++++-
 .../http/impl/io/ResponseOutOfOrderException.java  | 41 +++++++++++++
 .../java/org/apache/hc/core5/util/Timeout.java     |  5 ++
 .../impl/io/TestDefaultBHttpClientConnection.java  |  8 +++
 5 files changed, 120 insertions(+), 4 deletions(-)

diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java
index 4d8a80c..4ffae0e 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java
@@ -294,7 +294,7 @@ class BHttpConnectionBase implements BHttpConnection {
             return true;
         }
         try {
-            final int bytesRead = fillInputBuffer(Timeout.ofMilliseconds(1));
+            final int bytesRead = fillInputBuffer(Timeout.ONE_MILLISECOND);
             return bytesRead < 0;
         } catch (final SocketTimeoutException ex) {
             return false;
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java
index dd5c1e3..6994ec5 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java
@@ -28,11 +28,14 @@
 package org.apache.hc.core5.http.impl.io;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.Socket;
 import java.nio.charset.CharsetDecoder;
 import java.nio.charset.CharsetEncoder;
 
+import javax.net.ssl.SSLSocket;
+
 import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.ContentLengthStrategy;
@@ -52,6 +55,7 @@ import org.apache.hc.core5.http.io.HttpMessageParserFactory;
 import org.apache.hc.core5.http.io.HttpMessageWriter;
 import org.apache.hc.core5.http.io.HttpMessageWriterFactory;
 import org.apache.hc.core5.util.Args;
+import org.apache.hc.core5.util.Timeout;
 
 /**
  * Default implementation of {@link HttpClientConnection}.
@@ -149,8 +153,64 @@ public class DefaultBHttpClientConnection extends BHttpConnectionBase
         if (len == ContentLengthStrategy.UNDEFINED) {
             throw new LengthRequiredException();
         }
-        try (final OutputStream outStream = createContentOutputStream(len, this.outbuffer, socketHolder.getOutputStream(), entity.getTrailers())) {
+
+        try (final OutputStream outStream = createContentOutputStream(
+                len, this.outbuffer, new OutputStream() {
+
+                    final boolean ssl = socketHolder.getSocket() instanceof SSLSocket;
+                    final InputStream socketInputStream = socketHolder.getInputStream();
+                    final OutputStream socketOutputStream = socketHolder.getOutputStream();
+
+                    long totalBytes = 0;
+                    long chunks = -1;
+
+                    void checkForEarlyResponse() throws IOException {
+                        final long n = totalBytes / (8 * 1024);
+                        if (n > chunks) {
+                            chunks = n;
+                            if (ssl ? isDataAvailable(Timeout.ONE_MILLISECOND) : (socketInputStream.available() > 0)) {
+                                throw new ResponseOutOfOrderException();
+                            }
+                        }
+                    }
+
+                    @Override
+                    public void write(final byte[] b) throws IOException {
+                        totalBytes += b.length;
+                        checkForEarlyResponse();
+                        socketOutputStream.write(b);
+                    }
+
+                    @Override
+                    public void write(final byte[] b, final int off, final int len) throws IOException {
+                        totalBytes += len;
+                        checkForEarlyResponse();
+                        socketOutputStream.write(b, off, len);
+                    }
+
+                    @Override
+                    public void write(final int b) throws IOException {
+                        totalBytes++;
+                        checkForEarlyResponse();
+                        socketOutputStream.write(b);
+                    }
+
+                    @Override
+                    public void flush() throws IOException {
+                        socketOutputStream.flush();
+                    }
+
+                    @Override
+                    public void close() throws IOException {
+                        socketOutputStream.close();
+                    }
+
+                }, entity.getTrailers())) {
             entity.writeTo(outStream);
+        } catch (final ResponseOutOfOrderException ex) {
+            if (len > 0) {
+                this.consistent = false;
+            }
         }
     }
 
@@ -169,11 +229,13 @@ public class DefaultBHttpClientConnection extends BHttpConnectionBase
         }
         final long len = this.outgoingContentStrategy.determineLength(request);
         if (len == ContentLengthStrategy.CHUNKED) {
-            try (final OutputStream outStream = createContentOutputStream(len, this.outbuffer, socketHolder.getOutputStream(), entity.getTrailers())) {
+            try (final OutputStream outStream = createContentOutputStream(
+                    len, this.outbuffer, socketHolder.getOutputStream(), entity.getTrailers())) {
                 // just close
             }
         } else if (len >= 0 && len <= 1024) {
-            try (final OutputStream outStream = createContentOutputStream(len, this.outbuffer, socketHolder.getOutputStream(), null)) {
+            try (final OutputStream outStream = createContentOutputStream(
+                    len, this.outbuffer, socketHolder.getOutputStream(), null)) {
                 entity.writeTo(outStream);
             }
         } else {
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/ResponseOutOfOrderException.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/ResponseOutOfOrderException.java
new file mode 100644
index 0000000..1175712
--- /dev/null
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/ResponseOutOfOrderException.java
@@ -0,0 +1,41 @@
+/*
+ * ====================================================================
+ * 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
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.hc.core5.http.impl.io;
+
+import java.io.IOException;
+
+/**
+ * Signals an early (out of order) response.
+ */
+class ResponseOutOfOrderException extends IOException {
+
+    public ResponseOutOfOrderException() {
+        super();
+    }
+
+}
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/util/Timeout.java b/httpcore5/src/main/java/org/apache/hc/core5/util/Timeout.java
index f6a835a..b3be7bf 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/util/Timeout.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/util/Timeout.java
@@ -47,6 +47,11 @@ public class Timeout extends TimeValue {
     public static final Timeout ZERO_MILLISECONDS = Timeout.of(0, TimeUnit.MILLISECONDS);
 
     /**
+     * A one millisecond {@link Timeout}.
+     */
+    public static final Timeout ONE_MILLISECOND = Timeout.of(1, TimeUnit.MILLISECONDS);
+
+    /**
      * A disabled timeout represented as 0 {@code MILLISECONDS}.
      */
     public static final Timeout DISABLED = ZERO_MILLISECONDS;
diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpClientConnection.java b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpClientConnection.java
index 286f949..a69dc22 100644
--- a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpClientConnection.java
+++ b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpClientConnection.java
@@ -232,6 +232,7 @@ public class TestDefaultBHttpClientConnection {
     public void testWriteRequestHead() throws Exception {
         final ByteArrayOutputStream outStream = new ByteArrayOutputStream();
         Mockito.when(socket.getOutputStream()).thenReturn(outStream);
+        Mockito.when(socket.getInputStream()).thenReturn(Mockito.mock(InputStream.class));
 
         conn.bind(socket);
 
@@ -252,6 +253,7 @@ public class TestDefaultBHttpClientConnection {
     public void testWriteRequestEntityWithContentLength() throws Exception {
         final ByteArrayOutputStream outStream = new ByteArrayOutputStream();
         Mockito.when(socket.getOutputStream()).thenReturn(outStream);
+        Mockito.when(socket.getInputStream()).thenReturn(Mockito.mock(InputStream.class));
 
         conn.bind(socket);
 
@@ -275,6 +277,7 @@ public class TestDefaultBHttpClientConnection {
     public void testWriteRequestEntityChunkCoded() throws Exception {
         final ByteArrayOutputStream outStream = new ByteArrayOutputStream();
         Mockito.when(socket.getOutputStream()).thenReturn(outStream);
+        Mockito.when(socket.getInputStream()).thenReturn(Mockito.mock(InputStream.class));
 
         conn.bind(socket);
 
@@ -299,6 +302,7 @@ public class TestDefaultBHttpClientConnection {
     public void testWriteRequestEntityNoContentLength() throws Exception {
         final ByteArrayOutputStream outStream = new ByteArrayOutputStream();
         Mockito.when(socket.getOutputStream()).thenReturn(outStream);
+        Mockito.when(socket.getInputStream()).thenReturn(Mockito.mock(InputStream.class));
 
         conn.bind(socket);
 
@@ -316,6 +320,7 @@ public class TestDefaultBHttpClientConnection {
     public void testWriteRequestNoEntity() throws Exception {
         final ByteArrayOutputStream outStream = new ByteArrayOutputStream();
         Mockito.when(socket.getOutputStream()).thenReturn(outStream);
+        Mockito.when(socket.getInputStream()).thenReturn(Mockito.mock(InputStream.class));
 
         conn.bind(socket);
 
@@ -337,6 +342,7 @@ public class TestDefaultBHttpClientConnection {
     public void testTerminateRequestChunkedEntity() throws Exception {
         final ByteArrayOutputStream outStream = new ByteArrayOutputStream();
         Mockito.when(socket.getOutputStream()).thenReturn(outStream);
+        Mockito.when(socket.getInputStream()).thenReturn(Mockito.mock(InputStream.class));
 
         conn.bind(socket);
 
@@ -364,6 +370,7 @@ public class TestDefaultBHttpClientConnection {
     public void testTerminateRequestContentLengthShort() throws Exception {
         final ByteArrayOutputStream outStream = new ByteArrayOutputStream();
         Mockito.when(socket.getOutputStream()).thenReturn(outStream);
+        Mockito.when(socket.getInputStream()).thenReturn(Mockito.mock(InputStream.class));
 
         conn.bind(socket);
 
@@ -390,6 +397,7 @@ public class TestDefaultBHttpClientConnection {
     public void testTerminateRequestContentLengthLong() throws Exception {
         final ByteArrayOutputStream outStream = new ByteArrayOutputStream();
         Mockito.when(socket.getOutputStream()).thenReturn(outStream);
+        Mockito.when(socket.getInputStream()).thenReturn(Mockito.mock(InputStream.class));
 
         conn.bind(socket);