From b672731fc918f42291a09464dff28e211afa9d6d Mon Sep 17 00:00:00 2001
From: Jens Wille <jens.wille@gmail.com>
Date: Tue, 6 Sep 2022 19:11:20 +0200
Subject: [PATCH] Fix error stream handling for `HttpOpener`. (#463)

Only read `errorStream` _after_ reading `inputStream` failed. (Thanks to @dr0i for the hint!)

Drops use of response code range to determine failure handling. (864f0da)
---
 .../java/org/metafacture/io/HttpOpener.java   | 30 +++++--------------
 .../org/metafacture/io/HttpOpenerTest.java    |  8 ++---
 2 files changed, 10 insertions(+), 28 deletions(-)

diff --git a/metafacture-io/src/main/java/org/metafacture/io/HttpOpener.java b/metafacture-io/src/main/java/org/metafacture/io/HttpOpener.java
index 1f098f4b..5de3724b 100644
--- a/metafacture-io/src/main/java/org/metafacture/io/HttpOpener.java
+++ b/metafacture-io/src/main/java/org/metafacture/io/HttpOpener.java
@@ -67,9 +67,6 @@ public final class HttpOpener extends DefaultObjectPipe<String, ObjectReceiver<R
     private static final Pattern HEADER_FIELD_SEPARATOR_PATTERN = Pattern.compile(HEADER_FIELD_SEPARATOR);
     private static final Pattern HEADER_VALUE_SEPARATOR_PATTERN = Pattern.compile(HEADER_VALUE_SEPARATOR);
 
-    private static final int SUCCESS_CODE_MIN = 200;
-    private static final int SUCCESS_CODE_MAX = 399;
-
     private final Map<String, String> headers = new HashMap<>();
 
     private Method method;
@@ -260,11 +257,9 @@ public void process(final String input) {
                 connection.getOutputStream().write(requestBody.getBytes());
             }
 
-            final InputStream errorStream = connection.getErrorStream();
-            final InputStream inputStream = errorStream != null ?
-                getErrorStream(errorStream) : getInputStream(connection);
-
+            final InputStream inputStream = getInputStream(connection);
             final String contentEncoding = getEncoding(connection.getContentEncoding());
+
             getReceiver().process(new InputStreamReader(inputStream, contentEncoding));
         }
         catch (final IOException e) {
@@ -294,30 +289,19 @@ private InputStream getInputStream(final HttpURLConnection connection) throws IO
             return connection.getInputStream();
         }
         catch (final IOException e) {
-            final int responseCode = connection.getResponseCode();
-            if (responseCode >= SUCCESS_CODE_MIN && responseCode <= SUCCESS_CODE_MAX) {
-                throw e;
+            final InputStream errorStream = connection.getErrorStream();
+            if (errorStream != null) {
+                return getErrorStream(errorStream);
             }
             else {
-                final StringBuilder sb = new StringBuilder(String.valueOf(responseCode));
-
-                final String responseMessage = connection.getResponseMessage();
-                if (responseMessage != null) {
-                    sb.append(" - ").append(responseMessage);
-                }
-
-                return getErrorStream(getInputStream(sb.toString()));
+                throw e;
             }
         }
     }
 
-    private InputStream getInputStream(final String string) {
-        return new ByteArrayInputStream(string.getBytes());
-    }
-
     private InputStream getErrorStream(final InputStream errorStream) {
         if (errorPrefix != null) {
-            final InputStream errorPrefixStream = getInputStream(errorPrefix);
+            final InputStream errorPrefixStream = new ByteArrayInputStream(errorPrefix.getBytes());
             return new SequenceInputStream(errorPrefixStream, errorStream);
         }
         else {
diff --git a/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java b/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java
index 9d380200..f86cb19c 100644
--- a/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java
+++ b/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java
@@ -58,8 +58,6 @@ public final class HttpOpenerTest {
     private static final String TEST_STRING = "test string";
     private static final StringValuePattern TEST_VALUE = WireMock.equalTo(TEST_STRING);
 
-    private static final String TEST_ERROR = "400 - Bad Request";
-
     private static final String REQUEST_BODY = "request body";
     private static final String RESPONSE_BODY = "response bödy"; // UTF-8
 
@@ -263,19 +261,19 @@ public void shouldPerformPostRequestWithEncodingParameterAndContentEncodingRespo
     @Test
     public void shouldPerformGetRequestWithErrorResponse() throws IOException {
         shouldPerformRequest(TEST_URL, HttpOpener.Method.GET, (o, u) -> {},
-                null, null, WireMock.badRequest().withBody(RESPONSE_BODY), "ERROR: " + TEST_ERROR);
+                null, null, WireMock.badRequest().withBody(RESPONSE_BODY), "ERROR: " + RESPONSE_BODY);
     }
 
     @Test
     public void shouldPerformGetRequestWithErrorResponseAndErrorPrefixParameter() throws IOException {
         shouldPerformRequest(TEST_URL, HttpOpener.Method.GET, (o, u) -> o.setErrorPrefix(TEST_STRING),
-                null, null, WireMock.badRequest().withBody(RESPONSE_BODY), TEST_STRING + TEST_ERROR);
+                null, null, WireMock.badRequest().withBody(RESPONSE_BODY), TEST_STRING + RESPONSE_BODY);
     }
 
     @Test
     public void shouldPerformGetRequestWithErrorResponseAndWithoutErrorPrefixParameter() throws IOException {
         shouldPerformRequest(TEST_URL, HttpOpener.Method.GET, (o, u) -> o.setErrorPrefix(null),
-                null, null, WireMock.badRequest().withBody(RESPONSE_BODY), TEST_ERROR);
+                null, null, WireMock.badRequest().withBody(RESPONSE_BODY), RESPONSE_BODY);
     }
 
     private void shouldPerformRequest(final String input, final HttpOpener.Method method, final BiConsumer<HttpOpener, String> consumer, final String... headers) throws IOException {