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 2021/05/11 15:14:55 UTC

[httpcomponents-core] branch master updated: HTTPCORE-677: classic connections to return null message if the underlying connection has been closed by the opposite endpoint instead of throwing an I/O exception.

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 a5bafd2  HTTPCORE-677: classic connections to return null message if the underlying connection has been closed by the opposite endpoint instead of throwing an I/O exception.
a5bafd2 is described below

commit a5bafd2de013d7c81ca05a02c9eb97d612718880
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Mon May 10 22:56:14 2021 +0200

    HTTPCORE-677: classic connections to return null message if the underlying connection has been closed by the opposite endpoint instead of throwing an I/O exception.
---
 .../apache/hc/core5/http/impl/io/AbstractMessageParser.java    | 10 ++++++++--
 .../hc/core5/http/impl/io/DefaultBHttpClientConnection.java    |  4 ++++
 .../hc/core5/http/impl/io/DefaultBHttpServerConnection.java    |  3 +++
 .../apache/hc/core5/http/impl/io/DefaultHttpRequestParser.java |  6 ------
 .../hc/core5/http/impl/io/DefaultHttpResponseParser.java       |  6 ------
 .../java/org/apache/hc/core5/http/impl/io/HttpService.java     |  4 ++++
 .../java/org/apache/hc/core5/http/io/HttpClientConnection.java |  3 ++-
 .../java/org/apache/hc/core5/http/io/HttpMessageParser.java    |  3 ++-
 .../java/org/apache/hc/core5/http/io/HttpServerConnection.java |  3 ++-
 .../org/apache/hc/core5/http/impl/io/TestRequestParser.java    |  6 +++---
 .../org/apache/hc/core5/http/impl/io/TestResponseParser.java   |  6 +++---
 11 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/AbstractMessageParser.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/AbstractMessageParser.java
index 9d487c3..43329b2 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/AbstractMessageParser.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/AbstractMessageParser.java
@@ -32,6 +32,7 @@ import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.hc.core5.http.ConnectionClosedException;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpMessage;
@@ -226,8 +227,13 @@ public abstract class AbstractMessageParser<T extends HttpMessage> implements Ht
      * in case of unexpected connection termination by the peer endpoint.
      *
      * @since 5.0
+     *
+     * @deprecated do not use.
      */
-    protected abstract IOException createConnectionClosedException();
+    @Deprecated
+    protected IOException createConnectionClosedException() {
+        return new ConnectionClosedException();
+    }
 
     @Override
     public T parse(final SessionInputBuffer buffer, final InputStream inputStream) throws IOException, HttpException {
@@ -240,7 +246,7 @@ public abstract class AbstractMessageParser<T extends HttpMessage> implements Ht
                 this.headLine.clear();
                 final int i = buffer.readLine(this.headLine, inputStream);
                 if (i == -1) {
-                    throw createConnectionClosedException();
+                    return null;
                 }
                 if (this.headLine.length() > 0) {
                     this.message = createMessage(this.headLine);
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 9f11a76..78c9aff 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
@@ -45,6 +45,7 @@ import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.HttpVersion;
 import org.apache.hc.core5.http.LengthRequiredException;
+import org.apache.hc.core5.http.NoHttpResponseException;
 import org.apache.hc.core5.http.ProtocolException;
 import org.apache.hc.core5.http.ProtocolVersion;
 import org.apache.hc.core5.http.UnsupportedHttpVersionException;
@@ -296,6 +297,9 @@ public class DefaultBHttpClientConnection extends BHttpConnectionBase
     public ClassicHttpResponse receiveResponseHeader() throws HttpException, IOException {
         final SocketHolder socketHolder = ensureOpen();
         final ClassicHttpResponse response = this.responseParser.parse(this.inBuffer, socketHolder.getInputStream());
+        if (response == null) {
+            throw new NoHttpResponseException("The target server failed to respond");
+        }
         final ProtocolVersion transportVersion = response.getVersion();
         if (transportVersion != null && transportVersion.greaterEquals(HttpVersion.HTTP_2)) {
             throw new UnsupportedHttpVersionException(transportVersion);
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpServerConnection.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpServerConnection.java
index 199a8af..8fe324f 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpServerConnection.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpServerConnection.java
@@ -133,6 +133,9 @@ public class DefaultBHttpServerConnection extends BHttpConnectionBase implements
     public ClassicHttpRequest receiveRequestHeader() throws HttpException, IOException {
         final SocketHolder socketHolder = ensureOpen();
         final ClassicHttpRequest request = this.requestParser.parse(this.inBuffer, socketHolder.getInputStream());
+        if (request == null) {
+            return null;
+        }
         final ProtocolVersion transportVersion = request.getVersion();
         if (transportVersion != null && transportVersion.greaterEquals(HttpVersion.HTTP_2)) {
             throw new UnsupportedHttpVersionException(transportVersion);
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultHttpRequestParser.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultHttpRequestParser.java
index 55e6caf..b5acd53 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultHttpRequestParser.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultHttpRequestParser.java
@@ -31,7 +31,6 @@ import java.io.IOException;
 import java.io.InputStream;
 
 import org.apache.hc.core5.http.ClassicHttpRequest;
-import org.apache.hc.core5.http.ConnectionClosedException;
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpRequestFactory;
 import org.apache.hc.core5.http.MessageConstraintException;
@@ -87,11 +86,6 @@ public class DefaultHttpRequestParser extends AbstractMessageParser<ClassicHttpR
     }
 
     @Override
-    protected IOException createConnectionClosedException() {
-        return new ConnectionClosedException("Client closed connection");
-    }
-
-    @Override
     public ClassicHttpRequest parse(
             final SessionInputBuffer buffer, final InputStream inputStream) throws IOException, HttpException {
         try {
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultHttpResponseParser.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultHttpResponseParser.java
index 4bbf464..c2c4352 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultHttpResponseParser.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultHttpResponseParser.java
@@ -32,7 +32,6 @@ import java.io.IOException;
 import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpResponseFactory;
-import org.apache.hc.core5.http.NoHttpResponseException;
 import org.apache.hc.core5.http.config.Http1Config;
 import org.apache.hc.core5.http.message.LineParser;
 import org.apache.hc.core5.http.message.StatusLine;
@@ -83,11 +82,6 @@ public class DefaultHttpResponseParser extends AbstractMessageParser<ClassicHttp
     }
 
     @Override
-    protected IOException createConnectionClosedException() {
-        return new NoHttpResponseException("The target server failed to respond");
-    }
-
-    @Override
     protected ClassicHttpResponse createMessage(final CharArrayBuffer buffer) throws IOException, HttpException {
         final StatusLine statusline = getLineParser().parseStatusLine(buffer);
         final ClassicHttpResponse response = this.responseFactory.newHttpResponse(statusline.getStatusCode(), statusline.getReasonPhrase());
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpService.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpService.java
index a34ca75..8b27866 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpService.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpService.java
@@ -176,6 +176,10 @@ public class HttpService {
         final AtomicBoolean responseSubmitted = new AtomicBoolean(false);
         try {
             final ClassicHttpRequest request = conn.receiveRequestHeader();
+            if (request == null) {
+                conn.close();
+                return;
+            }
             if (streamListener != null) {
                 streamListener.onRequestHead(conn, request);
             }
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpClientConnection.java b/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpClientConnection.java
index e05d8ab..e8cab9c 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpClientConnection.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpClientConnection.java
@@ -92,7 +92,8 @@ public interface HttpClientConnection extends BHttpConnection {
      * find out if it should try to receive a response entity as well.
      *
      * @return a new HttpResponse object with status line and headers
-     *         initialized.
+     *         initialized or {@code null} if the connection has been closed
+     *         by the opposite endpoint.
      * @throws HttpException in case of HTTP protocol violation
      * @throws IOException in case of an I/O error
      */
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpMessageParser.java b/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpMessageParser.java
index e93a01a..f9909fd 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpMessageParser.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpMessageParser.java
@@ -48,7 +48,8 @@ public interface HttpMessageParser<T extends MessageHeaders> {
      *
      * @param buffer Session input buffer
      * @param inputStream Input stream
-     * @return HTTP message head
+     * @return HTTP message head or {@code null} if the input stream has been
+     * closed by the opposite endpoint.
      * @throws IOException in case of an I/O error
      * @throws HttpException in case of HTTP protocol violation
      */
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpServerConnection.java b/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpServerConnection.java
index 70bc40a..73947eb 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpServerConnection.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/io/HttpServerConnection.java
@@ -47,7 +47,8 @@ public interface HttpServerConnection extends BHttpConnection {
      * request entity as well.
      *
      * @return a new HttpRequest object whose request line and headers are
-     *         initialized.
+     *         initialized or {@code null} if the connection has been closed
+     *         by the opposite endpoint.
      * @throws HttpException in case of HTTP protocol violation
      * @throws IOException in case of an I/O error
      */
diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestRequestParser.java b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestRequestParser.java
index 0fe8a20..10a0614 100644
--- a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestRequestParser.java
+++ b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestRequestParser.java
@@ -31,7 +31,6 @@ import java.io.InterruptedIOException;
 import java.nio.charset.StandardCharsets;
 
 import org.apache.hc.core5.http.ClassicHttpRequest;
-import org.apache.hc.core5.http.ConnectionClosedException;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.Method;
 import org.apache.hc.core5.http.RequestHeaderFieldsTooLargeException;
@@ -65,13 +64,14 @@ public class TestRequestParser {
         Assert.assertEquals(3, headers.length);
     }
 
-    @Test(expected = ConnectionClosedException.class)
+    @Test
     public void testConnectionClosedException() throws Exception {
         final ByteArrayInputStream inputStream = new ByteArrayInputStream(new byte[] {});
         final SessionInputBuffer inBuffer = new SessionInputBufferImpl(16);
 
         final DefaultHttpRequestParser parser = new DefaultHttpRequestParser();
-        parser.parse(inBuffer, inputStream);
+        final ClassicHttpRequest request = parser.parse(inBuffer, inputStream);
+        Assert.assertNull(request);
     }
 
     @Test
diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestResponseParser.java b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestResponseParser.java
index c15ff98..c21e1bf 100644
--- a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestResponseParser.java
+++ b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestResponseParser.java
@@ -34,7 +34,6 @@ import java.nio.charset.StandardCharsets;
 import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.MessageConstraintException;
-import org.apache.hc.core5.http.NoHttpResponseException;
 import org.apache.hc.core5.http.config.Http1Config;
 import org.apache.hc.core5.http.io.SessionInputBuffer;
 import org.junit.Assert;
@@ -65,13 +64,14 @@ public class TestResponseParser {
         Assert.assertEquals(3, headers.length);
     }
 
-    @Test(expected = NoHttpResponseException.class)
+    @Test
     public void testConnectionClosedException() throws Exception {
         final ByteArrayInputStream inputStream = new ByteArrayInputStream(new byte[] {});
         final SessionInputBuffer inBuffer = new SessionInputBufferImpl(16);
 
         final DefaultHttpResponseParser parser = new DefaultHttpResponseParser();
-        parser.parse(inBuffer, inputStream);
+        final ClassicHttpResponse response = parser.parse(inBuffer, inputStream);
+        Assert.assertNull(response);
     }
 
     @Test