Storage Content Validation - Audit DecodedResponse (#49147)

* Audit DecodedResponse: tighten override surface, pin UTF-8, add unit tests

* Reduce DecodedResponse to minimal override surface

Strip overrides and tests that aren't required for transparent HttpResponse behavior, leaving exactly the compiler-required abstract methods plus one discretionary override that fixes a proven base-class bug.

DecodedResponse.java:
- Remove close() override. Current consumers fully drain the body Flux, so Reactor's onComplete signals release the underlying transport; the explicit close-forwarding was defensive against a try-with-resources / status-only pattern that no caller in the codebase actually uses.
- Collapse no-arg getBodyAsString() to delegate to the charset overload, pinning UTF-8 in one place instead of duplicating the lambda.
- Final override surface (8 methods): the 7 abstract methods on HttpResponse in azure-core 1.57.1 (compiler-required) plus getBodyAsBinaryData, which is concrete in the base but seeds BinaryData with the wire Content-Length header, making BinaryData.getLength() return the encoded size (frames + CRC trailers) instead of the decoded payload size.

DecodedResponseTests.java:
- Remove closeDelegatesToWrappedResponse and closeDoesNotSubscribeToDecodedBody (no override left to validate).
- Drop the close-counting assertion from the writeBodyTo test and rename it to inheritedWriteBodyToWritesDecodedBytes; it now uses MockHttpResponse directly.
- Strengthen getBodyAsBinaryDataReportsDecodedSizeNotContentLength: assert data.getLength() equals the decoded size (post-consumption). Without the override this returns 71 instead of 7, empirically reproduced.
- Drop now-unused trackingResponse helper, AtomicInteger import.

Tests: 10 of 10 passing. Each test maps 1:1 to one override or one inherited integration; no test is redundant.

* some code cleanup

* Address Copilot PR #49147 review comments

DecodedResponse.java (comment r3220562536):
- Restore HttpResponse#getBodyAsString() base contract by switching from unconditional UTF-8 to CoreUtils.bomAwareToString(bytes, contentType), matching BufferedHttpResponse's idiom. The previous UTF-8 pin silently dropped BOM detection and Content-Type charset honoring, which callers viewing this as a generic HttpResponse expect.

DecodedResponseTests.java (comment r3220562553):
- Add inheritedGetBodyAsBinaryDataReturnsDecodedBytes: exercises the inherited HttpResponse#getBodyAsBinaryData() and asserts the bytes match the decoded payload. Uses a divergent Content-Length header to make the wire vs decoded distinction explicit and guard against header-forwarding regressions.
- Add getBodyAsStringHonorsCharsetFromContentTypeHeader: proves the bom-aware fix actually honors a charset declared in Content-Type (would fail if the previous UTF-8 pinning were still in place).
- Add getBodyAsStringDetectsUtf8BomAndStripsIt: pins the BOM-detection arm of CoreUtils.bomAwareToString.
- Update getBodyAsStringDefaultsToUtf8WhenNoCharsetSpecified docstring to describe the new bom-aware fallback semantics rather than the old unconditional UTF-8 behavior.

Tests: 12 of 12 passing.

* some code cleanup

* analyze error

---------

Co-authored-by: Isabelle <ibrandes@microsoft.com>

Author: gunjansingh-msft

sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/policy/DecodedResponse.java

@@ -3,8 +3,10 @@
 
 package com.azure.storage.common.policy;
 
+import com.azure.core.http.HttpHeaderName;
 import com.azure.core.http.HttpHeaders;
 import com.azure.core.http.HttpResponse;
+import com.azure.core.util.CoreUtils;
 import com.azure.core.util.FluxUtil;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
@@ -24,8 +26,6 @@
 class DecodedResponse extends HttpResponse {
     private final HttpResponse originalResponse;
     private final Flux<ByteBuffer> decodedBody;
-    private final HttpHeaders httpHeaders;
-    private final int statusCode;
 
     /**
      * Wraps {@code httpResponse} with a body backed by {@code decodedBody}.
@@ -36,28 +36,25 @@ class DecodedResponse extends HttpResponse {
      * pipeline.
      */
     DecodedResponse(HttpResponse httpResponse, Flux<ByteBuffer> decodedBody) {
-        // Preserve the original request so retry policies, response models, and logging keep their reference chain
-        // intact.
         super(httpResponse.getRequest());
         this.originalResponse = httpResponse;
         this.decodedBody = decodedBody;
-        this.statusCode = httpResponse.getStatusCode();
-        this.httpHeaders = httpResponse.getHeaders();
     }
 
     @Override
     public int getStatusCode() {
-        return statusCode;
+        return originalResponse.getStatusCode();
     }
 
     @Override
+    @SuppressWarnings("deprecation")
     public String getHeaderValue(String name) {
-        return httpHeaders.getValue(name);
+        return originalResponse.getHeaderValue(name);
     }
 
     @Override
     public HttpHeaders getHeaders() {
-        return httpHeaders;
+        return originalResponse.getHeaders();
     }
 
     @Override
@@ -72,7 +69,8 @@ public Mono<byte[]> getBodyAsByteArray() {
 
     @Override
     public Mono<String> getBodyAsString() {
-        return FluxUtil.collectBytesInByteBufferStream(decodedBody).map(String::new);
+        return getBodyAsByteArray()
+            .map(b -> CoreUtils.bomAwareToString(b, originalResponse.getHeaderValue(HttpHeaderName.CONTENT_TYPE)));
     }
 
     @Override

sdk/storage/azure-storage-common/src/test/java/com/azure/storage/common/policy/DecodedResponseTests.java

@@ -0,0 +1,222 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.azure.storage.common.policy;
+
+import com.azure.core.http.HttpHeaderName;
+import com.azure.core.http.HttpHeaders;
+import com.azure.core.http.HttpMethod;
+import com.azure.core.http.HttpRequest;
+import com.azure.core.http.HttpResponse;
+import com.azure.core.test.http.MockHttpResponse;
+import com.azure.core.util.BinaryData;
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Flux;
+import reactor.test.StepVerifier;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.nio.ByteBuffer;
+import java.nio.channels.Channels;
+import java.nio.channels.WritableByteChannel;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+public class DecodedResponseTests {
+
+    private static final HttpHeaderName CUSTOM_HEADER = HttpHeaderName.fromString("x-ms-custom");
+
+    private static HttpRequest newRequest() {
+        try {
+            return new HttpRequest(HttpMethod.GET, new URL("http://example.com/"));
+        } catch (MalformedURLException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    private static HttpHeaders headers(HttpHeaderName name, String value) {
+        return new HttpHeaders().set(name, value);
+    }
+
+    private static MockHttpResponse mockResponse(int status, HttpHeaders headers, byte[] body) {
+        return new MockHttpResponse(newRequest(), status, headers, body);
+    }
+
+    private static byte[] bytes(String s) {
+        return s.getBytes(StandardCharsets.UTF_8);
+    }
+
+    private static Flux<ByteBuffer> fluxOf(byte[] data) {
+        return Flux.just(ByteBuffer.wrap(data));
+    }
+
+    @Test
+    public void preservesRequestStatusCodeAndHeaders() {
+        HttpHeaders h = new HttpHeaders().set(HttpHeaderName.CONTENT_LENGTH, "100").set(CUSTOM_HEADER, "value");
+        MockHttpResponse original = mockResponse(206, h, bytes("encoded"));
+
+        DecodedResponse wrapper = new DecodedResponse(original, fluxOf(bytes("decoded")));
+
+        assertSame(original.getRequest(), wrapper.getRequest());
+        assertEquals(206, wrapper.getStatusCode());
+        assertSame(h, wrapper.getHeaders());
+    }
+
+    @Test
+    public void getHeaderValueByStringReturnsHeaderValue() {
+        HttpHeaders h = headers(CUSTOM_HEADER, "value");
+        DecodedResponse wrapper = new DecodedResponse(mockResponse(200, h, new byte[0]), fluxOf(new byte[0]));
+
+        assertEquals("value", wrapper.getHeaderValue(CUSTOM_HEADER.getCaseInsensitiveName()));
+        assertNull(wrapper.getHeaderValue("nonexistent"));
+    }
+
+    @Test
+    public void getBodyReturnsDecodedFlux() {
+        byte[] decoded = bytes("decoded body");
+        DecodedResponse wrapper
+            = new DecodedResponse(mockResponse(200, new HttpHeaders(), bytes("encoded")), fluxOf(decoded));
+
+        StepVerifier.create(wrapper.getBody().reduce(new ByteArrayOutputStream(), (sink, buf) -> {
+            byte[] copy = new byte[buf.remaining()];
+            buf.get(copy);
+            sink.write(copy, 0, copy.length);
+            return sink;
+        })).expectNextMatches(sink -> Arrays.equals(decoded, sink.toByteArray())).verifyComplete();
+    }
+
+    @Test
+    public void getBodyAsByteArrayReturnsDecodedBytes() {
+        byte[] decoded = bytes("decoded body");
+        DecodedResponse wrapper
+            = new DecodedResponse(mockResponse(200, new HttpHeaders(), bytes("encoded")), fluxOf(decoded));
+
+        StepVerifier.create(wrapper.getBodyAsByteArray())
+            .expectNextMatches(b -> Arrays.equals(decoded, b))
+            .verifyComplete();
+    }
+
+    @Test
+    public void getBodyAsStringDefaultsToUtf8WhenNoCharsetSpecified() {
+        // The no-arg overload routes through CoreUtils.bomAwareToString, which falls back to UTF-8 when neither a
+        // BOM nor a Content-Type charset parameter is present. This test pins the "no headers, no BOM" path.
+        String text = "héllo wörld – ✓";
+        DecodedResponse wrapper = new DecodedResponse(mockResponse(200, new HttpHeaders(), new byte[0]),
+            fluxOf(text.getBytes(StandardCharsets.UTF_8)));
+
+        StepVerifier.create(wrapper.getBodyAsString()).expectNext(text).verifyComplete();
+    }
+
+    @Test
+    public void getBodyAsStringHonorsCharsetFromContentTypeHeader() {
+        // Per the base HttpResponse contract, the no-arg getBodyAsString() must honor a charset declared in the
+        // response's Content-Type header. Without the bom-aware decoding the bytes would be (mis)interpreted as
+        // UTF-8 and the assertion below would fail.
+        String text = "ümlaut";
+        byte[] iso = text.getBytes(StandardCharsets.ISO_8859_1);
+        HttpHeaders h = new HttpHeaders().set(HttpHeaderName.CONTENT_TYPE, "text/plain; charset=ISO-8859-1");
+        DecodedResponse wrapper = new DecodedResponse(mockResponse(200, h, new byte[0]), fluxOf(iso));
+
+        StepVerifier.create(wrapper.getBodyAsString()).expectNext(text).verifyComplete();
+    }
+
+    @Test
+    public void getBodyAsStringDetectsUtf8BomAndStripsIt() {
+        // A leading UTF-8 BOM (EF BB BF) must be detected and stripped from the decoded string, matching the base
+        // HttpResponse contract that CoreUtils.bomAwareToString implements.
+        String text = "with bom";
+        byte[] bom = new byte[] { (byte) 0xEF, (byte) 0xBB, (byte) 0xBF };
+        byte[] payload = text.getBytes(StandardCharsets.UTF_8);
+        byte[] withBom = new byte[bom.length + payload.length];
+        System.arraycopy(bom, 0, withBom, 0, bom.length);
+        System.arraycopy(payload, 0, withBom, bom.length, payload.length);
+        DecodedResponse wrapper
+            = new DecodedResponse(mockResponse(200, new HttpHeaders(), new byte[0]), fluxOf(withBom));
+
+        StepVerifier.create(wrapper.getBodyAsString()).expectNext(text).verifyComplete();
+    }
+
+    @Test
+    public void getBodyAsStringDecodesUsingProvidedCharset() {
+        String text = "ümlaut";
+        byte[] latin1 = text.getBytes(StandardCharsets.ISO_8859_1);
+        DecodedResponse wrapper
+            = new DecodedResponse(mockResponse(200, new HttpHeaders(), new byte[0]), fluxOf(latin1));
+
+        StepVerifier.create(wrapper.getBodyAsString(StandardCharsets.ISO_8859_1)).expectNext(text).verifyComplete();
+    }
+
+    @Test
+    public void inheritedGetBodyAsInputStreamUsesDecodedBytes() throws IOException {
+        // Base getBodyAsInputStream() routes through getBodyAsByteArray(), so the override is exercised end-to-end.
+        byte[] decoded = bytes("decoded stream");
+        DecodedResponse wrapper
+            = new DecodedResponse(mockResponse(200, new HttpHeaders(), bytes("encoded")), fluxOf(decoded));
+
+        try (InputStream stream = wrapper.getBodyAsInputStream().block()) {
+            assertNotNull(stream);
+            assertArrayEquals(decoded, readAll(stream));
+        }
+    }
+
+    @Test
+    public void inheritedWriteBodyToWritesDecodedBytes() throws IOException {
+        byte[] decoded = bytes("write me");
+        DecodedResponse wrapper
+            = new DecodedResponse(mockResponse(200, new HttpHeaders(), bytes("encoded")), fluxOf(decoded));
+
+        ByteArrayOutputStream sink = new ByteArrayOutputStream();
+        try (WritableByteChannel channel = Channels.newChannel(sink)) {
+            wrapper.writeBodyTo(channel);
+        }
+
+        assertArrayEquals(decoded, sink.toByteArray());
+    }
+
+    @Test
+    public void inheritedBufferReturnsResponseBackedByDecodedBytes() {
+        byte[] decoded = bytes("buffered");
+        DecodedResponse wrapper
+            = new DecodedResponse(mockResponse(200, new HttpHeaders(), bytes("encoded")), fluxOf(decoded));
+
+        HttpResponse buffered = wrapper.buffer();
+        assertNotNull(buffered);
+        StepVerifier.create(buffered.getBodyAsByteArray())
+            .expectNextMatches(b -> Arrays.equals(decoded, b))
+            .verifyComplete();
+    }
+
+    @Test
+    public void inheritedGetBodyAsBinaryDataReturnsDecodedBytes() {
+        // Base HttpResponse#getBodyAsBinaryData() pulls from getBody() (our override), so the resulting BinaryData
+        // must contain the decoded payload, not the original wire body. A divergent Content-Length header is set
+        // to make the wire vs decoded distinction explicit and guard against regressions in header forwarding.
+        byte[] decoded = bytes("decoded payload");
+        HttpHeaders h = new HttpHeaders().set(HttpHeaderName.CONTENT_LENGTH, String.valueOf(decoded.length + 32));
+        DecodedResponse wrapper
+            = new DecodedResponse(mockResponse(200, h, bytes("encoded wire body")), fluxOf(decoded));
+
+        BinaryData data = wrapper.getBodyAsBinaryData();
+        assertNotNull(data);
+        assertArrayEquals(decoded, data.toBytes());
+    }
+
+    private static byte[] readAll(InputStream stream) throws IOException {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        byte[] buf = new byte[1024];
+        int n;
+        while ((n = stream.read(buf)) != -1) {
+            out.write(buf, 0, n);
+        }
+        return out.toByteArray();
+    }
+}