fix: (OData) HttpResponse entity buffering (#839)

Author: newtork

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataHealthyResponseValidator.java

@@ -38,13 +38,13 @@ class ODataHealthyResponseValidator
     static void requireHealthyResponse( @Nonnull final ODataRequestResult result )
     {
         final ODataRequestGeneric originalRequest = result.getODataRequest();
-        final HttpResponse httpResponse = result.getHttpResponse();
-        final StatusLine statusLine = httpResponse.getStatusLine();
+        final StatusLine statusLine = result.getStatusLine();
 
         if( statusLine != null && statusLine.getStatusCode() < HttpStatus.SC_BAD_REQUEST ) { // code < 400
             return;
         }
 
+        final HttpResponse httpResponse = result.getHttpResponse();
         final ODataRequestGeneric requestRelevantForException =
             findPotentialBatchItem(httpResponse, originalRequest).getOrElse(originalRequest);
 

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestResult.java

@@ -7,9 +7,11 @@
 import java.util.TreeMap;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 import org.apache.http.Header;
 import org.apache.http.HttpResponse;
+import org.apache.http.StatusLine;
 
 /**
  * Generic type of an OData request result.
@@ -32,6 +34,17 @@ public interface ODataRequestResult
     @Nonnull
     HttpResponse getHttpResponse();
 
+    /**
+     * Get the HTTP response object status line.
+     *
+     * @return the StatusLine.
+     */
+    @Nullable
+    default StatusLine getStatusLine()
+    {
+        return getHttpResponse().getStatusLine();
+    }
+
     /**
      * Get the iterable list of HTTP response header names.
      *

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestResultGeneric.java

@@ -126,6 +126,13 @@ public ODataRequestResultGeneric(
         deserializer = new ODataResponseDeserializer(protocol);
     }
 
+    @Nullable
+    @Override
+    public StatusLine getStatusLine()
+    {
+        return httpResponse.getStatusLine();
+    }
+
     /**
      * Set the default number deserialization strategy for generic JSON numbers without target type mapping.
      *

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestResultMultipartGeneric.java

@@ -8,6 +8,7 @@
 
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
+import org.apache.http.StatusLine;
 import org.apache.http.util.EntityUtils;
 
 import com.sap.cloud.sdk.datamodel.odata.client.exception.ODataDeserializationException;
@@ -61,6 +62,13 @@ public class ODataRequestResultMultipartGeneric
         this.httpResponse = httpResponse;
     }
 
+    @Nullable
+    @Override
+    public StatusLine getStatusLine()
+    {
+        return httpResponse.getStatusLine();
+    }
+
     /**
      * Get the original {@link ODataRequestBatch batch request} that was used for running the OData request.
      *

datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataHealthyResponseValidatorTest.java

@@ -1,19 +1,15 @@
 package com.sap.cloud.sdk.datamodel.odata.client.request;
 
+import static org.apache.http.HttpVersion.HTTP_1_1;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.mockito.Mockito.lenient;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 
 import java.nio.charset.StandardCharsets;
 
-import org.apache.http.Header;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
-import org.apache.http.StatusLine;
 import org.apache.http.entity.StringEntity;
-import org.junit.jupiter.api.BeforeEach;
+import org.apache.http.message.BasicHttpResponse;
 import org.junit.jupiter.api.Test;
 
 import com.sap.cloud.sdk.datamodel.odata.client.ODataProtocol;
@@ -22,32 +18,23 @@
 
 class ODataHealthyResponseValidatorTest
 {
-    private final ODataRequestResult odataResult = mock(ODataRequestResult.class);
-    private final ODataRequestGeneric odataRequest = mock(ODataRequestGeneric.class);
-    private final HttpResponse httpResponse = mock(HttpResponse.class);
-    private final StatusLine httpResponseStatusLine = mock(StatusLine.class);
-
-    @BeforeEach
-    void adjustMocks()
-    {
-        lenient().when(odataRequest.getProtocol()).thenReturn(ODataProtocol.V2);
-        lenient().when(odataResult.getHttpResponse()).thenReturn(httpResponse);
-        lenient().when(odataResult.getODataRequest()).thenReturn(odataRequest);
-        lenient().when(httpResponse.getStatusLine()).thenReturn(httpResponseStatusLine);
-        lenient().when(httpResponse.getAllHeaders()).thenReturn(new Header[0]);
-        lenient().when(httpResponseStatusLine.getStatusCode()).thenReturn(HttpStatus.SC_OK);
-    }
+    private static final ODataRequestGeneric REQUEST =
+        new ODataRequestRead("service-path", "EntitySet", null, ODataProtocol.V2);
 
     @Test
     void testSuccess()
     {
+        final HttpResponse httpResponse = new BasicHttpResponse(HTTP_1_1, HttpStatus.SC_OK, "OK");
+        final ODataRequestResult odataResult = new ODataRequestResultGeneric(REQUEST, httpResponse);
+
         ODataHealthyResponseValidator.requireHealthyResponse(odataResult);
     }
 
     @Test
     void testNotFound()
     {
-        when(httpResponseStatusLine.getStatusCode()).thenReturn(HttpStatus.SC_NOT_FOUND);
+        final HttpResponse httpResponse = new BasicHttpResponse(HTTP_1_1, HttpStatus.SC_NOT_FOUND, "Not Found");
+        final ODataRequestResult odataResult = new ODataRequestResultGeneric(REQUEST, httpResponse);
 
         assertThatExceptionOfType(ODataResponseException.class)
             .isThrownBy(() -> ODataHealthyResponseValidator.requireHealthyResponse(odataResult))
@@ -78,8 +65,9 @@ void testODataError()
             }
             """;
 
-        when(httpResponseStatusLine.getStatusCode()).thenReturn(HttpStatus.SC_INTERNAL_SERVER_ERROR);
-        when(httpResponse.getEntity()).thenReturn(new StringEntity(odata_error_json, StandardCharsets.UTF_8));
+        final HttpResponse httpResponse = new BasicHttpResponse(HTTP_1_1, HttpStatus.SC_INTERNAL_SERVER_ERROR, "Oh!");
+        httpResponse.setEntity(new StringEntity(odata_error_json, StandardCharsets.UTF_8));
+        final ODataRequestResult odataResult = new ODataRequestResultGeneric(REQUEST, httpResponse);
 
         assertThatExceptionOfType(ODataServiceErrorException.class)
             .isThrownBy(() -> ODataHealthyResponseValidator.requireHealthyResponse(odataResult))

datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestResultGenericTest.java

@@ -11,7 +11,10 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
 import java.net.SocketException;
+import java.nio.charset.StandardCharsets;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
@@ -20,6 +23,7 @@
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.HttpClient;
+import org.apache.http.entity.InputStreamEntity;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.message.BasicHeader;
 import org.apache.http.message.BasicHttpResponse;
@@ -162,4 +166,35 @@ void ensureNoRedundantHeadersForPaginatedRequests()
         final Map<String, Collection<String>> lastRequestHeaders = nextResult.getODataRequest().getHeaders();
         assertThat(lastRequestHeaders).containsExactly(entry("Accept", Collections.singletonList("application/json")));
     }
+
+    @Test
+    @SneakyThrows
+    void testDisabledBuffer()
+    {
+        // test setup for request
+        final ODataRequestGeneric oDataRequest =
+            new ODataRequestRead("generic/service/path", "entity(123)", null, ODataProtocol.V4);
+
+        // test setup for streamed http response
+        final BasicHttpResponse httpResponse = new BasicHttpResponse(HTTP_1_1, 200, "OK");
+        final String json = "{\"value\":[]}";
+        final InputStream inputStream = new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8));
+        httpResponse.setEntity(new InputStreamEntity(inputStream, json.length()));
+
+        // system under test
+        final ODataRequestResultGeneric testResult = new ODataRequestResultGeneric(oDataRequest, httpResponse);
+        testResult.disableBufferingHttpResponse();
+
+        // sanity checks do not consume the response
+        assertThat(testResult.getHeaderValues("Content-Length")).isEmpty();
+        assertThat(testResult.getHttpResponse().getStatusLine().getStatusCode()).isEqualTo(200);
+
+        // true-positive, successfully read once
+        assertThat(testResult.asListOfMaps()).isEmpty();
+
+        // true-negative, no second read possible
+        assertThatThrownBy(testResult::asListOfMaps)
+            .isInstanceOf(ODataDeserializationException.class)
+            .hasMessageContaining("Unable to read OData 4.0 response.");
+    }
 }

release_notes.md

@@ -21,3 +21,4 @@
 ### 🐛 Fixed Issues
 
 - Fix `CVE-2025-48734` by transitive dependency update in `connectivity-ztis`.
+- For OData Generic Client: Fix `disableBufferingHttpResponse()` in `ODataRequestResultGeneric`.