From c1b019dfa10125b8e665b6044f77c3f2d65f075d Mon Sep 17 00:00:00 2001 From: jencymaryjoseph <35571282+jencymaryjoseph@users.noreply.github.com> Date: Fri, 28 Aug 2026 12:53:17 -0700 Subject: [PATCH 1/4] fix: prevent concatenated gzip response truncation when read via GZIPInputStream --- .../bugfix-AWSSDKforJavav2-c8ce5ba.json | 6 + .../awssdk/core/ResponseInputStream.java | 5 +- .../io/GzipAvailabilityInputStream.java | 164 +++++++++ .../awssdk/core/ResponseInputStreamTest.java | 261 ++++++++++++++ .../io/GzipAvailabilityInputStreamTest.java | 327 ++++++++++++++++++ 5 files changed, 761 insertions(+), 2 deletions(-) create mode 100644 .changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json create mode 100644 core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStream.java create mode 100644 core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStreamTest.java diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json b/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json new file mode 100644 index 000000000000..69ccf94f9839 --- /dev/null +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "AWS SDK for Java v2", + "contributor": "", + "description": "Fixed an issue where concatenated gzip response streams could be truncated to the first member when decoded with GZIPInputStream. A transient available()==0 at a gzip member boundary was treated as end of stream; ResponseInputStream now reports available() as at least 1 for gzip content while the stream is open." +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseInputStream.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseInputStream.java index 8f87186d0edd..f517f7993118 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseInputStream.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseInputStream.java @@ -24,6 +24,7 @@ import java.util.concurrent.TimeUnit; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.annotations.SdkTestInternalApi; +import software.amazon.awssdk.core.internal.io.GzipAvailabilityInputStream; import software.amazon.awssdk.core.io.SdkFilterInputStream; import software.amazon.awssdk.http.Abortable; import software.amazon.awssdk.http.AbortableInputStream; @@ -72,7 +73,7 @@ public ResponseInputStream(ResponseT resp, AbortableInputStream in) { } public ResponseInputStream(ResponseT resp, AbortableInputStream in, Duration timeout) { - super(in); + super(new GzipAvailabilityInputStream(in)); this.response = Validate.paramNotNull(resp, "response"); this.abortable = Validate.paramNotNull(in, "abortableInputStream"); @@ -85,7 +86,7 @@ public ResponseInputStream(ResponseT resp, InputStream in) { } public ResponseInputStream(ResponseT resp, InputStream in, Duration timeout) { - super(in); + super(new GzipAvailabilityInputStream(in)); this.response = Validate.paramNotNull(resp, "response"); this.abortable = in instanceof Abortable ? (Abortable) in : null; diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStream.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStream.java new file mode 100644 index 000000000000..b1b5b5cc6716 --- /dev/null +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStream.java @@ -0,0 +1,164 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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. + */ + +package software.amazon.awssdk.core.internal.io; + +import java.io.FilterInputStream; +import java.io.IOException; +import java.io.InputStream; +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.utils.IoUtils; + +/** + * Wraps a response body so {@code available()} never returns {@code 0} for gzip content while the stream is open. + * {@link java.util.zip.GZIPInputStream} treats a transient {@code 0} from {@code available()} at a member boundary + * as end of stream and stops, truncating concatenated gzip. Gzip is detected passively from the leading bytes + * ({@code 1f 8b 08}); non-gzip streams keep honest {@code available()}. + */ +@SdkInternalApi +public final class GzipAvailabilityInputStream extends FilterInputStream implements Releasable { + + private static final int GZIP_MAGIC_1 = 0x1f; + private static final int GZIP_MAGIC_2 = 0x8b; + private static final int GZIP_METHOD_DEFLATE = 0x08; + private static final int HEADER_LENGTH = 3; + + private final byte[] header = new byte[HEADER_LENGTH]; + private volatile int headerLen; + private volatile boolean classified; + private volatile boolean gzipDetected; + private volatile boolean eof; + private volatile boolean closed; + + private int markHeaderLen; + private boolean markClassified; + private boolean markGzipDetected; + private boolean markEof; + private boolean marked; + + public GzipAvailabilityInputStream(InputStream in) { + super(in); + } + + @Override + public int read() throws IOException { + int b = in.read(); + if (b == -1) { + eof = true; + } else { + if (eof) { + eof = false; + } + observe((byte) b); + } + return b; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + int n = in.read(b, off, len); + if (n == -1) { + eof = true; + } else if (n > 0) { + if (eof) { + eof = false; + } + observe(b, off, n); + } + return n; + } + + @Override + public int available() throws IOException { + if (closed) { + return 0; + } + int available = in.available(); + return available == 0 && gzipDetected && !eof ? 1 : available; + } + + @Override + public long skip(long n) throws IOException { + long skipped = in.skip(n); + if (skipped > 0) { + classified = true; + } + return skipped; + } + + @Override + public synchronized void mark(int readlimit) { + markHeaderLen = headerLen; + markClassified = classified; + markGzipDetected = gzipDetected; + markEof = eof; + marked = true; + in.mark(readlimit); + } + + @Override + public synchronized void reset() throws IOException { + in.reset(); + if (marked) { + headerLen = markHeaderLen; + classified = markClassified; + gzipDetected = markGzipDetected; + eof = markEof; + } else { + headerLen = 0; + classified = false; + gzipDetected = false; + eof = false; + } + } + + @Override + public void close() throws IOException { + closed = true; + in.close(); + } + + @Override + public void release() { + IoUtils.closeQuietly(this, null); + if (in instanceof Releasable) { + ((Releasable) in).release(); + } + } + + private void observe(byte[] b, int off, int len) { + if (classified) { + return; + } + for (int i = 0; i < len && !classified; i++) { + observe(b[off + i]); + } + } + + private void observe(byte b) { + if (classified) { + return; + } + int len = headerLen; + header[len] = b; + headerLen = len + 1; + if (headerLen == HEADER_LENGTH) { + classified = true; + gzipDetected = (header[0] & 0xff) == GZIP_MAGIC_1 + && (header[1] & 0xff) == GZIP_MAGIC_2 + && (header[2] & 0xff) == GZIP_METHOD_DEFLATE; + } + } +} diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseInputStreamTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseInputStreamTest.java index 6710465a899d..75c0617cc707 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseInputStreamTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseInputStreamTest.java @@ -17,12 +17,23 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -133,7 +144,257 @@ void negativeTimeout_disablesTimeout() throws Exception { assertThat(responseInputStream.hasTimeoutTask()).isFalse(); } + @Test + void gzipConcatenatedMembers_whenAvailableTransientlyZero_decodesAllMembers() throws IOException { + InputStream underlying = new TrickleStream(concatenatedGzip("PART_ONE;", "PART_TWO;"), false); + ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); + + assertThat(readAllGzip(ris)).isEqualTo("PART_ONE;PART_TWO;"); + } + + @Test + void gzipManyMembers_whenAvailableTransientlyZero_decodesAllMembers() throws IOException { + InputStream underlying = new TrickleStream(concatenatedGzip("A;", "B;", "C;", "D;", "E;"), false); + ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); + + assertThat(readAllGzip(ris)).isEqualTo("A;B;C;D;E;"); + } + + @Test + void gzipSingleMember_whenAvailableZero_decodesWithoutHanging() { + // Preemptive timeout guards against the final-boundary probe hanging. + String decoded = assertTimeoutPreemptively(Duration.ofSeconds(5), () -> { + InputStream underlying = new TrickleStream(concatenatedGzip("ONLY_ONE_MEMBER;"), false); + ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); + return readAllGzip(ris); + }); + + assertThat(decoded).isEqualTo("ONLY_ONE_MEMBER;"); + } + + @Test + void gzipConcatenatedMembers_whenNeverZeroAvailable_decodesAllMembers() throws IOException { + InputStream underlying = new TrickleStream(concatenatedGzip("PART_ONE;", "PART_TWO;"), true); + ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); + + assertThat(readAllGzip(ris)).isEqualTo("PART_ONE;PART_TWO;"); + } + + @Test + void bufferedReader_whenNonGzip_deliversLineWithoutBlocking() { + ControllableStream underlying = new ControllableStream(); + underlying.feed("event: E1\n"); + ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); + BufferedReader reader = new BufferedReader(new InputStreamReader(ris, StandardCharsets.UTF_8)); + + assertTimeoutPreemptively(Duration.ofSeconds(2), () -> + assertThat(reader.readLine()).isEqualTo("event: E1")); + } + + @Test + void abort_whenGzipWrapperInserted_propagatesToOriginalAbortable() throws IOException { + AtomicBoolean aborted = new AtomicBoolean(false); + InputStream body = new TrickleStream(concatenatedGzip("HELLO"), false); + AbortableInputStream abortableBody = AbortableInputStream.create(body, () -> aborted.set(true)); + ResponseInputStream ris = new ResponseInputStream<>(new Object(), abortableBody, Duration.ZERO); + + ris.abort(); + + assertThat(aborted).isTrue(); + } + + @Test + void gzip_whenSourceBlocksThenSignalsEof_decodesWithoutHanging() { + // A genuinely blocking source: after the coerced available()==1 triggers a final-boundary probe, the read must + // still terminate once the source signals EOF rather than hang. + String decoded = assertTimeoutPreemptively(Duration.ofSeconds(5), () -> { + InputStream underlying = new BlockingEofStream(concatenatedGzip("ONLY_ONE;")); + ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); + return readAllGzip(ris); + }); + + assertThat(decoded).isEqualTo("ONLY_ONE;"); + } + + @Test + void markReset_throughWrapper_reReadsSameBytes() throws IOException { + InputStream body = new ByteArrayInputStream("hello-world".getBytes(StandardCharsets.UTF_8)); + ResponseInputStream ris = new ResponseInputStream<>(new Object(), body, Duration.ZERO); + + assertThat(ris.markSupported()).isTrue(); + ris.mark(16); + int first = ris.read(); + int second = ris.read(); + ris.reset(); + + assertThat(ris.read()).isEqualTo(first); + assertThat(ris.read()).isEqualTo(second); + } + private ResponseInputStream responseInputStream(Duration timeout) { return new ResponseInputStream<>(new Object(), abortableInputStream, timeout); } + + private static String readAllGzip(InputStream in) throws IOException { + try (GZIPInputStream gz = new GZIPInputStream(in)) { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buf = new byte[64]; + int n; + while ((n = gz.read(buf)) != -1) { + out.write(buf, 0, n); + } + return new String(out.toByteArray(), StandardCharsets.UTF_8); + } + } + + private static byte[] concatenatedGzip(String... members) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + for (String member : members) { + ByteArrayOutputStream one = new ByteArrayOutputStream(); + try (GZIPOutputStream gz = new GZIPOutputStream(one)) { + gz.write(member.getBytes(StandardCharsets.UTF_8)); + } + out.write(one.toByteArray()); + } + return out.toByteArray(); + } + + /** Serves bytes one at a time; reports available()==0 unless {@code neverZero} */ + private static final class TrickleStream extends InputStream { + private final byte[] data; + private final boolean neverZero; + private int pos = 0; + + TrickleStream(byte[] data, boolean neverZero) { + this.data = data; + this.neverZero = neverZero; + } + + @Override + public int read() { + return pos < data.length ? (data[pos++] & 0xff) : -1; + } + + @Override + public int read(byte[] b, int off, int len) { + if (len == 0) { + return 0; + } + if (pos >= data.length) { + return -1; + } + b[off] = (byte) (data[pos++] & 0xff); + return 1; + } + + @Override + public int available() { + return neverZero ? 1 : 0; + } + } + + /** A blocking "live feed": read() waits for fed data or finish(); read(byte[]) returns only buffered bytes. */ + private static final class ControllableStream extends InputStream { + private final LinkedBlockingQueue queue = new LinkedBlockingQueue<>(); + private volatile boolean finished = false; + + void feed(String s) { + for (byte b : s.getBytes(StandardCharsets.UTF_8)) { + queue.add(b & 0xff); + } + } + + void finish() { + finished = true; + } + + @Override + public int read() throws IOException { + try { + Integer b; + while ((b = queue.poll(50, TimeUnit.MILLISECONDS)) == null) { + if (finished) { + return -1; + } + } + return b; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException(e); + } + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + if (len == 0) { + return 0; + } + int first = read(); + if (first < 0) { + return -1; + } + b[off] = (byte) first; + int n = 1; + while (n < len) { + Integer next = queue.poll(); + if (next == null) { + break; + } + b[off + n] = (byte) (int) next; + n++; + } + return n; + } + + @Override + public int available() { + return queue.size(); + } + } + + /** Serves bytes one at a time (available()==0), then blocks briefly once before signalling EOF. */ + private static final class BlockingEofStream extends InputStream { + private final byte[] data; + private int pos = 0; + private boolean blocked = false; + + BlockingEofStream(byte[] data) { + this.data = data; + } + + @Override + public int read() throws IOException { + if (pos < data.length) { + return data[pos++] & 0xff; + } + if (!blocked) { + blocked = true; + try { + Thread.sleep(150); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException(e); + } + } + return -1; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + if (len == 0) { + return 0; + } + int first = read(); + if (first < 0) { + return -1; + } + b[off] = (byte) first; + return 1; + } + + @Override + public int available() { + return 0; + } + } } diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStreamTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStreamTest.java new file mode 100644 index 000000000000..ec2757e628ce --- /dev/null +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStreamTest.java @@ -0,0 +1,327 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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. + */ + +package software.amazon.awssdk.core.internal.io; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.params.provider.Arguments.arguments; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.stream.Stream; +import java.util.zip.GZIPOutputStream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +/** Unit tests for {@link GzipAvailabilityInputStream}. */ +class GzipAvailabilityInputStreamTest { + + @ParameterizedTest + @MethodSource + void available_afterHeaderRead_returnsExpected(byte[] payload, int expected) throws IOException { + GzipAvailabilityInputStream stream = new GzipAvailabilityInputStream(new ZeroAvailableStream(payload)); + + stream.read(); + stream.read(); + stream.read(); + + assertThat(stream.available()).isEqualTo(expected); + } + + static Stream available_afterHeaderRead_returnsExpected() throws IOException { + return Stream.of( + arguments(gzip("HELLO"), 1), + arguments("event: E1\n".getBytes(StandardCharsets.UTF_8), 0), + arguments(new byte[] {(byte) 0x1f, (byte) 0x8b, 0x09, 0, 0}, 0)); // wrong method (09), not gzip + } + + @Test + void available_whenGzipButDelegateNonZero_returnsDelegateValue() throws IOException { + GzipAvailabilityInputStream stream = + new GzipAvailabilityInputStream(new FixedAvailableStream(gzip("HELLO"), 5)); + + stream.read(); + stream.read(); + stream.read(); + + assertThat(stream.available()).isEqualTo(5); + } + + @Test + void available_whenGzipAtEof_returnsZero() throws IOException { + GzipAvailabilityInputStream stream = new GzipAvailabilityInputStream(new ZeroAvailableStream(gzip("HI"))); + + while (stream.read() != -1) { + } + + assertThat(stream.available()).isEqualTo(0); + } + + @Test + void available_whenClosed_returnsZero() throws IOException { + GzipAvailabilityInputStream stream = + new GzipAvailabilityInputStream(new ZeroAvailableStream(gzip("HELLO"))); + + stream.read(); + stream.read(); + stream.read(); + assertThat(stream.available()).isEqualTo(1); + + stream.close(); + + assertThat(stream.available()).isEqualTo(0); + } + + @Test + void available_whenGzipDetectedViaBulkRead_returnsOne() throws IOException { + GzipAvailabilityInputStream stream = + new GzipAvailabilityInputStream(new BulkZeroStream(gzip("HELLO"))); + + stream.read(new byte[8], 0, 8); + + assertThat(stream.available()).isEqualTo(1); + } + + @Test + void available_whenPartialHeaderThenEof_returnsZero() throws IOException { + byte[] partial = {(byte) 0x1f, (byte) 0x8b}; + GzipAvailabilityInputStream stream = + new GzipAvailabilityInputStream(new ZeroAvailableStream(partial)); + + stream.read(); + stream.read(); + assertThat(stream.read()).isEqualTo(-1); + assertThat(stream.available()).isEqualTo(0); + } + + @Test + void read_whenZeroLengthAfterEof_keepsAvailableZero() throws IOException { + // A zero-length read after EOF returns 0 without moving the stream and must not clear EOF. + GzipAvailabilityInputStream stream = + new GzipAvailabilityInputStream(new ZeroAvailableStream(gzip("HELLO"))); + + while (stream.read() != -1) { + } + assertThat(stream.available()).isEqualTo(0); + + int n = stream.read(new byte[4], 0, 0); + + assertThat(n).isEqualTo(0); + assertThat(stream.available()).isEqualTo(0); + } + + @Test + void skip_whenBeforeClassification_abandonsGzipDetection() throws IOException { + // Junk prefix then a real gzip header: without abandoning detection, skipping the 2 junk bytes would + // expose 1f 8b 08 and be misdetected as gzip. + byte[] gz = gzip("HELLO"); + byte[] data = new byte[gz.length + 2]; + System.arraycopy(gz, 0, data, 2, gz.length); + GzipAvailabilityInputStream stream = new GzipAvailabilityInputStream(new ZeroAvailableStream(data)); + + stream.skip(2); + stream.read(); + stream.read(); + stream.read(); + + assertThat(stream.available()).isEqualTo(0); + } + + @Test + void reset_whenMarkedMidHeader_restoresClassification() throws IOException { + GzipAvailabilityInputStream stream = + new GzipAvailabilityInputStream(new MarkableZeroStream(gzip("HELLO"))); + + stream.mark(100); + stream.read(); + stream.read(); + stream.reset(); + + stream.read(); + stream.read(); + stream.read(); + + assertThat(stream.available()).isEqualTo(1); + } + + @Test + void reset_whenAfterEofWithoutMark_reDetectsGzip() throws IOException { + // reset() without a prior mark rewinds to position 0 (like ByteArrayInputStream, whose mark defaults to 0); + // the wrapper must clear its stale EOF/classification so gzip is re-detected on re-read. + GzipAvailabilityInputStream stream = + new GzipAvailabilityInputStream(new MarkableZeroStream(gzip("HELLO"))); + + while (stream.read() != -1) { + } + stream.reset(); + + stream.read(); + stream.read(); + stream.read(); + + assertThat(stream.available()).isEqualTo(1); + } + + @Test + void release_whenDelegateReleasable_propagates() { + ReleasableZeroStream delegate = new ReleasableZeroStream(); + GzipAvailabilityInputStream stream = new GzipAvailabilityInputStream(delegate); + + stream.release(); + + assertThat(delegate.released).isTrue(); + } + + private static byte[] gzip(String s) throws IOException { + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + try (GZIPOutputStream g = new GZIPOutputStream(bos)) { + g.write(s.getBytes(StandardCharsets.UTF_8)); + } + return bos.toByteArray(); + } + + /** Serves bytes but always reports available()==0. */ + private static final class ZeroAvailableStream extends InputStream { + private final byte[] data; + private int pos; + + ZeroAvailableStream(byte[] data) { + this.data = data; + } + + @Override + public int read() { + return pos < data.length ? (data[pos++] & 0xff) : -1; + } + + @Override + public int available() { + return 0; + } + } + + /** {@link ZeroAvailableStream} that also supports mark/reset (mark defaults to 0). */ + private static final class MarkableZeroStream extends InputStream { + private final byte[] data; + private int pos; + private int markPos; + + MarkableZeroStream(byte[] data) { + this.data = data; + } + + @Override + public int read() { + return pos < data.length ? (data[pos++] & 0xff) : -1; + } + + @Override + public int available() { + return 0; + } + + @Override + public boolean markSupported() { + return true; + } + + @Override + public synchronized void mark(int readlimit) { + markPos = pos; + } + + @Override + public synchronized void reset() { + pos = markPos; + } + } + + /** Returns bytes in bulk (up to len per read) but reports available()==0. */ + private static final class BulkZeroStream extends InputStream { + private final byte[] data; + private int pos; + + BulkZeroStream(byte[] data) { + this.data = data; + } + + @Override + public int read() { + return pos < data.length ? (data[pos++] & 0xff) : -1; + } + + @Override + public int read(byte[] b, int off, int len) { + if (pos >= data.length) { + return -1; + } + int n = Math.min(len, data.length - pos); + System.arraycopy(data, pos, b, off, n); + pos += n; + return n; + } + + @Override + public int available() { + return 0; + } + } + + /** Records whether release() was called; its close() is a no-op. */ + private static final class ReleasableZeroStream extends InputStream implements Releasable { + private boolean released; + + @Override + public int read() { + return -1; + } + + @Override + public int available() { + return 0; + } + + @Override + public void release() { + released = true; + } + } + + /** Serves bytes but reports a fixed available() value. */ + private static final class FixedAvailableStream extends InputStream { + private final byte[] data; + private final int avail; + private int pos; + + FixedAvailableStream(byte[] data, int avail) { + this.data = data; + this.avail = avail; + } + + @Override + public int read() { + return pos < data.length ? (data[pos++] & 0xff) : -1; + } + + @Override + public int available() { + return avail; + } + } +} From 230dedc40cf1ac96413d22932d022c6f865db0b7 Mon Sep 17 00:00:00 2001 From: jencymaryjoseph <35571282+jencymaryjoseph@users.noreply.github.com> Date: Mon, 21 Sep 2026 09:55:35 -0700 Subject: [PATCH 2/4] Fix truncation of concatenated GZIP response streams --- .../bugfix-AWSSDKforJavav2-c8ce5ba.json | 2 +- .../awssdk/core/ResponseInputStream.java | 6 +- .../async/InputStreamResponseTransformer.java | 5 +- .../handler/BaseSyncClientHandler.java | 14 +- .../io/GzipAvailabilityInputStream.java | 12 + .../awssdk/core/ResponseInputStreamTest.java | 261 ------------------ .../client/handler/SyncClientHandlerTest.java | 91 ++++++ .../InputStreamResponseTransformerTest.java | 17 +- .../io/GzipAvailabilityInputStreamTest.java | 243 ++++++++++++++++ .../GetObjectConcatenatedGzipTest.java | 125 +++++++++ 10 files changed, 507 insertions(+), 269 deletions(-) create mode 100644 services/s3/src/test/java/software/amazon/awssdk/services/s3/functionaltests/GetObjectConcatenatedGzipTest.java diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json b/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json index 69ccf94f9839..ce8635fc5f3c 100644 --- a/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json @@ -2,5 +2,5 @@ "type": "bugfix", "category": "AWS SDK for Java v2", "contributor": "", - "description": "Fixed an issue where concatenated gzip response streams could be truncated to the first member when decoded with GZIPInputStream. A transient available()==0 at a gzip member boundary was treated as end of stream; ResponseInputStream now reports available() as at least 1 for gzip content while the stream is open." + "description": "Fixed an issue where concatenated (multi-member) gzip response streams could be truncated to the first member when decoded with GZIPInputStream, because a transient available()==0 at a member boundary was treated as end of stream. Response bodies detected as gzip now report available() as at least 1 while the stream is open." } diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseInputStream.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseInputStream.java index f517f7993118..3fc280ea9fad 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseInputStream.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseInputStream.java @@ -24,7 +24,6 @@ import java.util.concurrent.TimeUnit; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.annotations.SdkTestInternalApi; -import software.amazon.awssdk.core.internal.io.GzipAvailabilityInputStream; import software.amazon.awssdk.core.io.SdkFilterInputStream; import software.amazon.awssdk.http.Abortable; import software.amazon.awssdk.http.AbortableInputStream; @@ -73,10 +72,9 @@ public ResponseInputStream(ResponseT resp, AbortableInputStream in) { } public ResponseInputStream(ResponseT resp, AbortableInputStream in, Duration timeout) { - super(new GzipAvailabilityInputStream(in)); + super(in); this.response = Validate.paramNotNull(resp, "response"); this.abortable = Validate.paramNotNull(in, "abortableInputStream"); - Duration resolvedTimeout = timeout != null ? timeout : DEFAULT_TIMEOUT; scheduleTimeoutTask(resolvedTimeout); } @@ -86,7 +84,7 @@ public ResponseInputStream(ResponseT resp, InputStream in) { } public ResponseInputStream(ResponseT resp, InputStream in, Duration timeout) { - super(new GzipAvailabilityInputStream(in)); + super(in); this.response = Validate.paramNotNull(resp, "response"); this.abortable = in instanceof Abortable ? (Abortable) in : null; diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java index 4083f6fa04c1..eb59beeb2a52 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java @@ -24,6 +24,8 @@ import software.amazon.awssdk.core.SdkResponse; import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.core.async.SdkPublisher; +import software.amazon.awssdk.core.internal.io.GzipAvailabilityInputStream; +import software.amazon.awssdk.http.AbortableInputStream; import software.amazon.awssdk.http.async.AbortableInputStreamSubscriber; /** @@ -59,7 +61,8 @@ public void onStream(SdkPublisher publisher) { this.subscriber = waitForSubscribeSubscriber; publisher.subscribe(waitForSubscribeSubscriber); - future.complete(new ResponseInputStream<>(response, inputStreamSubscriber)); + AbortableInputStream content = GzipAvailabilityInputStream.wrap(inputStreamSubscriber, inputStreamSubscriber); + future.complete(new ResponseInputStream<>(response, content)); } @Override diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java index 03ea0683397b..96173087602d 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java @@ -34,6 +34,7 @@ import software.amazon.awssdk.core.internal.http.AmazonSyncHttpClient; import software.amazon.awssdk.core.internal.http.CombinedResponseHandler; import software.amazon.awssdk.core.internal.http.InterruptMonitor; +import software.amazon.awssdk.core.internal.io.GzipAvailabilityInputStream; import software.amazon.awssdk.core.metrics.CoreMetric; import software.amazon.awssdk.core.sync.RequestBody; import software.amazon.awssdk.core.sync.ResponseTransformer; @@ -210,7 +211,9 @@ private HttpResponseHandlerAdapter(HttpResponseHandler httpResponseHand @Override public ReturnT handle(SdkHttpFullResponse response, ExecutionAttributes executionAttributes) throws Exception { OutputT resp = httpResponseHandler.handle(response, executionAttributes); - return transformResponse(resp, response.content().orElseGet(AbortableInputStream::createEmpty)); + AbortableInputStream content = response.content().orElseGet(AbortableInputStream::createEmpty); + AbortableInputStream body = wrapForConcatenatedGzipSupport(content); + return transformResponse(resp, body); } @Override @@ -218,6 +221,15 @@ public boolean needsConnectionLeftOpen() { return responseTransformer.needsConnectionLeftOpen(); } + /** + * Wraps a gzip response body so {@code available()} never returns {@code 0} while the stream is open, working + * around {@link java.util.zip.GZIPInputStream} truncating concatenated (multi-member) gzip at a member boundary. + * Non-gzip content passes through unchanged and the original stream's {@code abort()} is preserved. + */ + private static AbortableInputStream wrapForConcatenatedGzipSupport(AbortableInputStream content) { + return GzipAvailabilityInputStream.wrap(content, content); + } + private ReturnT transformResponse(OutputT resp, AbortableInputStream inputStream) throws Exception { try { diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStream.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStream.java index b1b5b5cc6716..74565c461ab6 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStream.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStream.java @@ -19,6 +19,8 @@ import java.io.IOException; import java.io.InputStream; import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.http.Abortable; +import software.amazon.awssdk.http.AbortableInputStream; import software.amazon.awssdk.utils.IoUtils; /** @@ -52,6 +54,16 @@ public GzipAvailabilityInputStream(InputStream in) { super(in); } + /** + * Wraps a response body's content so {@code available()} is gzip-safe, while preserving {@code abort()} on the + * original stream. Applied by the SDK at the sync and async blocking-stream boundaries (where the caller's + * {@link java.util.zip.GZIPInputStream} reads), so that {@link software.amazon.awssdk.core.ResponseInputStream} + * itself stays content-type agnostic. + */ + public static AbortableInputStream wrap(InputStream content, Abortable abortable) { + return AbortableInputStream.create(new GzipAvailabilityInputStream(content), abortable); + } + @Override public int read() throws IOException { int b = in.read(); diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseInputStreamTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseInputStreamTest.java index 75c0617cc707..6710465a899d 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseInputStreamTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseInputStreamTest.java @@ -17,23 +17,12 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; -import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import java.io.BufferedReader; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.io.InputStreamReader; -import java.nio.charset.StandardCharsets; import java.time.Duration; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.zip.GZIPInputStream; -import java.util.zip.GZIPOutputStream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -144,257 +133,7 @@ void negativeTimeout_disablesTimeout() throws Exception { assertThat(responseInputStream.hasTimeoutTask()).isFalse(); } - @Test - void gzipConcatenatedMembers_whenAvailableTransientlyZero_decodesAllMembers() throws IOException { - InputStream underlying = new TrickleStream(concatenatedGzip("PART_ONE;", "PART_TWO;"), false); - ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); - - assertThat(readAllGzip(ris)).isEqualTo("PART_ONE;PART_TWO;"); - } - - @Test - void gzipManyMembers_whenAvailableTransientlyZero_decodesAllMembers() throws IOException { - InputStream underlying = new TrickleStream(concatenatedGzip("A;", "B;", "C;", "D;", "E;"), false); - ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); - - assertThat(readAllGzip(ris)).isEqualTo("A;B;C;D;E;"); - } - - @Test - void gzipSingleMember_whenAvailableZero_decodesWithoutHanging() { - // Preemptive timeout guards against the final-boundary probe hanging. - String decoded = assertTimeoutPreemptively(Duration.ofSeconds(5), () -> { - InputStream underlying = new TrickleStream(concatenatedGzip("ONLY_ONE_MEMBER;"), false); - ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); - return readAllGzip(ris); - }); - - assertThat(decoded).isEqualTo("ONLY_ONE_MEMBER;"); - } - - @Test - void gzipConcatenatedMembers_whenNeverZeroAvailable_decodesAllMembers() throws IOException { - InputStream underlying = new TrickleStream(concatenatedGzip("PART_ONE;", "PART_TWO;"), true); - ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); - - assertThat(readAllGzip(ris)).isEqualTo("PART_ONE;PART_TWO;"); - } - - @Test - void bufferedReader_whenNonGzip_deliversLineWithoutBlocking() { - ControllableStream underlying = new ControllableStream(); - underlying.feed("event: E1\n"); - ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); - BufferedReader reader = new BufferedReader(new InputStreamReader(ris, StandardCharsets.UTF_8)); - - assertTimeoutPreemptively(Duration.ofSeconds(2), () -> - assertThat(reader.readLine()).isEqualTo("event: E1")); - } - - @Test - void abort_whenGzipWrapperInserted_propagatesToOriginalAbortable() throws IOException { - AtomicBoolean aborted = new AtomicBoolean(false); - InputStream body = new TrickleStream(concatenatedGzip("HELLO"), false); - AbortableInputStream abortableBody = AbortableInputStream.create(body, () -> aborted.set(true)); - ResponseInputStream ris = new ResponseInputStream<>(new Object(), abortableBody, Duration.ZERO); - - ris.abort(); - - assertThat(aborted).isTrue(); - } - - @Test - void gzip_whenSourceBlocksThenSignalsEof_decodesWithoutHanging() { - // A genuinely blocking source: after the coerced available()==1 triggers a final-boundary probe, the read must - // still terminate once the source signals EOF rather than hang. - String decoded = assertTimeoutPreemptively(Duration.ofSeconds(5), () -> { - InputStream underlying = new BlockingEofStream(concatenatedGzip("ONLY_ONE;")); - ResponseInputStream ris = new ResponseInputStream<>(new Object(), underlying, Duration.ZERO); - return readAllGzip(ris); - }); - - assertThat(decoded).isEqualTo("ONLY_ONE;"); - } - - @Test - void markReset_throughWrapper_reReadsSameBytes() throws IOException { - InputStream body = new ByteArrayInputStream("hello-world".getBytes(StandardCharsets.UTF_8)); - ResponseInputStream ris = new ResponseInputStream<>(new Object(), body, Duration.ZERO); - - assertThat(ris.markSupported()).isTrue(); - ris.mark(16); - int first = ris.read(); - int second = ris.read(); - ris.reset(); - - assertThat(ris.read()).isEqualTo(first); - assertThat(ris.read()).isEqualTo(second); - } - private ResponseInputStream responseInputStream(Duration timeout) { return new ResponseInputStream<>(new Object(), abortableInputStream, timeout); } - - private static String readAllGzip(InputStream in) throws IOException { - try (GZIPInputStream gz = new GZIPInputStream(in)) { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - byte[] buf = new byte[64]; - int n; - while ((n = gz.read(buf)) != -1) { - out.write(buf, 0, n); - } - return new String(out.toByteArray(), StandardCharsets.UTF_8); - } - } - - private static byte[] concatenatedGzip(String... members) throws IOException { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - for (String member : members) { - ByteArrayOutputStream one = new ByteArrayOutputStream(); - try (GZIPOutputStream gz = new GZIPOutputStream(one)) { - gz.write(member.getBytes(StandardCharsets.UTF_8)); - } - out.write(one.toByteArray()); - } - return out.toByteArray(); - } - - /** Serves bytes one at a time; reports available()==0 unless {@code neverZero} */ - private static final class TrickleStream extends InputStream { - private final byte[] data; - private final boolean neverZero; - private int pos = 0; - - TrickleStream(byte[] data, boolean neverZero) { - this.data = data; - this.neverZero = neverZero; - } - - @Override - public int read() { - return pos < data.length ? (data[pos++] & 0xff) : -1; - } - - @Override - public int read(byte[] b, int off, int len) { - if (len == 0) { - return 0; - } - if (pos >= data.length) { - return -1; - } - b[off] = (byte) (data[pos++] & 0xff); - return 1; - } - - @Override - public int available() { - return neverZero ? 1 : 0; - } - } - - /** A blocking "live feed": read() waits for fed data or finish(); read(byte[]) returns only buffered bytes. */ - private static final class ControllableStream extends InputStream { - private final LinkedBlockingQueue queue = new LinkedBlockingQueue<>(); - private volatile boolean finished = false; - - void feed(String s) { - for (byte b : s.getBytes(StandardCharsets.UTF_8)) { - queue.add(b & 0xff); - } - } - - void finish() { - finished = true; - } - - @Override - public int read() throws IOException { - try { - Integer b; - while ((b = queue.poll(50, TimeUnit.MILLISECONDS)) == null) { - if (finished) { - return -1; - } - } - return b; - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IOException(e); - } - } - - @Override - public int read(byte[] b, int off, int len) throws IOException { - if (len == 0) { - return 0; - } - int first = read(); - if (first < 0) { - return -1; - } - b[off] = (byte) first; - int n = 1; - while (n < len) { - Integer next = queue.poll(); - if (next == null) { - break; - } - b[off + n] = (byte) (int) next; - n++; - } - return n; - } - - @Override - public int available() { - return queue.size(); - } - } - - /** Serves bytes one at a time (available()==0), then blocks briefly once before signalling EOF. */ - private static final class BlockingEofStream extends InputStream { - private final byte[] data; - private int pos = 0; - private boolean blocked = false; - - BlockingEofStream(byte[] data) { - this.data = data; - } - - @Override - public int read() throws IOException { - if (pos < data.length) { - return data[pos++] & 0xff; - } - if (!blocked) { - blocked = true; - try { - Thread.sleep(150); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IOException(e); - } - } - return -1; - } - - @Override - public int read(byte[] b, int off, int len) throws IOException { - if (len == 0) { - return 0; - } - int first = read(); - if (first < 0) { - return -1; - } - b[off] = (byte) first; - return 1; - } - - @Override - public int available() { - return 0; - } - } } diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java index efbc983e6b0e..b5e46f8fba8a 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java @@ -22,11 +22,17 @@ import static org.mockito.Mockito.when; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -171,6 +177,29 @@ public void responseTransformerThrowsOtherException_shouldWrapWithNonRetryableEx .hasCauseInstanceOf(NonRetryableException.class); } + @Test + public void execute_whenConcatenatedGzipHasTransientZeroAvailable_decodesAllMembers() throws Exception { + mockSuccessfulStreamingCall(concatenatedGzip("member-0", "member-1", "member-2", "member-3", "member-4")); + when(responseTransformer.needsConnectionLeftOpen()).thenReturn(true); // customer-held stream, e.g. toInputStream() + when(responseTransformer.transform(any(SdkResponse.class), any(AbortableInputStream.class))) + .thenAnswer(invocation -> readAllGzip(invocation.getArgument(1))); + + Object decoded = syncClientHandler.execute(clientExecutionParams(), responseTransformer); + + assertThat(decoded).isEqualTo("member-0member-1member-2member-3member-4"); + } + + @Test + public void execute_whenCustomTransformerDoesNotLeaveConnectionOpen_decodesAllGzipMembers() throws Exception { + mockSuccessfulStreamingCall(concatenatedGzip("member-0", "member-1", "member-2")); + ResponseTransformer customTransformer = + (response, inputStream) -> readAllGzip(inputStream); + + String decoded = syncClientHandler.execute(clientExecutionParams(), customTransformer); + + assertThat(decoded).isEqualTo("member-0member-1member-2"); + } + private void verifyResponseTransformerPropagateException(Exception exception) throws Exception { mockSuccessfulApiCall(); when(responseTransformer.transform(any(SdkResponse.class), any(AbortableInputStream.class))).thenThrow( @@ -194,6 +223,68 @@ private void expectRetrievalFromMocks() { when(httpClient.prepareRequest(any())).thenReturn(httpClientCall); } + private void mockSuccessfulStreamingCall(byte[] body) throws Exception { + expectRetrievalFromMocks(); + when(httpClientCall.call()).thenReturn(HttpExecuteResponse.builder() + .responseBody(AbortableInputStream.create(zeroAvailableDripStream(body))) + .response(SdkHttpResponse.builder().statusCode(200).build()) + .build()); + when(responseHandler.handle(any(), any())).thenReturn(VoidSdkResponse.builder().build()); + } + + /** Serves one byte per read and always reports {@code available()==0}, mimicking a socket with no bytes buffered. */ + private static InputStream zeroAvailableDripStream(byte[] data) { + return new InputStream() { + private int pos; + + @Override + public int read() { + return pos < data.length ? data[pos++] & 0xff : -1; + } + + @Override + public int read(byte[] b, int off, int len) { + if (len == 0) { + return 0; + } + if (pos >= data.length) { + return -1; + } + b[off] = (byte) (data[pos++] & 0xff); + return 1; + } + + @Override + public int available() { + return 0; + } + }; + } + + private static byte[] concatenatedGzip(String... members) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + for (String member : members) { + ByteArrayOutputStream one = new ByteArrayOutputStream(); + try (GZIPOutputStream gz = new GZIPOutputStream(one)) { + gz.write(member.getBytes(StandardCharsets.UTF_8)); + } + out.write(one.toByteArray()); + } + return out.toByteArray(); + } + + private static String readAllGzip(InputStream in) throws IOException { + try (GZIPInputStream gz = new GZIPInputStream(in)) { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buf = new byte[64]; + int n; + while ((n = gz.read(buf)) != -1) { + out.write(buf, 0, n); + } + return new String(out.toByteArray(), StandardCharsets.UTF_8); + } + } + private ClientExecutionParams clientExecutionParams() { return new ClientExecutionParams() .withInput(request) diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java index 026ba1a9cc85..9ed16eaab223 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java @@ -142,4 +142,19 @@ public void cancel() { assertThat(onSubscribeCalled.get()).isTrue(); } } -} \ No newline at end of file + + @Test + void onStream_whenGzipDetected_coercesAvailableAboveZeroWhileOpen() throws IOException { + ResponseInputStream stream = resultFuture.join(); + + publisher.send(ByteBuffer.wrap(new byte[] {0x1f, (byte) 0x8b, 0x08})); // gzip magic + stream.read(); + stream.read(); + stream.read(); + + // Nothing buffered now, so the raw stream would report 0; the gzip-aware wrapper coerces it to >= 1 so a + // wrapping GZIPInputStream does not truncate concatenated (multi-member) gzip at a member boundary. + assertThat(stream.available()).isGreaterThanOrEqualTo(1); + } + +} diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStreamTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStreamTest.java index ec2757e628ce..97d6fdc795f3 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStreamTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/io/GzipAvailabilityInputStreamTest.java @@ -16,18 +16,28 @@ package software.amazon.awssdk.core.internal.io; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; import static org.junit.jupiter.params.provider.Arguments.arguments; +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Stream; +import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import software.amazon.awssdk.http.AbortableInputStream; /** Unit tests for {@link GzipAvailabilityInputStream}. */ class GzipAvailabilityInputStreamTest { @@ -188,6 +198,84 @@ void release_whenDelegateReleasable_propagates() { assertThat(delegate.released).isTrue(); } + @Test + void read_whenConcatenatedMembersHaveTransientZeroAvailable_decodesAllMembers() throws IOException { + InputStream underlying = new TrickleStream(concatenatedGzip("PART_ONE;", "PART_TWO;"), false); + + assertThat(readAllGzip(new GzipAvailabilityInputStream(underlying))).isEqualTo("PART_ONE;PART_TWO;"); + } + + @Test + void read_whenManyConcatenatedMembersHaveTransientZeroAvailable_decodesAllMembers() throws IOException { + InputStream underlying = new TrickleStream(concatenatedGzip("A;", "B;", "C;", "D;", "E;"), false); + + assertThat(readAllGzip(new GzipAvailabilityInputStream(underlying))).isEqualTo("A;B;C;D;E;"); + } + + @Test + void read_whenSingleMemberHasZeroAvailable_decodesWithoutHanging() { + String decoded = assertTimeoutPreemptively(Duration.ofSeconds(5), () -> { + InputStream underlying = new TrickleStream(concatenatedGzip("ONLY_ONE_MEMBER;"), false); + return readAllGzip(new GzipAvailabilityInputStream(underlying)); + }); + + assertThat(decoded).isEqualTo("ONLY_ONE_MEMBER;"); + } + + @Test + void read_whenConcatenatedMembersNeverReportZeroAvailable_decodesAllMembers() throws IOException { + InputStream underlying = new TrickleStream(concatenatedGzip("PART_ONE;", "PART_TWO;"), true); + + assertThat(readAllGzip(new GzipAvailabilityInputStream(underlying))).isEqualTo("PART_ONE;PART_TWO;"); + } + + @Test + void readLine_whenContentIsNonGzip_deliversLineWithoutBlocking() { + ControllableStream underlying = new ControllableStream(); + underlying.feed("event: E1\n"); + BufferedReader reader = + new BufferedReader(new InputStreamReader(new GzipAvailabilityInputStream(underlying), StandardCharsets.UTF_8)); + + assertTimeoutPreemptively(Duration.ofSeconds(2), () -> + assertThat(reader.readLine()).isEqualTo("event: E1")); + } + + @Test + void wrap_whenAborted_propagatesToOriginalAbortable() throws IOException { + AtomicBoolean aborted = new AtomicBoolean(false); + InputStream body = new TrickleStream(concatenatedGzip("HELLO"), false); + AbortableInputStream wrapped = GzipAvailabilityInputStream.wrap(body, () -> aborted.set(true)); + + wrapped.abort(); + + assertThat(aborted).isTrue(); + } + + @Test + void read_whenSourceBlocksThenSignalsEof_decodesWithoutHanging() { + String decoded = assertTimeoutPreemptively(Duration.ofSeconds(5), () -> { + InputStream underlying = new BlockingEofStream(concatenatedGzip("ONLY_ONE;")); + return readAllGzip(new GzipAvailabilityInputStream(underlying)); + }); + + assertThat(decoded).isEqualTo("ONLY_ONE;"); + } + + @Test + void reset_whenMarkedBeforeReading_reReadsSameBytes() throws IOException { + InputStream body = new ByteArrayInputStream("hello-world".getBytes(StandardCharsets.UTF_8)); + GzipAvailabilityInputStream stream = new GzipAvailabilityInputStream(body); + + assertThat(stream.markSupported()).isTrue(); + stream.mark(16); + int first = stream.read(); + int second = stream.read(); + stream.reset(); + + assertThat(stream.read()).isEqualTo(first); + assertThat(stream.read()).isEqualTo(second); + } + private static byte[] gzip(String s) throws IOException { ByteArrayOutputStream bos = new ByteArrayOutputStream(); try (GZIPOutputStream g = new GZIPOutputStream(bos)) { @@ -196,6 +284,26 @@ private static byte[] gzip(String s) throws IOException { return bos.toByteArray(); } + private static String readAllGzip(InputStream in) throws IOException { + try (GZIPInputStream gz = new GZIPInputStream(in)) { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buf = new byte[64]; + int n; + while ((n = gz.read(buf)) != -1) { + out.write(buf, 0, n); + } + return new String(out.toByteArray(), StandardCharsets.UTF_8); + } + } + + private static byte[] concatenatedGzip(String... members) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + for (String member : members) { + out.write(gzip(member)); + } + return out.toByteArray(); + } + /** Serves bytes but always reports available()==0. */ private static final class ZeroAvailableStream extends InputStream { private final byte[] data; @@ -324,4 +432,139 @@ public int available() { return avail; } } + + /** Serves bytes one at a time; reports available()==0 unless {@code neverZero}. */ + private static final class TrickleStream extends InputStream { + private final byte[] data; + private final boolean neverZero; + private int pos; + + TrickleStream(byte[] data, boolean neverZero) { + this.data = data; + this.neverZero = neverZero; + } + + @Override + public int read() { + return pos < data.length ? data[pos++] & 0xff : -1; + } + + @Override + public int read(byte[] b, int off, int len) { + if (len == 0) { + return 0; + } + if (pos >= data.length) { + return -1; + } + b[off] = (byte) (data[pos++] & 0xff); + return 1; + } + + @Override + public int available() { + return neverZero ? 1 : 0; + } + } + + /** A blocking live feed: read waits for fed data and bulk reads return only buffered bytes. */ + private static final class ControllableStream extends InputStream { + private final LinkedBlockingQueue queue = new LinkedBlockingQueue<>(); + private volatile boolean finished; + + void feed(String s) { + for (byte b : s.getBytes(StandardCharsets.UTF_8)) { + queue.add(b & 0xff); + } + } + + @Override + public int read() throws IOException { + try { + Integer b; + while ((b = queue.poll(50, TimeUnit.MILLISECONDS)) == null) { + if (finished) { + return -1; + } + } + return b; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException(e); + } + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + if (len == 0) { + return 0; + } + int first = read(); + if (first < 0) { + return -1; + } + b[off] = (byte) first; + int n = 1; + while (n < len) { + Integer next = queue.poll(); + if (next == null) { + break; + } + b[off + n] = (byte) (int) next; + n++; + } + return n; + } + + @Override + public int available() { + return queue.size(); + } + } + + /** Serves bytes one at a time, then blocks briefly once before signalling EOF. */ + private static final class BlockingEofStream extends InputStream { + private final byte[] data; + private int pos; + private boolean blocked; + + BlockingEofStream(byte[] data) { + this.data = data; + } + + @Override + public int read() throws IOException { + if (pos < data.length) { + return data[pos++] & 0xff; + } + if (!blocked) { + blocked = true; + try { + Thread.sleep(150); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException(e); + } + } + return -1; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + if (len == 0) { + return 0; + } + int first = read(); + if (first < 0) { + return -1; + } + b[off] = (byte) first; + return 1; + } + + @Override + public int available() { + return 0; + } + } } diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/functionaltests/GetObjectConcatenatedGzipTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/functionaltests/GetObjectConcatenatedGzipTest.java new file mode 100644 index 000000000000..28009a1aaded --- /dev/null +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/functionaltests/GetObjectConcatenatedGzipTest.java @@ -0,0 +1,125 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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. + */ + +package software.amazon.awssdk.services.s3.functionaltests; + +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.any; +import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static org.assertj.core.api.Assertions.assertThat; + +import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; +import com.github.tomakehurst.wiremock.junit5.WireMockTest; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; +import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.core.ResponseInputStream; +import software.amazon.awssdk.core.async.AsyncResponseTransformer; +import software.amazon.awssdk.core.sync.ResponseTransformer; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.s3.S3AsyncClient; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.GetObjectResponse; + +/** + * End-to-end (through a real client over WireMock) verification that a concatenated (multi-member) gzip response body + * with NO {@code Content-Encoding: gzip} header — i.e. the caller decodes it themselves — round-trips through both the + * sync {@code toInputStream()} and async {@code toBlockingInputStream()} paths and decodes to ALL members. + * + *

This is a happy-path regression guard: it proves the {@code GzipAvailabilityInputStream} wrap the SDK now applies + * does not corrupt, stall, or truncate a streaming gzip download over the real HTTP stack. It does NOT reproduce the + * underlying truncation — WireMock delivers the whole body into the socket buffer, so {@code available()} is never + * transiently {@code 0} at a member boundary. Deterministic reproduction of the boundary condition lives in the + * lower-level stream and handler tests. + */ +@WireMockTest +public class GetObjectConcatenatedGzipTest { + + private static final String BUCKET = "Example-Bucket"; + private static final String KEY = "concatenated.gz"; + private static final String[] MEMBERS = {"member-0", "member-1", "member-2", "member-3", "member-4", + "member-5", "member-6", "member-7", "member-8", "member-9"}; + private static final String EXPECTED = String.join("", MEMBERS); + + @Test + public void syncGetObject_whenBodyContainsConcatenatedGzip_decodesAllMembers(WireMockRuntimeInfo wm) throws Exception { + stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200).withBody(concatenatedGzip(MEMBERS)))); + + try (S3Client s3 = syncClient(wm)) { + ResponseInputStream body = + s3.getObject(r -> r.bucket(BUCKET).key(KEY), ResponseTransformer.toInputStream()); + assertThat(gunzip(body)).isEqualTo(EXPECTED); + } + } + + @Test + public void asyncGetObject_whenBodyContainsConcatenatedGzip_decodesAllMembers(WireMockRuntimeInfo wm) throws Exception { + stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200).withBody(concatenatedGzip(MEMBERS)))); + + try (S3AsyncClient s3Async = asyncClient(wm)) { + ResponseInputStream body = + s3Async.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBlockingInputStream()).join(); + assertThat(gunzip(body)).isEqualTo(EXPECTED); + } + } + + private static S3Client syncClient(WireMockRuntimeInfo wm) { + return S3Client.builder() + .region(Region.US_EAST_1) + .endpointOverride(URI.create(wm.getHttpBaseUrl())) + .credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create("key", "secret"))) + .build(); + } + + private static S3AsyncClient asyncClient(WireMockRuntimeInfo wm) { + return S3AsyncClient.builder() + .region(Region.US_EAST_1) + .endpointOverride(URI.create(wm.getHttpBaseUrl())) + .credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create("key", "secret"))) + .build(); + } + + private static byte[] concatenatedGzip(String... members) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + for (String member : members) { + ByteArrayOutputStream one = new ByteArrayOutputStream(); + try (GZIPOutputStream gz = new GZIPOutputStream(one)) { + gz.write(member.getBytes(StandardCharsets.UTF_8)); + } + out.write(one.toByteArray()); + } + return out.toByteArray(); + } + + private static String gunzip(InputStream in) throws IOException { + try (GZIPInputStream gz = new GZIPInputStream(in)) { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buf = new byte[64]; + int n; + while ((n = gz.read(buf)) != -1) { + out.write(buf, 0, n); + } + return new String(out.toByteArray(), StandardCharsets.UTF_8); + } + } +} From f902dc280e894019100fef8943419c3131b312a1 Mon Sep 17 00:00:00 2001 From: jencymaryjoseph <35571282+jencymaryjoseph@users.noreply.github.com> Date: Mon, 21 Sep 2026 10:21:07 -0700 Subject: [PATCH 3/4] Add opt-out for concatenated GZIP stream support --- .../bugfix-AWSSDKforJavav2-c8ce5ba.json | 2 +- .../builder/AwsDefaultClientBuilder.java | 1 + .../internal/AwsExecutionContextBuilder.java | 2 + .../AsyncResponseTransformerListener.java | 15 +- .../config/SdkAdvancedClientOption.java | 8 + .../SdkInternalExecutionAttribute.java | 6 + .../ConfigurableAsyncResponseTransformer.java | 43 ++++ .../async/InputStreamResponseTransformer.java | 22 +- .../handler/BaseAsyncClientHandler.java | 16 +- .../handler/BaseSyncClientHandler.java | 10 +- .../AsyncResponseTransformerUtilsTest.java | 83 ++++++++ .../handler/AsyncClientHandlerTest.java | 85 ++++++++ .../client/handler/SyncClientHandlerTest.java | 23 +++ .../InputStreamResponseTransformerTest.java | 30 +++ ...tenatedGzipStreamSupportS3AsyncClient.java | 60 ++++++ .../client/S3AsyncClientDecorator.java | 9 + ...S3AsyncClientDecoratorGzipSupportTest.java | 191 ++++++++++++++++++ 17 files changed, 598 insertions(+), 8 deletions(-) create mode 100644 core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/ConfigurableAsyncResponseTransformer.java create mode 100644 core/sdk-core/src/test/java/software/amazon/awssdk/core/async/AsyncResponseTransformerUtilsTest.java create mode 100644 services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/ConcatenatedGzipStreamSupportS3AsyncClient.java create mode 100644 services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecoratorGzipSupportTest.java diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json b/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json index ce8635fc5f3c..ae0b44d35f50 100644 --- a/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json @@ -2,5 +2,5 @@ "type": "bugfix", "category": "AWS SDK for Java v2", "contributor": "", - "description": "Fixed an issue where concatenated (multi-member) gzip response streams could be truncated to the first member when decoded with GZIPInputStream, because a transient available()==0 at a member boundary was treated as end of stream. Response bodies detected as gzip now report available() as at least 1 while the stream is open." + "description": "Fixed an issue where concatenated (multi-member) gzip response streams could be truncated to the first member when decoded with GZIPInputStream, because a transient available()==0 at a member boundary was treated as end of stream. The corrected behavior is enabled by default and can be disabled with SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED." } diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java index 92036575eeb1..7542ceb232ad 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java @@ -138,6 +138,7 @@ protected final SdkClientConfiguration mergeChildDefaults(SdkClientConfiguration SdkClientConfiguration config = mergeServiceDefaults(configuration); config = config.merge(c -> c.option(AwsAdvancedClientOption.ENABLE_DEFAULT_REGION_DETECTION, true) .option(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION, false) + .option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED, true) .option(AwsClientOption.SERVICE_SIGNING_NAME, signingName()) .option(SdkClientOption.SERVICE_NAME, serviceName()) .option(AwsClientOption.ENDPOINT_PREFIX, serviceEndpointPrefix())); diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java index 9a6434186dd2..4d26e18986de 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java @@ -130,6 +130,8 @@ private AwsExecutionContextBuilder() { clientConfig.option(SdkClientOption.CLIENT_CONTEXT_PARAMS)) .putAttribute(SdkInternalExecutionAttribute.DISABLE_HOST_PREFIX_INJECTION, clientConfig.option(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION)) + .putAttribute(SdkInternalExecutionAttribute.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED, + clientConfig.option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED)) .putAttribute(SdkInternalExecutionAttribute.SDK_CLIENT, clientConfig.option(SdkClientOption.SDK_CLIENT)) .putAttribute(SdkExecutionAttribute.SIGNER_OVERRIDDEN, clientConfig.option(SdkClientOption.SIGNER_OVERRIDDEN)) .putAttribute(AwsExecutionAttribute.USE_GLOBAL_ENDPOINT, diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/listener/AsyncResponseTransformerListener.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/listener/AsyncResponseTransformerListener.java index f1612e1b1f3e..dfaacc431f38 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/listener/AsyncResponseTransformerListener.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/listener/AsyncResponseTransformerListener.java @@ -23,6 +23,7 @@ import software.amazon.awssdk.core.SplittingTransformerConfiguration; import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.core.async.SdkPublisher; +import software.amazon.awssdk.core.internal.async.ConfigurableAsyncResponseTransformer; import software.amazon.awssdk.utils.Logger; import software.amazon.awssdk.utils.Validate; @@ -65,7 +66,9 @@ static AsyncResponseTransformer wrap( } @SdkInternalApi - final class NotifyingAsyncResponseTransformer implements AsyncResponseTransformer { + final class NotifyingAsyncResponseTransformer + implements AsyncResponseTransformer, + ConfigurableAsyncResponseTransformer { private static final Logger log = Logger.loggerFor(NotifyingAsyncResponseTransformer.class); private final AsyncResponseTransformer delegate; @@ -81,6 +84,16 @@ public AsyncResponseTransformer getDelegate() { return delegate; } + @Override + public AsyncResponseTransformer withConcatenatedGzipStreamSupportEnabled(boolean enabled) { + AsyncResponseTransformer configuredDelegate = + ConfigurableAsyncResponseTransformer.configure(delegate, enabled); + if (configuredDelegate == delegate) { + return this; + } + return new NotifyingAsyncResponseTransformer<>(configuredDelegate, listener); + } + @Override public CompletableFuture prepare() { return delegate.prepare(); diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/SdkAdvancedClientOption.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/SdkAdvancedClientOption.java index 28924e9aea2d..b5204963038d 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/SdkAdvancedClientOption.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/SdkAdvancedClientOption.java @@ -88,6 +88,14 @@ public class SdkAdvancedClientOption extends ClientOption { public static final SdkAdvancedClientOption DISABLE_HOST_PREFIX_INJECTION = new SdkAdvancedClientOption<>(Boolean.class); + /** + * Whether response streams support concatenated (multi-member) gzip by preventing a temporary + * {@code InputStream.available() == 0} from being interpreted as the end of the gzip stream. + * Enabled by default. + */ + public static final SdkAdvancedClientOption CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED = + new SdkAdvancedClientOption<>(Boolean.class); + protected SdkAdvancedClientOption(Class valueClass) { super(valueClass); OPTIONS.add(this); diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/SdkInternalExecutionAttribute.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/SdkInternalExecutionAttribute.java index eea10003c89b..4114f05b1a36 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/SdkInternalExecutionAttribute.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/SdkInternalExecutionAttribute.java @@ -87,6 +87,12 @@ public final class SdkInternalExecutionAttribute extends SdkExecutionAttribute { public static final ExecutionAttribute DISABLE_HOST_PREFIX_INJECTION = new ExecutionAttribute<>("DisableHostPrefixInjection"); + /** + * Whether concatenated-gzip response stream support is enabled. + */ + public static final ExecutionAttribute CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED = + new ExecutionAttribute<>("ConcatenatedGzipStreamSupportEnabled"); + /** * Key to indicate if the Http Checksums that are valid for an operation. */ diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/ConfigurableAsyncResponseTransformer.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/ConfigurableAsyncResponseTransformer.java new file mode 100644 index 000000000000..72c6a46ad4be --- /dev/null +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/ConfigurableAsyncResponseTransformer.java @@ -0,0 +1,43 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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. + */ + +package software.amazon.awssdk.core.internal.async; + +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.core.async.AsyncResponseTransformer; + +/** + * Internal contract for SDK response transformers that support concatenated-gzip configuration. + */ +@SdkInternalApi +public interface ConfigurableAsyncResponseTransformer + extends AsyncResponseTransformer { + + AsyncResponseTransformer withConcatenatedGzipStreamSupportEnabled(boolean enabled); + + /** + * Configures a transformer when it supports this internal contract. Customer transformers are returned unchanged. + */ + @SuppressWarnings("unchecked") + static AsyncResponseTransformer configure( + AsyncResponseTransformer transformer, boolean enabled) { + + if (transformer instanceof ConfigurableAsyncResponseTransformer) { + return ((ConfigurableAsyncResponseTransformer) transformer) + .withConcatenatedGzipStreamSupportEnabled(enabled); + } + return transformer; + } +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java index eb59beeb2a52..eb1b5d9f913d 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java @@ -35,11 +35,21 @@ */ @SdkInternalApi public class InputStreamResponseTransformer - implements AsyncResponseTransformer> { + implements AsyncResponseTransformer>, + ConfigurableAsyncResponseTransformer> { private volatile CompletableFuture> future; private volatile ResponseT response; private volatile WaitForSubscribeOnErrorWrapper subscriber; + private final boolean concatenatedGzipStreamSupportEnabled; + + public InputStreamResponseTransformer() { + this(true); + } + + private InputStreamResponseTransformer(boolean concatenatedGzipStreamSupportEnabled) { + this.concatenatedGzipStreamSupportEnabled = concatenatedGzipStreamSupportEnabled; + } @Override public CompletableFuture> prepare() { @@ -61,7 +71,9 @@ public void onStream(SdkPublisher publisher) { this.subscriber = waitForSubscribeSubscriber; publisher.subscribe(waitForSubscribeSubscriber); - AbortableInputStream content = GzipAvailabilityInputStream.wrap(inputStreamSubscriber, inputStreamSubscriber); + AbortableInputStream content = concatenatedGzipStreamSupportEnabled + ? GzipAvailabilityInputStream.wrap(inputStreamSubscriber, inputStreamSubscriber) + : AbortableInputStream.create(inputStreamSubscriber, inputStreamSubscriber); future.complete(new ResponseInputStream<>(response, content)); } @@ -78,6 +90,12 @@ public String name() { return TransformerType.STREAM.getName(); } + @Override + public AsyncResponseTransformer> + withConcatenatedGzipStreamSupportEnabled(boolean enabled) { + return concatenatedGzipStreamSupportEnabled == enabled ? this : new InputStreamResponseTransformer<>(enabled); + } + // Simple wrapper subscriber that ensures we don't forward the `onError` to the delegate until onSubscribe is called, to be // compliant with the reactive streams spec. We use onError for forwarding the exception given to exceptionOccurred. private static final class WaitForSubscribeOnErrorWrapper implements Subscriber { diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseAsyncClientHandler.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseAsyncClientHandler.java index 0c2f91a3a424..682f6c847dc3 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseAsyncClientHandler.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseAsyncClientHandler.java @@ -27,6 +27,7 @@ import software.amazon.awssdk.core.SdkResponse; import software.amazon.awssdk.core.async.AsyncRequestBody; import software.amazon.awssdk.core.async.AsyncResponseTransformer; +import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.handler.AsyncClientHandler; import software.amazon.awssdk.core.client.handler.ClientExecutionParams; @@ -36,7 +37,9 @@ import software.amazon.awssdk.core.http.HttpResponseHandler; import software.amazon.awssdk.core.interceptor.ExecutionAttributes; import software.amazon.awssdk.core.interceptor.InterceptorContext; +import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute; import software.amazon.awssdk.core.internal.InternalCoreExecutionAttribute; +import software.amazon.awssdk.core.internal.async.ConfigurableAsyncResponseTransformer; import software.amazon.awssdk.core.internal.http.AmazonAsyncHttpClient; import software.amazon.awssdk.core.internal.http.IdempotentAsyncResponseHandler; import software.amazon.awssdk.core.internal.http.TransformingAsyncResponseHandler; @@ -98,8 +101,19 @@ public Complet ExecutionAttributes executionAttributes = executionParams.executionAttributes(); executionAttributes.putAttribute(InternalCoreExecutionAttribute.EXECUTION_ATTEMPT, 1); + Boolean configuredValue = executionAttributes.getAttribute( + SdkInternalExecutionAttribute.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED); + if (configuredValue == null) { + configuredValue = resolveRequestConfiguration(executionParams) + .option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED); + } + boolean concatenatedGzipStreamSupportEnabled = configuredValue == null || configuredValue; + AsyncResponseTransformer configuredTransformer = + ConfigurableAsyncResponseTransformer.configure(asyncResponseTransformer, + concatenatedGzipStreamSupportEnabled); + AsyncStreamingResponseHandler asyncStreamingResponseHandler = - new AsyncStreamingResponseHandler<>(asyncResponseTransformer); + new AsyncStreamingResponseHandler<>(configuredTransformer); // For streaming requests, prepare() should be called as early as possible to avoid NPE in client // See https://github.com/aws/aws-sdk-java-v2/issues/1268. We do this with a wrapper that caches the prepare diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java index 96173087602d..090dae9c571c 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java @@ -31,6 +31,7 @@ import software.amazon.awssdk.core.http.HttpResponseHandler; import software.amazon.awssdk.core.interceptor.ExecutionAttributes; import software.amazon.awssdk.core.interceptor.InterceptorContext; +import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute; import software.amazon.awssdk.core.internal.http.AmazonSyncHttpClient; import software.amazon.awssdk.core.internal.http.CombinedResponseHandler; import software.amazon.awssdk.core.internal.http.InterruptMonitor; @@ -212,7 +213,7 @@ private HttpResponseHandlerAdapter(HttpResponseHandler httpResponseHand public ReturnT handle(SdkHttpFullResponse response, ExecutionAttributes executionAttributes) throws Exception { OutputT resp = httpResponseHandler.handle(response, executionAttributes); AbortableInputStream content = response.content().orElseGet(AbortableInputStream::createEmpty); - AbortableInputStream body = wrapForConcatenatedGzipSupport(content); + AbortableInputStream body = wrapForConcatenatedGzipSupport(content, executionAttributes); return transformResponse(resp, body); } @@ -226,8 +227,11 @@ public boolean needsConnectionLeftOpen() { * around {@link java.util.zip.GZIPInputStream} truncating concatenated (multi-member) gzip at a member boundary. * Non-gzip content passes through unchanged and the original stream's {@code abort()} is preserved. */ - private static AbortableInputStream wrapForConcatenatedGzipSupport(AbortableInputStream content) { - return GzipAvailabilityInputStream.wrap(content, content); + private static AbortableInputStream wrapForConcatenatedGzipSupport( + AbortableInputStream content, ExecutionAttributes executionAttributes) { + boolean enabled = executionAttributes.getOptionalAttribute( + SdkInternalExecutionAttribute.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED).orElse(true); + return enabled ? GzipAvailabilityInputStream.wrap(content, content) : content; } diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/async/AsyncResponseTransformerUtilsTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/async/AsyncResponseTransformerUtilsTest.java new file mode 100644 index 000000000000..9de4666bae56 --- /dev/null +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/async/AsyncResponseTransformerUtilsTest.java @@ -0,0 +1,83 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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. + */ + +package software.amazon.awssdk.core.async; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.concurrent.CompletableFuture; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.core.ResponseInputStream; +import software.amazon.awssdk.core.SdkResponse; +import software.amazon.awssdk.core.SplittingTransformerConfiguration; +import software.amazon.awssdk.core.internal.async.ConfigurableAsyncResponseTransformer; +import software.amazon.awssdk.core.internal.async.InputStreamResponseTransformer; +import software.amazon.awssdk.core.protocol.VoidSdkResponse; +import software.amazon.awssdk.utils.Pair; +import software.amazon.awssdk.utils.async.SimplePublisher; + +class AsyncResponseTransformerUtilsTest { + + @Test + void wrapWithEndOfStreamFuture_whenSplitCalled_delegatesToOriginalTransformer() { + AsyncResponseTransformer transformer = mock(AsyncResponseTransformer.class); + AsyncResponseTransformer.SplitResult splitResult = + mock(AsyncResponseTransformer.SplitResult.class); + SplittingTransformerConfiguration splitConfiguration = + SplittingTransformerConfiguration.builder().bufferSizeInBytes(1024L).build(); + when(transformer.split(splitConfiguration)).thenReturn(splitResult); + + Pair, ?> wrapped = + AsyncResponseTransformerUtils.wrapWithEndOfStreamFuture(transformer); + + assertThat(wrapped.left()).isNotSameAs(transformer); + assertThat(wrapped.left().split(splitConfiguration)).isSameAs(splitResult); + verify(transformer).split(splitConfiguration); + } + + @Test + void configure_whenBlockingTransformerWrapped_forwardsConcatenatedGzipConfigurationToDelegate() throws IOException { + AsyncResponseTransformer> wrapped = + AsyncResponseTransformerUtils.wrapWithEndOfStreamFuture(new InputStreamResponseTransformer()).left(); + + AsyncResponseTransformer> configured = + ConfigurableAsyncResponseTransformer.configure(wrapped, false); + + assertThat(configured).isNotSameAs(wrapped); + assertThat(availableAfterGzipHeader(configured)).isZero(); + } + + private static int availableAfterGzipHeader( + AsyncResponseTransformer> transformer) throws IOException { + SimplePublisher body = new SimplePublisher<>(); + CompletableFuture> future = transformer.prepare(); + transformer.onResponse(VoidSdkResponse.builder().build()); + transformer.onStream(SdkPublisher.adapt(body)); + ResponseInputStream stream = future.join(); + body.send(ByteBuffer.wrap(new byte[] {0x1f, (byte) 0x8b, 0x08})); + stream.read(); + stream.read(); + stream.read(); + int result = stream.available(); + body.complete(); + stream.close(); + return result; + } +} diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/AsyncClientHandlerTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/AsyncClientHandlerTest.java index 2d4af1aedf9c..20e245d2fd4f 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/AsyncClientHandlerTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/AsyncClientHandlerTest.java @@ -19,9 +19,14 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import java.io.IOException; +import java.io.InputStream; +import java.nio.ByteBuffer; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -36,13 +41,21 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import software.amazon.awssdk.core.ResponseInputStream; import software.amazon.awssdk.core.SdkRequest; import software.amazon.awssdk.core.SdkResponse; +import software.amazon.awssdk.core.async.AsyncResponseTransformer; +import software.amazon.awssdk.core.async.AsyncResponseTransformerUtils; import software.amazon.awssdk.core.async.EmptyPublisher; +import software.amazon.awssdk.core.async.SdkPublisher; +import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.config.SdkClientOption; import software.amazon.awssdk.core.exception.SdkServiceException; import software.amazon.awssdk.core.http.HttpResponseHandler; +import software.amazon.awssdk.core.interceptor.Context; +import software.amazon.awssdk.core.interceptor.ExecutionAttributes; +import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; import software.amazon.awssdk.core.protocol.VoidSdkResponse; import software.amazon.awssdk.core.retry.RetryPolicy; import software.amazon.awssdk.core.runtime.transform.Marshaller; @@ -52,6 +65,7 @@ import software.amazon.awssdk.http.async.SdkAsyncHttpClient; import software.amazon.awssdk.http.async.SdkAsyncHttpResponseHandler; import software.amazon.awssdk.retries.DefaultRetryStrategy; +import software.amazon.awssdk.utils.async.SimplePublisher; import utils.HttpTestUtils; import utils.ValidSdkObjects; @@ -132,6 +146,77 @@ public void failedExecutionCallsErrorResponseHandler() throws Exception { verifyNoMoreInteractions(responseHandler); // Response handler is not called } + @Test + public void execute_whenGzipSupportDisabledAndTransformerWrapped_doesNotCoerceAvailable() throws Exception { + SdkClientConfiguration disabledConfiguration = + clientConfiguration().toBuilder() + .option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED, false) + .build(); + asyncClientHandler = new SdkAsyncClientHandler(disabledConfiguration); + SimplePublisher body = new SimplePublisher<>(); + AsyncResponseTransformer> transformer = + AsyncResponseTransformerUtils.wrapWithEndOfStreamFuture( + AsyncResponseTransformer.toBlockingInputStream()).left(); + + ResponseInputStream stream = driveStreamingExecute(transformer, body); + try { + body.send(ByteBuffer.wrap(new byte[] {0x1f, (byte) 0x8b, 0x08})); + readFully(stream, 3); + assertThat(stream.available()).isZero(); + } finally { + body.complete(); + stream.close(); + } + } + + @Test + public void execute_whenBeforeExecutionThrows_callsPrepareFirstAndPropagatesOriginalFailure() throws Exception { + IllegalStateException interceptorFailure = new IllegalStateException("beforeExecution failure"); + ExecutionInterceptor failingInterceptor = new ExecutionInterceptor() { + @Override + public void beforeExecution(Context.BeforeExecution context, ExecutionAttributes executionAttributes) { + throw interceptorFailure; + } + }; + SdkClientConfiguration configuration = + clientConfiguration().toBuilder() + .option(SdkClientOption.EXECUTION_INTERCEPTORS, + Arrays.asList(failingInterceptor)) + .build(); + asyncClientHandler = new SdkAsyncClientHandler(configuration); + AsyncResponseTransformer> transformer = + spy(AsyncResponseTransformer.toBlockingInputStream()); + + CompletableFuture> result = + asyncClientHandler.execute(clientExecutionParams(), transformer); + + verify(transformer).prepare(); + assertThatThrownBy(() -> result.get(1, TimeUnit.SECONDS)) + .hasRootCause(interceptorFailure); + } + + private ResponseInputStream driveStreamingExecute( + AsyncResponseTransformer> transformer, + SimplePublisher body) throws Exception { + ArgumentCaptor executeRequest = ArgumentCaptor.forClass(AsyncExecuteRequest.class); + expectRetrievalFromMocks(); + when(httpClient.execute(executeRequest.capture())).thenReturn(httpClientFuture); + when(responseHandler.handle(any(), any())).thenReturn(VoidSdkResponse.builder().build()); + + CompletableFuture> future = + asyncClientHandler.execute(clientExecutionParams(), transformer); + SdkAsyncHttpResponseHandler capturedHandler = executeRequest.getValue().responseHandler(); + capturedHandler.onHeaders(SdkHttpFullResponse.builder().statusCode(200).build()); + capturedHandler.onStream(SdkPublisher.adapt(body)); + return future.get(1, TimeUnit.SECONDS); + } + + private static void readFully(InputStream stream, int byteCount) throws IOException { + for (int i = 0; i < byteCount; i++) { + assertThat(stream.read()).isNotEqualTo(-1); + } + } + private void expectRetrievalFromMocks() { when(marshaller.marshall(request)).thenReturn(marshalledRequest); } diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java index b5e46f8fba8a..4ee02991ec9e 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; import org.junit.Before; @@ -47,6 +48,7 @@ import software.amazon.awssdk.core.exception.RetryableException; import software.amazon.awssdk.core.exception.SdkServiceException; import software.amazon.awssdk.core.http.HttpResponseHandler; +import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute; import software.amazon.awssdk.core.protocol.VoidSdkResponse; import software.amazon.awssdk.core.runtime.transform.Marshaller; import software.amazon.awssdk.core.sync.ResponseTransformer; @@ -189,6 +191,27 @@ public void execute_whenConcatenatedGzipHasTransientZeroAvailable_decodesAllMemb assertThat(decoded).isEqualTo("member-0member-1member-2member-3member-4"); } + @Test + public void execute_whenConcatenatedGzipSupportDisabled_doesNotCoerceAvailable() throws Exception { + mockSuccessfulStreamingCall(concatenatedGzip("member-0", "member-1")); + AtomicInteger availableAfterHeader = new AtomicInteger(-1); + when(responseTransformer.transform(any(SdkResponse.class), any(AbortableInputStream.class))) + .thenAnswer(invocation -> { + AbortableInputStream stream = invocation.getArgument(1); + stream.read(); + stream.read(); + stream.read(); + availableAfterHeader.set(stream.available()); + return null; + }); + ClientExecutionParams params = clientExecutionParams() + .putExecutionAttribute(SdkInternalExecutionAttribute.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED, false); + + syncClientHandler.execute(params, responseTransformer); + + assertThat(availableAfterHeader.get()).isZero(); + } + @Test public void execute_whenCustomTransformerDoesNotLeaveConnectionOpen_decodesAllGzipMembers() throws Exception { mockSuccessfulStreamingCall(concatenatedGzip("member-0", "member-1", "member-2")); diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java index 9ed16eaab223..d2a79191d480 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java @@ -32,6 +32,7 @@ import org.reactivestreams.Subscription; import software.amazon.awssdk.core.ResponseInputStream; import software.amazon.awssdk.core.SdkResponse; +import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.core.async.SdkPublisher; import software.amazon.awssdk.core.protocol.VoidSdkResponse; import software.amazon.awssdk.utils.async.SimplePublisher; @@ -157,4 +158,33 @@ void onStream_whenGzipDetected_coercesAvailableAboveZeroWhileOpen() throws IOExc assertThat(stream.available()).isGreaterThanOrEqualTo(1); } + @Test + void withConcatenatedGzipStreamSupportEnabled_whenDisabled_returnsImmutableCopyWithoutAvailableCoercion() + throws IOException { + InputStreamResponseTransformer original = new InputStreamResponseTransformer<>(); + AsyncResponseTransformer> disabled = + original.withConcatenatedGzipStreamSupportEnabled(false); + + assertThat(original.withConcatenatedGzipStreamSupportEnabled(true)).isSameAs(original); + assertThat(disabled).isNotSameAs(original); + assertThat(availableAfterGzipHeader(original)).isGreaterThanOrEqualTo(1); + assertThat(availableAfterGzipHeader(disabled)).isZero(); + } + + private static int availableAfterGzipHeader( + AsyncResponseTransformer> responseTransformer) throws IOException { + SimplePublisher body = new SimplePublisher<>(); + CompletableFuture> future = responseTransformer.prepare(); + responseTransformer.onResponse(VoidSdkResponse.builder().build()); + responseTransformer.onStream(SdkPublisher.adapt(body)); + ResponseInputStream stream = future.join(); + body.send(ByteBuffer.wrap(new byte[] {0x1f, (byte) 0x8b, 0x08})); + stream.read(); + stream.read(); + stream.read(); + int result = stream.available(); + body.complete(); + stream.close(); + return result; + } } diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/ConcatenatedGzipStreamSupportS3AsyncClient.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/ConcatenatedGzipStreamSupportS3AsyncClient.java new file mode 100644 index 000000000000..8343ddfc0c93 --- /dev/null +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/ConcatenatedGzipStreamSupportS3AsyncClient.java @@ -0,0 +1,60 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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. + */ + +package software.amazon.awssdk.services.s3.internal.client; + +import java.util.concurrent.CompletableFuture; +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.core.async.AsyncResponseTransformer; +import software.amazon.awssdk.core.internal.async.ConfigurableAsyncResponseTransformer; +import software.amazon.awssdk.services.s3.DelegatingS3AsyncClient; +import software.amazon.awssdk.services.s3.S3AsyncClient; +import software.amazon.awssdk.services.s3.model.GetObjectRequest; +import software.amazon.awssdk.services.s3.model.GetObjectResponse; +import software.amazon.awssdk.services.s3.presignedurl.AsyncPresignedUrlExtension; +import software.amazon.awssdk.services.s3.presignedurl.model.PresignedUrlDownloadRequest; + +/** + * Configures the caller's transformer before an S3 decorator can capture it by calling {@code split()}. + */ +@SdkInternalApi +final class ConcatenatedGzipStreamSupportS3AsyncClient extends DelegatingS3AsyncClient { + private final boolean enabled; + + ConcatenatedGzipStreamSupportS3AsyncClient(S3AsyncClient delegate, boolean enabled) { + super(delegate); + this.enabled = enabled; + } + + @Override + public CompletableFuture getObject( + GetObjectRequest request, AsyncResponseTransformer transformer) { + return super.getObject(request, ConfigurableAsyncResponseTransformer.configure(transformer, enabled)); + } + + @Override + public AsyncPresignedUrlExtension presignedUrlExtension() { + AsyncPresignedUrlExtension delegateExtension = super.presignedUrlExtension(); + return new AsyncPresignedUrlExtension() { + @Override + public CompletableFuture getObject( + PresignedUrlDownloadRequest request, + AsyncResponseTransformer transformer) { + return delegateExtension.getObject( + request, ConfigurableAsyncResponseTransformer.configure(transformer, enabled)); + } + }; + } +} diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java index 90f07d42eace..8e5316c494d5 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java @@ -20,6 +20,7 @@ import java.util.function.Predicate; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.checksums.RequestChecksumCalculation; +import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.config.SdkClientOption; import software.amazon.awssdk.services.s3.S3AsyncClient; @@ -56,6 +57,14 @@ public S3AsyncClient decorate(S3AsyncClient base, == RequestChecksumCalculation.WHEN_SUPPORTED; return MultipartS3AsyncClient.create(client, multipartConfiguration, checksumEnabled); })); + + boolean concatenatedGzipStreamSupportEnabled = !Boolean.FALSE.equals( + clientConfiguration.option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED)); + decorators.add(ConditionalDecorator.create( + client -> !concatenatedGzipStreamSupportEnabled, + client -> new ConcatenatedGzipStreamSupportS3AsyncClient( + client, concatenatedGzipStreamSupportEnabled))); + return ConditionalDecorator.decorate(base, decorators); } diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecoratorGzipSupportTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecoratorGzipSupportTest.java new file mode 100644 index 000000000000..2e13c859ab04 --- /dev/null +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecoratorGzipSupportTest.java @@ -0,0 +1,191 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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. + */ + +package software.amazon.awssdk.services.s3.internal.client; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.net.MalformedURLException; +import java.net.URL; +import java.util.concurrent.CompletableFuture; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.core.SplittingTransformerConfiguration; +import software.amazon.awssdk.core.async.AsyncResponseTransformer; +import software.amazon.awssdk.core.checksums.RequestChecksumCalculation; +import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; +import software.amazon.awssdk.core.client.config.SdkClientConfiguration; +import software.amazon.awssdk.core.client.config.SdkClientOption; +import software.amazon.awssdk.core.internal.async.ConfigurableAsyncResponseTransformer; +import software.amazon.awssdk.services.s3.S3AsyncClient; +import software.amazon.awssdk.services.s3.endpoints.S3ClientContextParams; +import software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionAsyncClient; +import software.amazon.awssdk.services.s3.internal.multipart.MultipartS3AsyncClient; +import software.amazon.awssdk.services.s3.model.GetObjectRequest; +import software.amazon.awssdk.services.s3.model.GetObjectResponse; +import software.amazon.awssdk.services.s3.multipart.MultipartConfiguration; +import software.amazon.awssdk.services.s3.presignedurl.AsyncPresignedUrlExtension; +import software.amazon.awssdk.services.s3.presignedurl.model.PresignedUrlDownloadRequest; +import software.amazon.awssdk.utils.AttributeMap; + +class S3AsyncClientDecoratorGzipSupportTest { + + @Test + void decorate_whenSupportDefaultOrExplicitlyEnabled_addsNoGzipDecorator() { + S3AsyncClient base = mock(S3AsyncClient.class); + + assertThat(decorate(base, false, null, null)).isSameAs(base); + assertThat(decorate(base, false, null, true)).isSameAs(base); + } + + @Test + void decorate_whenSupportDisabled_addsGzipDecorator() { + S3AsyncClient decorated = decorate(mock(S3AsyncClient.class), false, null, false); + + assertThat(decorated).isInstanceOf(ConcatenatedGzipStreamSupportS3AsyncClient.class); + } + + @Test + void decorate_whenSupportDisabledAndMultipartEnabled_addsGzipDecoratorOutermost() { + S3AsyncClient decorated = decorate(mock(S3AsyncClient.class), true, null, false); + + assertThat(decorated).isInstanceOf(ConcatenatedGzipStreamSupportS3AsyncClient.class); + assertThat(((ConcatenatedGzipStreamSupportS3AsyncClient) decorated).delegate()) + .isInstanceOf(MultipartS3AsyncClient.class); + } + + @Test + void decorate_whenSupportDisabledAndCrossRegionEnabled_addsGzipDecoratorOutermost() { + S3AsyncClient decorated = decorate(mock(S3AsyncClient.class), false, true, false); + + assertThat(decorated).isInstanceOf(ConcatenatedGzipStreamSupportS3AsyncClient.class); + assertThat(((ConcatenatedGzipStreamSupportS3AsyncClient) decorated).delegate()) + .isInstanceOf(S3CrossRegionAsyncClient.class); + } + + @Test + void getObject_whenSupportDisabled_configuresTransformerBeforeDelegating() { + S3AsyncClient base = mock(S3AsyncClient.class); + ConfigurableAsyncResponseTransformer original = + mock(ConfigurableAsyncResponseTransformer.class); + AsyncResponseTransformer configured = mock(AsyncResponseTransformer.class); + GetObjectRequest request = GetObjectRequest.builder().bucket("bucket").key("key").build(); + CompletableFuture expected = CompletableFuture.completedFuture("result"); + when(original.withConcatenatedGzipStreamSupportEnabled(false)).thenReturn(configured); + when(base.getObject(request, configured)).thenReturn(expected); + S3AsyncClient decorated = decorate(base, false, null, false); + + assertThat(decorated.getObject(request, original)).isSameAs(expected); + + verify(original).withConcatenatedGzipStreamSupportEnabled(false); + verify(base).getObject(request, configured); + } + + @Test + void presignedGetObject_whenSupportDisabled_configuresTransformerBeforeDelegating() throws MalformedURLException { + S3AsyncClient base = mock(S3AsyncClient.class); + AsyncPresignedUrlExtension baseExtension = mock(AsyncPresignedUrlExtension.class); + when(base.presignedUrlExtension()).thenReturn(baseExtension); + ConfigurableAsyncResponseTransformer original = + mock(ConfigurableAsyncResponseTransformer.class); + AsyncResponseTransformer configured = mock(AsyncResponseTransformer.class); + PresignedUrlDownloadRequest request = + PresignedUrlDownloadRequest.builder() + .presignedUrl(new URL("https://s3.amazonaws.com/bucket/key?signature=abc")) + .build(); + CompletableFuture expected = CompletableFuture.completedFuture("result"); + when(original.withConcatenatedGzipStreamSupportEnabled(false)).thenReturn(configured); + when(baseExtension.getObject(request, configured)).thenReturn(expected); + S3AsyncClient decorated = decorate(base, false, null, false); + + assertThat(decorated.presignedUrlExtension().getObject(request, original)).isSameAs(expected); + + verify(original).withConcatenatedGzipStreamSupportEnabled(false); + verify(baseExtension).getObject(request, configured); + } + + @Test + void getObject_whenSupportDisabledAndMultipartEnabled_configuresTransformerBeforeSplit() { + S3AsyncClient base = mock(S3AsyncClient.class); + ConfigurableAsyncResponseTransformer original = + mock(ConfigurableAsyncResponseTransformer.class); + AsyncResponseTransformer configured = mock(AsyncResponseTransformer.class); + IllegalStateException splitFailure = new IllegalStateException("configured transformer split"); + when(original.withConcatenatedGzipStreamSupportEnabled(false)).thenReturn(configured); + when(configured.split(any(SplittingTransformerConfiguration.class))).thenThrow(splitFailure); + S3AsyncClient decorated = decorate(base, true, null, false); + + assertThatThrownBy(() -> decorated.getObject( + GetObjectRequest.builder().bucket("bucket").key("key").build(), original)).isSameAs(splitFailure); + + verify(original).withConcatenatedGzipStreamSupportEnabled(false); + verify(configured).split(any(SplittingTransformerConfiguration.class)); + verify(original, never()).split(any(SplittingTransformerConfiguration.class)); + } + + @Test + void presignedGetObject_whenSupportDisabledAndMultipartEnabled_configuresTransformerBeforeSplit() + throws MalformedURLException { + S3AsyncClient base = mock(S3AsyncClient.class); + when(base.presignedUrlExtension()).thenReturn(mock(AsyncPresignedUrlExtension.class)); + ConfigurableAsyncResponseTransformer original = + mock(ConfigurableAsyncResponseTransformer.class); + AsyncResponseTransformer configured = mock(AsyncResponseTransformer.class); + IllegalStateException splitFailure = new IllegalStateException("configured transformer split"); + when(original.withConcatenatedGzipStreamSupportEnabled(false)).thenReturn(configured); + when(configured.split(any(SplittingTransformerConfiguration.class))).thenThrow(splitFailure); + S3AsyncClient decorated = decorate(base, true, null, false); + PresignedUrlDownloadRequest request = + PresignedUrlDownloadRequest.builder() + .presignedUrl(new URL("https://s3.amazonaws.com/bucket/key?signature=abc")) + .build(); + + assertThatThrownBy(() -> decorated.presignedUrlExtension().getObject(request, original)).isSameAs(splitFailure); + + verify(original).withConcatenatedGzipStreamSupportEnabled(false); + verify(configured).split(any(SplittingTransformerConfiguration.class)); + verify(original, never()).split(any(SplittingTransformerConfiguration.class)); + } + + private static S3AsyncClient decorate(S3AsyncClient base, + boolean multipartEnabled, + Boolean crossRegionEnabled, + Boolean gzipSupportEnabled) { + AttributeMap.Builder context = AttributeMap.builder(); + if (multipartEnabled) { + context.put(S3AsyncClientDecorator.MULTIPART_ENABLED_KEY, true); + context.put(S3AsyncClientDecorator.MULTIPART_CONFIGURATION_KEY, + MultipartConfiguration.builder().build()); + } + if (crossRegionEnabled != null) { + context.put(S3ClientContextParams.CROSS_REGION_ACCESS_ENABLED, crossRegionEnabled); + } + SdkClientConfiguration.Builder configuration = + SdkClientConfiguration.builder() + .option(SdkClientOption.CLIENT_CONTEXT_PARAMS, context.build()) + .option(SdkClientOption.REQUEST_CHECKSUM_CALCULATION, + RequestChecksumCalculation.WHEN_SUPPORTED); + if (gzipSupportEnabled != null) { + configuration.option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED, + gzipSupportEnabled); + } + return new S3AsyncClientDecorator().decorate(base, configuration.build()); + } +} From 916ba629e160587ac61ea8282e57e171168235e7 Mon Sep 17 00:00:00 2001 From: jencymaryjoseph <35571282+jencymaryjoseph@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:43:56 -0700 Subject: [PATCH 4/4] Replace GZIP client option with transformer opt-in APIs This reverts commit f902dc280e894019100fef8943419c3131b312a1. --- .../bugfix-AWSSDKforJavav2-c8ce5ba.json | 2 +- .../builder/AwsDefaultClientBuilder.java | 1 - .../internal/AwsExecutionContextBuilder.java | 2 - .../core/async/AsyncResponseTransformer.java | 22 +- .../AsyncResponseTransformerListener.java | 15 +- .../config/SdkAdvancedClientOption.java | 8 - .../SdkInternalExecutionAttribute.java | 6 - .../ConfigurableAsyncResponseTransformer.java | 43 ---- .../async/InputStreamResponseTransformer.java | 25 +-- .../handler/BaseAsyncClientHandler.java | 16 +- .../handler/BaseSyncClientHandler.java | 18 +- .../awssdk/core/sync/ResponseTransformer.java | 59 +++++- .../AsyncResponseTransformerUtilsTest.java | 83 -------- .../handler/AsyncClientHandlerTest.java | 85 -------- .../client/handler/SyncClientHandlerTest.java | 114 ----------- .../InputStreamResponseTransformerTest.java | 127 +++++++++--- .../ResponseTransformerToInputStreamTest.java | 146 +++++++++++++ ...tenatedGzipStreamSupportS3AsyncClient.java | 60 ------ .../client/S3AsyncClientDecorator.java | 9 - .../GetObjectConcatenatedGzipTest.java | 8 +- ...S3AsyncClientDecoratorGzipSupportTest.java | 191 ------------------ 21 files changed, 333 insertions(+), 707 deletions(-) delete mode 100644 core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/ConfigurableAsyncResponseTransformer.java delete mode 100644 core/sdk-core/src/test/java/software/amazon/awssdk/core/async/AsyncResponseTransformerUtilsTest.java create mode 100644 core/sdk-core/src/test/java/software/amazon/awssdk/core/sync/ResponseTransformerToInputStreamTest.java delete mode 100644 services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/ConcatenatedGzipStreamSupportS3AsyncClient.java delete mode 100644 services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecoratorGzipSupportTest.java diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json b/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json index ae0b44d35f50..0c356ccc8bce 100644 --- a/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-c8ce5ba.json @@ -2,5 +2,5 @@ "type": "bugfix", "category": "AWS SDK for Java v2", "contributor": "", - "description": "Fixed an issue where concatenated (multi-member) gzip response streams could be truncated to the first member when decoded with GZIPInputStream, because a transient available()==0 at a member boundary was treated as end of stream. The corrected behavior is enabled by default and can be disabled with SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED." + "description": "Added opt-in GZIPInputStream compatibility for blocking response streams to prevent concatenated (multi-member) gzip responses from being truncated when available() temporarily returns 0 at a member boundary. Enable it with ResponseTransformer.toInputStream(true) or AsyncResponseTransformer.toBlockingInputStream(true)." } diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java index 7542ceb232ad..92036575eeb1 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java @@ -138,7 +138,6 @@ protected final SdkClientConfiguration mergeChildDefaults(SdkClientConfiguration SdkClientConfiguration config = mergeServiceDefaults(configuration); config = config.merge(c -> c.option(AwsAdvancedClientOption.ENABLE_DEFAULT_REGION_DETECTION, true) .option(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION, false) - .option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED, true) .option(AwsClientOption.SERVICE_SIGNING_NAME, signingName()) .option(SdkClientOption.SERVICE_NAME, serviceName()) .option(AwsClientOption.ENDPOINT_PREFIX, serviceEndpointPrefix())); diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java index 4d26e18986de..9a6434186dd2 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java @@ -130,8 +130,6 @@ private AwsExecutionContextBuilder() { clientConfig.option(SdkClientOption.CLIENT_CONTEXT_PARAMS)) .putAttribute(SdkInternalExecutionAttribute.DISABLE_HOST_PREFIX_INJECTION, clientConfig.option(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION)) - .putAttribute(SdkInternalExecutionAttribute.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED, - clientConfig.option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED)) .putAttribute(SdkInternalExecutionAttribute.SDK_CLIENT, clientConfig.option(SdkClientOption.SDK_CLIENT)) .putAttribute(SdkExecutionAttribute.SIGNER_OVERRIDDEN, clientConfig.option(SdkClientOption.SIGNER_OVERRIDDEN)) .putAttribute(AwsExecutionAttribute.USE_GLOBAL_ENDPOINT, diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/AsyncResponseTransformer.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/AsyncResponseTransformer.java index 70ff1e6aef50..a575f68ae1da 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/AsyncResponseTransformer.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/AsyncResponseTransformer.java @@ -386,7 +386,27 @@ ResponsePublisher> toPublisher(Duration timeout) { */ static AsyncResponseTransformer> toBlockingInputStream() { - return new InputStreamResponseTransformer<>(); + return new InputStreamResponseTransformer<>(false); + } + + /** + * Creates an {@link AsyncResponseTransformer} that allows reading the response body content as an {@link InputStream}. + * You are responsible for performing blocking reads from this input stream and closing the stream when you are finished. + * + *

When enabled, gzip response streams are adapted so that {@link InputStream#available()} does not temporarily return + * {@code 0} while the stream is still open. This works around {@link java.util.zip.GZIPInputStream} treating a temporary + * {@code 0} at a concatenated gzip member boundary as the end of the complete stream. Because this can cause a read after + * {@code available()} to block, it should only be enabled when the response will be read with {@code GZIPInputStream}. + * + * @param gzipInputStreamCompatibilityEnabled Whether to enable {@code GZIPInputStream} compatibility for concatenated gzip. + * @param Type of unmarshalled response POJO. + * @return AsyncResponseTransformer instance. + * @see #toBlockingInputStream() + */ + static + AsyncResponseTransformer> toBlockingInputStream( + boolean gzipInputStreamCompatibilityEnabled) { + return new InputStreamResponseTransformer<>(gzipInputStreamCompatibilityEnabled); } /** diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/listener/AsyncResponseTransformerListener.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/listener/AsyncResponseTransformerListener.java index dfaacc431f38..f1612e1b1f3e 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/listener/AsyncResponseTransformerListener.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/async/listener/AsyncResponseTransformerListener.java @@ -23,7 +23,6 @@ import software.amazon.awssdk.core.SplittingTransformerConfiguration; import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.core.async.SdkPublisher; -import software.amazon.awssdk.core.internal.async.ConfigurableAsyncResponseTransformer; import software.amazon.awssdk.utils.Logger; import software.amazon.awssdk.utils.Validate; @@ -66,9 +65,7 @@ static AsyncResponseTransformer wrap( } @SdkInternalApi - final class NotifyingAsyncResponseTransformer - implements AsyncResponseTransformer, - ConfigurableAsyncResponseTransformer { + final class NotifyingAsyncResponseTransformer implements AsyncResponseTransformer { private static final Logger log = Logger.loggerFor(NotifyingAsyncResponseTransformer.class); private final AsyncResponseTransformer delegate; @@ -84,16 +81,6 @@ public AsyncResponseTransformer getDelegate() { return delegate; } - @Override - public AsyncResponseTransformer withConcatenatedGzipStreamSupportEnabled(boolean enabled) { - AsyncResponseTransformer configuredDelegate = - ConfigurableAsyncResponseTransformer.configure(delegate, enabled); - if (configuredDelegate == delegate) { - return this; - } - return new NotifyingAsyncResponseTransformer<>(configuredDelegate, listener); - } - @Override public CompletableFuture prepare() { return delegate.prepare(); diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/SdkAdvancedClientOption.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/SdkAdvancedClientOption.java index b5204963038d..28924e9aea2d 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/SdkAdvancedClientOption.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/SdkAdvancedClientOption.java @@ -88,14 +88,6 @@ public class SdkAdvancedClientOption extends ClientOption { public static final SdkAdvancedClientOption DISABLE_HOST_PREFIX_INJECTION = new SdkAdvancedClientOption<>(Boolean.class); - /** - * Whether response streams support concatenated (multi-member) gzip by preventing a temporary - * {@code InputStream.available() == 0} from being interpreted as the end of the gzip stream. - * Enabled by default. - */ - public static final SdkAdvancedClientOption CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED = - new SdkAdvancedClientOption<>(Boolean.class); - protected SdkAdvancedClientOption(Class valueClass) { super(valueClass); OPTIONS.add(this); diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/SdkInternalExecutionAttribute.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/SdkInternalExecutionAttribute.java index 4114f05b1a36..eea10003c89b 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/SdkInternalExecutionAttribute.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/SdkInternalExecutionAttribute.java @@ -87,12 +87,6 @@ public final class SdkInternalExecutionAttribute extends SdkExecutionAttribute { public static final ExecutionAttribute DISABLE_HOST_PREFIX_INJECTION = new ExecutionAttribute<>("DisableHostPrefixInjection"); - /** - * Whether concatenated-gzip response stream support is enabled. - */ - public static final ExecutionAttribute CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED = - new ExecutionAttribute<>("ConcatenatedGzipStreamSupportEnabled"); - /** * Key to indicate if the Http Checksums that are valid for an operation. */ diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/ConfigurableAsyncResponseTransformer.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/ConfigurableAsyncResponseTransformer.java deleted file mode 100644 index 72c6a46ad4be..000000000000 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/ConfigurableAsyncResponseTransformer.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://aws.amazon.com/apache2.0 - * - * or in the "license" file accompanying this file. This file 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. - */ - -package software.amazon.awssdk.core.internal.async; - -import software.amazon.awssdk.annotations.SdkInternalApi; -import software.amazon.awssdk.core.async.AsyncResponseTransformer; - -/** - * Internal contract for SDK response transformers that support concatenated-gzip configuration. - */ -@SdkInternalApi -public interface ConfigurableAsyncResponseTransformer - extends AsyncResponseTransformer { - - AsyncResponseTransformer withConcatenatedGzipStreamSupportEnabled(boolean enabled); - - /** - * Configures a transformer when it supports this internal contract. Customer transformers are returned unchanged. - */ - @SuppressWarnings("unchecked") - static AsyncResponseTransformer configure( - AsyncResponseTransformer transformer, boolean enabled) { - - if (transformer instanceof ConfigurableAsyncResponseTransformer) { - return ((ConfigurableAsyncResponseTransformer) transformer) - .withConcatenatedGzipStreamSupportEnabled(enabled); - } - return transformer; - } -} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java index eb1b5d9f913d..8b6510dfe779 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformer.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.core.internal.async; +import java.io.InputStream; import java.nio.ByteBuffer; import java.util.concurrent.CompletableFuture; import org.reactivestreams.Subscriber; @@ -25,7 +26,6 @@ import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.core.async.SdkPublisher; import software.amazon.awssdk.core.internal.io.GzipAvailabilityInputStream; -import software.amazon.awssdk.http.AbortableInputStream; import software.amazon.awssdk.http.async.AbortableInputStreamSubscriber; /** @@ -35,20 +35,19 @@ */ @SdkInternalApi public class InputStreamResponseTransformer - implements AsyncResponseTransformer>, - ConfigurableAsyncResponseTransformer> { + implements AsyncResponseTransformer> { private volatile CompletableFuture> future; private volatile ResponseT response; private volatile WaitForSubscribeOnErrorWrapper subscriber; - private final boolean concatenatedGzipStreamSupportEnabled; + private final boolean gzipInputStreamCompatibilityEnabled; public InputStreamResponseTransformer() { - this(true); + this(false); } - private InputStreamResponseTransformer(boolean concatenatedGzipStreamSupportEnabled) { - this.concatenatedGzipStreamSupportEnabled = concatenatedGzipStreamSupportEnabled; + public InputStreamResponseTransformer(boolean gzipInputStreamCompatibilityEnabled) { + this.gzipInputStreamCompatibilityEnabled = gzipInputStreamCompatibilityEnabled; } @Override @@ -71,9 +70,9 @@ public void onStream(SdkPublisher publisher) { this.subscriber = waitForSubscribeSubscriber; publisher.subscribe(waitForSubscribeSubscriber); - AbortableInputStream content = concatenatedGzipStreamSupportEnabled - ? GzipAvailabilityInputStream.wrap(inputStreamSubscriber, inputStreamSubscriber) - : AbortableInputStream.create(inputStreamSubscriber, inputStreamSubscriber); + InputStream content = gzipInputStreamCompatibilityEnabled + ? GzipAvailabilityInputStream.wrap(inputStreamSubscriber, inputStreamSubscriber) + : inputStreamSubscriber; future.complete(new ResponseInputStream<>(response, content)); } @@ -90,12 +89,6 @@ public String name() { return TransformerType.STREAM.getName(); } - @Override - public AsyncResponseTransformer> - withConcatenatedGzipStreamSupportEnabled(boolean enabled) { - return concatenatedGzipStreamSupportEnabled == enabled ? this : new InputStreamResponseTransformer<>(enabled); - } - // Simple wrapper subscriber that ensures we don't forward the `onError` to the delegate until onSubscribe is called, to be // compliant with the reactive streams spec. We use onError for forwarding the exception given to exceptionOccurred. private static final class WaitForSubscribeOnErrorWrapper implements Subscriber { diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseAsyncClientHandler.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseAsyncClientHandler.java index 682f6c847dc3..0c2f91a3a424 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseAsyncClientHandler.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseAsyncClientHandler.java @@ -27,7 +27,6 @@ import software.amazon.awssdk.core.SdkResponse; import software.amazon.awssdk.core.async.AsyncRequestBody; import software.amazon.awssdk.core.async.AsyncResponseTransformer; -import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.handler.AsyncClientHandler; import software.amazon.awssdk.core.client.handler.ClientExecutionParams; @@ -37,9 +36,7 @@ import software.amazon.awssdk.core.http.HttpResponseHandler; import software.amazon.awssdk.core.interceptor.ExecutionAttributes; import software.amazon.awssdk.core.interceptor.InterceptorContext; -import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute; import software.amazon.awssdk.core.internal.InternalCoreExecutionAttribute; -import software.amazon.awssdk.core.internal.async.ConfigurableAsyncResponseTransformer; import software.amazon.awssdk.core.internal.http.AmazonAsyncHttpClient; import software.amazon.awssdk.core.internal.http.IdempotentAsyncResponseHandler; import software.amazon.awssdk.core.internal.http.TransformingAsyncResponseHandler; @@ -101,19 +98,8 @@ public Complet ExecutionAttributes executionAttributes = executionParams.executionAttributes(); executionAttributes.putAttribute(InternalCoreExecutionAttribute.EXECUTION_ATTEMPT, 1); - Boolean configuredValue = executionAttributes.getAttribute( - SdkInternalExecutionAttribute.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED); - if (configuredValue == null) { - configuredValue = resolveRequestConfiguration(executionParams) - .option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED); - } - boolean concatenatedGzipStreamSupportEnabled = configuredValue == null || configuredValue; - AsyncResponseTransformer configuredTransformer = - ConfigurableAsyncResponseTransformer.configure(asyncResponseTransformer, - concatenatedGzipStreamSupportEnabled); - AsyncStreamingResponseHandler asyncStreamingResponseHandler = - new AsyncStreamingResponseHandler<>(configuredTransformer); + new AsyncStreamingResponseHandler<>(asyncResponseTransformer); // For streaming requests, prepare() should be called as early as possible to avoid NPE in client // See https://github.com/aws/aws-sdk-java-v2/issues/1268. We do this with a wrapper that caches the prepare diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java index 090dae9c571c..03ea0683397b 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseSyncClientHandler.java @@ -31,11 +31,9 @@ import software.amazon.awssdk.core.http.HttpResponseHandler; import software.amazon.awssdk.core.interceptor.ExecutionAttributes; import software.amazon.awssdk.core.interceptor.InterceptorContext; -import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute; import software.amazon.awssdk.core.internal.http.AmazonSyncHttpClient; import software.amazon.awssdk.core.internal.http.CombinedResponseHandler; import software.amazon.awssdk.core.internal.http.InterruptMonitor; -import software.amazon.awssdk.core.internal.io.GzipAvailabilityInputStream; import software.amazon.awssdk.core.metrics.CoreMetric; import software.amazon.awssdk.core.sync.RequestBody; import software.amazon.awssdk.core.sync.ResponseTransformer; @@ -212,9 +210,7 @@ private HttpResponseHandlerAdapter(HttpResponseHandler httpResponseHand @Override public ReturnT handle(SdkHttpFullResponse response, ExecutionAttributes executionAttributes) throws Exception { OutputT resp = httpResponseHandler.handle(response, executionAttributes); - AbortableInputStream content = response.content().orElseGet(AbortableInputStream::createEmpty); - AbortableInputStream body = wrapForConcatenatedGzipSupport(content, executionAttributes); - return transformResponse(resp, body); + return transformResponse(resp, response.content().orElseGet(AbortableInputStream::createEmpty)); } @Override @@ -222,18 +218,6 @@ public boolean needsConnectionLeftOpen() { return responseTransformer.needsConnectionLeftOpen(); } - /** - * Wraps a gzip response body so {@code available()} never returns {@code 0} while the stream is open, working - * around {@link java.util.zip.GZIPInputStream} truncating concatenated (multi-member) gzip at a member boundary. - * Non-gzip content passes through unchanged and the original stream's {@code abort()} is preserved. - */ - private static AbortableInputStream wrapForConcatenatedGzipSupport( - AbortableInputStream content, ExecutionAttributes executionAttributes) { - boolean enabled = executionAttributes.getOptionalAttribute( - SdkInternalExecutionAttribute.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED).orElse(true); - return enabled ? GzipAvailabilityInputStream.wrap(content, content) : content; - } - private ReturnT transformResponse(OutputT resp, AbortableInputStream inputStream) throws Exception { try { diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/ResponseTransformer.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/ResponseTransformer.java index 279c29c29c3c..174381a73538 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/ResponseTransformer.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/ResponseTransformer.java @@ -36,6 +36,7 @@ import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.core.exception.SdkException; import software.amazon.awssdk.core.internal.http.InterruptMonitor; +import software.amazon.awssdk.core.internal.io.GzipAvailabilityInputStream; import software.amazon.awssdk.core.retry.RetryPolicy; import software.amazon.awssdk.http.AbortableInputStream; import software.amazon.awssdk.utils.IoUtils; @@ -261,17 +262,30 @@ public String name() { * @see #toInputStream(Duration) */ static ResponseTransformer> toInputStream() { - return unmanaged(new ResponseTransformer>() { - @Override - public ResponseInputStream transform(ResponseT response, AbortableInputStream inputStream) { - return new ResponseInputStream<>(response, inputStream); - } + return toInputStream(null, false); + } - @Override - public String name() { - return TransformerType.STREAM.getName(); - } - }); + /** + * Creates a response transformer that returns an unmanaged input stream with the response content. This input stream must + * be explicitly closed to release the connection. + * + *

The stream has the default first-read timeout of 60 seconds. Use {@link #toInputStream(Duration, boolean)} to specify a + * custom timeout. + * + *

When enabled, gzip response streams are adapted so that {@link InputStream#available()} does not temporarily return + * {@code 0} while the stream is still open. This works around {@link java.util.zip.GZIPInputStream} treating a temporary + * {@code 0} at a concatenated gzip member boundary as the end of the complete stream. Because this can cause a read after + * {@code available()} to block, it should only be enabled when the response will be read with {@code GZIPInputStream}. + * + * @param gzipInputStreamCompatibilityEnabled Whether to enable {@code GZIPInputStream} compatibility for concatenated gzip. + * @param Type of unmarshalled response POJO. + * @return ResponseTransformer instance. + * @see #toInputStream() + * @see #toInputStream(Duration, boolean) + */ + static ResponseTransformer> toInputStream( + boolean gzipInputStreamCompatibilityEnabled) { + return toInputStream(null, gzipInputStreamCompatibilityEnabled); } /** @@ -289,10 +303,33 @@ public String name() { * @see #toInputStream() */ static ResponseTransformer> toInputStream(Duration timeout) { + return toInputStream(timeout, false); + } + + /** + * Creates a response transformer that returns an unmanaged input stream with the response content and a custom timeout. + * This input stream must be explicitly closed to release the connection. + * + *

When enabled, gzip response streams are adapted so that {@link InputStream#available()} does not temporarily return + * {@code 0} while the stream is still open. This works around {@link java.util.zip.GZIPInputStream} treating a temporary + * {@code 0} at a concatenated gzip member boundary as the end of the complete stream. Because this can cause a read after + * {@code available()} to block, it should only be enabled when the response will be read with {@code GZIPInputStream}. + * + * @param timeout Maximum time to wait for first read operation before aborting. Use {@link Duration#ZERO} or a negative + * {@link Duration} to disable timeout. + * @param gzipInputStreamCompatibilityEnabled Whether to enable {@code GZIPInputStream} compatibility for concatenated gzip. + * @param Type of unmarshalled response POJO. + * @return ResponseTransformer instance. + */ + static ResponseTransformer> toInputStream( + Duration timeout, boolean gzipInputStreamCompatibilityEnabled) { return unmanaged(new ResponseTransformer>() { @Override public ResponseInputStream transform(ResponseT response, AbortableInputStream inputStream) { - return new ResponseInputStream<>(response, inputStream, timeout); + AbortableInputStream content = gzipInputStreamCompatibilityEnabled + ? GzipAvailabilityInputStream.wrap(inputStream, inputStream) + : inputStream; + return new ResponseInputStream<>(response, content, timeout); } @Override diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/async/AsyncResponseTransformerUtilsTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/async/AsyncResponseTransformerUtilsTest.java deleted file mode 100644 index 9de4666bae56..000000000000 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/async/AsyncResponseTransformerUtilsTest.java +++ /dev/null @@ -1,83 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://aws.amazon.com/apache2.0 - * - * or in the "license" file accompanying this file. This file 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. - */ - -package software.amazon.awssdk.core.async; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import java.io.IOException; -import java.nio.ByteBuffer; -import java.util.concurrent.CompletableFuture; -import org.junit.jupiter.api.Test; -import software.amazon.awssdk.core.ResponseInputStream; -import software.amazon.awssdk.core.SdkResponse; -import software.amazon.awssdk.core.SplittingTransformerConfiguration; -import software.amazon.awssdk.core.internal.async.ConfigurableAsyncResponseTransformer; -import software.amazon.awssdk.core.internal.async.InputStreamResponseTransformer; -import software.amazon.awssdk.core.protocol.VoidSdkResponse; -import software.amazon.awssdk.utils.Pair; -import software.amazon.awssdk.utils.async.SimplePublisher; - -class AsyncResponseTransformerUtilsTest { - - @Test - void wrapWithEndOfStreamFuture_whenSplitCalled_delegatesToOriginalTransformer() { - AsyncResponseTransformer transformer = mock(AsyncResponseTransformer.class); - AsyncResponseTransformer.SplitResult splitResult = - mock(AsyncResponseTransformer.SplitResult.class); - SplittingTransformerConfiguration splitConfiguration = - SplittingTransformerConfiguration.builder().bufferSizeInBytes(1024L).build(); - when(transformer.split(splitConfiguration)).thenReturn(splitResult); - - Pair, ?> wrapped = - AsyncResponseTransformerUtils.wrapWithEndOfStreamFuture(transformer); - - assertThat(wrapped.left()).isNotSameAs(transformer); - assertThat(wrapped.left().split(splitConfiguration)).isSameAs(splitResult); - verify(transformer).split(splitConfiguration); - } - - @Test - void configure_whenBlockingTransformerWrapped_forwardsConcatenatedGzipConfigurationToDelegate() throws IOException { - AsyncResponseTransformer> wrapped = - AsyncResponseTransformerUtils.wrapWithEndOfStreamFuture(new InputStreamResponseTransformer()).left(); - - AsyncResponseTransformer> configured = - ConfigurableAsyncResponseTransformer.configure(wrapped, false); - - assertThat(configured).isNotSameAs(wrapped); - assertThat(availableAfterGzipHeader(configured)).isZero(); - } - - private static int availableAfterGzipHeader( - AsyncResponseTransformer> transformer) throws IOException { - SimplePublisher body = new SimplePublisher<>(); - CompletableFuture> future = transformer.prepare(); - transformer.onResponse(VoidSdkResponse.builder().build()); - transformer.onStream(SdkPublisher.adapt(body)); - ResponseInputStream stream = future.join(); - body.send(ByteBuffer.wrap(new byte[] {0x1f, (byte) 0x8b, 0x08})); - stream.read(); - stream.read(); - stream.read(); - int result = stream.available(); - body.complete(); - stream.close(); - return result; - } -} diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/AsyncClientHandlerTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/AsyncClientHandlerTest.java index 20e245d2fd4f..2d4af1aedf9c 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/AsyncClientHandlerTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/AsyncClientHandlerTest.java @@ -19,14 +19,9 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; -import java.io.IOException; -import java.io.InputStream; -import java.nio.ByteBuffer; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -41,21 +36,13 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import software.amazon.awssdk.core.ResponseInputStream; import software.amazon.awssdk.core.SdkRequest; import software.amazon.awssdk.core.SdkResponse; -import software.amazon.awssdk.core.async.AsyncResponseTransformer; -import software.amazon.awssdk.core.async.AsyncResponseTransformerUtils; import software.amazon.awssdk.core.async.EmptyPublisher; -import software.amazon.awssdk.core.async.SdkPublisher; -import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.config.SdkClientOption; import software.amazon.awssdk.core.exception.SdkServiceException; import software.amazon.awssdk.core.http.HttpResponseHandler; -import software.amazon.awssdk.core.interceptor.Context; -import software.amazon.awssdk.core.interceptor.ExecutionAttributes; -import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; import software.amazon.awssdk.core.protocol.VoidSdkResponse; import software.amazon.awssdk.core.retry.RetryPolicy; import software.amazon.awssdk.core.runtime.transform.Marshaller; @@ -65,7 +52,6 @@ import software.amazon.awssdk.http.async.SdkAsyncHttpClient; import software.amazon.awssdk.http.async.SdkAsyncHttpResponseHandler; import software.amazon.awssdk.retries.DefaultRetryStrategy; -import software.amazon.awssdk.utils.async.SimplePublisher; import utils.HttpTestUtils; import utils.ValidSdkObjects; @@ -146,77 +132,6 @@ public void failedExecutionCallsErrorResponseHandler() throws Exception { verifyNoMoreInteractions(responseHandler); // Response handler is not called } - @Test - public void execute_whenGzipSupportDisabledAndTransformerWrapped_doesNotCoerceAvailable() throws Exception { - SdkClientConfiguration disabledConfiguration = - clientConfiguration().toBuilder() - .option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED, false) - .build(); - asyncClientHandler = new SdkAsyncClientHandler(disabledConfiguration); - SimplePublisher body = new SimplePublisher<>(); - AsyncResponseTransformer> transformer = - AsyncResponseTransformerUtils.wrapWithEndOfStreamFuture( - AsyncResponseTransformer.toBlockingInputStream()).left(); - - ResponseInputStream stream = driveStreamingExecute(transformer, body); - try { - body.send(ByteBuffer.wrap(new byte[] {0x1f, (byte) 0x8b, 0x08})); - readFully(stream, 3); - assertThat(stream.available()).isZero(); - } finally { - body.complete(); - stream.close(); - } - } - - @Test - public void execute_whenBeforeExecutionThrows_callsPrepareFirstAndPropagatesOriginalFailure() throws Exception { - IllegalStateException interceptorFailure = new IllegalStateException("beforeExecution failure"); - ExecutionInterceptor failingInterceptor = new ExecutionInterceptor() { - @Override - public void beforeExecution(Context.BeforeExecution context, ExecutionAttributes executionAttributes) { - throw interceptorFailure; - } - }; - SdkClientConfiguration configuration = - clientConfiguration().toBuilder() - .option(SdkClientOption.EXECUTION_INTERCEPTORS, - Arrays.asList(failingInterceptor)) - .build(); - asyncClientHandler = new SdkAsyncClientHandler(configuration); - AsyncResponseTransformer> transformer = - spy(AsyncResponseTransformer.toBlockingInputStream()); - - CompletableFuture> result = - asyncClientHandler.execute(clientExecutionParams(), transformer); - - verify(transformer).prepare(); - assertThatThrownBy(() -> result.get(1, TimeUnit.SECONDS)) - .hasRootCause(interceptorFailure); - } - - private ResponseInputStream driveStreamingExecute( - AsyncResponseTransformer> transformer, - SimplePublisher body) throws Exception { - ArgumentCaptor executeRequest = ArgumentCaptor.forClass(AsyncExecuteRequest.class); - expectRetrievalFromMocks(); - when(httpClient.execute(executeRequest.capture())).thenReturn(httpClientFuture); - when(responseHandler.handle(any(), any())).thenReturn(VoidSdkResponse.builder().build()); - - CompletableFuture> future = - asyncClientHandler.execute(clientExecutionParams(), transformer); - SdkAsyncHttpResponseHandler capturedHandler = executeRequest.getValue().responseHandler(); - capturedHandler.onHeaders(SdkHttpFullResponse.builder().statusCode(200).build()); - capturedHandler.onStream(SdkPublisher.adapt(body)); - return future.get(1, TimeUnit.SECONDS); - } - - private static void readFully(InputStream stream, int byteCount) throws IOException { - for (int i = 0; i < byteCount; i++) { - assertThat(stream.read()).isNotEqualTo(-1); - } - } - private void expectRetrievalFromMocks() { when(marshaller.marshall(request)).thenReturn(marshalledRequest); } diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java index 4ee02991ec9e..efbc983e6b0e 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java @@ -22,18 +22,11 @@ import static org.mockito.Mockito.when; import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.zip.GZIPInputStream; -import java.util.zip.GZIPOutputStream; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -48,7 +41,6 @@ import software.amazon.awssdk.core.exception.RetryableException; import software.amazon.awssdk.core.exception.SdkServiceException; import software.amazon.awssdk.core.http.HttpResponseHandler; -import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute; import software.amazon.awssdk.core.protocol.VoidSdkResponse; import software.amazon.awssdk.core.runtime.transform.Marshaller; import software.amazon.awssdk.core.sync.ResponseTransformer; @@ -179,50 +171,6 @@ public void responseTransformerThrowsOtherException_shouldWrapWithNonRetryableEx .hasCauseInstanceOf(NonRetryableException.class); } - @Test - public void execute_whenConcatenatedGzipHasTransientZeroAvailable_decodesAllMembers() throws Exception { - mockSuccessfulStreamingCall(concatenatedGzip("member-0", "member-1", "member-2", "member-3", "member-4")); - when(responseTransformer.needsConnectionLeftOpen()).thenReturn(true); // customer-held stream, e.g. toInputStream() - when(responseTransformer.transform(any(SdkResponse.class), any(AbortableInputStream.class))) - .thenAnswer(invocation -> readAllGzip(invocation.getArgument(1))); - - Object decoded = syncClientHandler.execute(clientExecutionParams(), responseTransformer); - - assertThat(decoded).isEqualTo("member-0member-1member-2member-3member-4"); - } - - @Test - public void execute_whenConcatenatedGzipSupportDisabled_doesNotCoerceAvailable() throws Exception { - mockSuccessfulStreamingCall(concatenatedGzip("member-0", "member-1")); - AtomicInteger availableAfterHeader = new AtomicInteger(-1); - when(responseTransformer.transform(any(SdkResponse.class), any(AbortableInputStream.class))) - .thenAnswer(invocation -> { - AbortableInputStream stream = invocation.getArgument(1); - stream.read(); - stream.read(); - stream.read(); - availableAfterHeader.set(stream.available()); - return null; - }); - ClientExecutionParams params = clientExecutionParams() - .putExecutionAttribute(SdkInternalExecutionAttribute.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED, false); - - syncClientHandler.execute(params, responseTransformer); - - assertThat(availableAfterHeader.get()).isZero(); - } - - @Test - public void execute_whenCustomTransformerDoesNotLeaveConnectionOpen_decodesAllGzipMembers() throws Exception { - mockSuccessfulStreamingCall(concatenatedGzip("member-0", "member-1", "member-2")); - ResponseTransformer customTransformer = - (response, inputStream) -> readAllGzip(inputStream); - - String decoded = syncClientHandler.execute(clientExecutionParams(), customTransformer); - - assertThat(decoded).isEqualTo("member-0member-1member-2"); - } - private void verifyResponseTransformerPropagateException(Exception exception) throws Exception { mockSuccessfulApiCall(); when(responseTransformer.transform(any(SdkResponse.class), any(AbortableInputStream.class))).thenThrow( @@ -246,68 +194,6 @@ private void expectRetrievalFromMocks() { when(httpClient.prepareRequest(any())).thenReturn(httpClientCall); } - private void mockSuccessfulStreamingCall(byte[] body) throws Exception { - expectRetrievalFromMocks(); - when(httpClientCall.call()).thenReturn(HttpExecuteResponse.builder() - .responseBody(AbortableInputStream.create(zeroAvailableDripStream(body))) - .response(SdkHttpResponse.builder().statusCode(200).build()) - .build()); - when(responseHandler.handle(any(), any())).thenReturn(VoidSdkResponse.builder().build()); - } - - /** Serves one byte per read and always reports {@code available()==0}, mimicking a socket with no bytes buffered. */ - private static InputStream zeroAvailableDripStream(byte[] data) { - return new InputStream() { - private int pos; - - @Override - public int read() { - return pos < data.length ? data[pos++] & 0xff : -1; - } - - @Override - public int read(byte[] b, int off, int len) { - if (len == 0) { - return 0; - } - if (pos >= data.length) { - return -1; - } - b[off] = (byte) (data[pos++] & 0xff); - return 1; - } - - @Override - public int available() { - return 0; - } - }; - } - - private static byte[] concatenatedGzip(String... members) throws IOException { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - for (String member : members) { - ByteArrayOutputStream one = new ByteArrayOutputStream(); - try (GZIPOutputStream gz = new GZIPOutputStream(one)) { - gz.write(member.getBytes(StandardCharsets.UTF_8)); - } - out.write(one.toByteArray()); - } - return out.toByteArray(); - } - - private static String readAllGzip(InputStream in) throws IOException { - try (GZIPInputStream gz = new GZIPInputStream(in)) { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - byte[] buf = new byte[64]; - int n; - while ((n = gz.read(buf)) != -1) { - out.write(buf, 0, n); - } - return new String(out.toByteArray(), StandardCharsets.UTF_8); - } - } - private ClientExecutionParams clientExecutionParams() { return new ClientExecutionParams() .withInput(request) diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java index d2a79191d480..13bd302c2907 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/InputStreamResponseTransformerTest.java @@ -23,15 +23,19 @@ import java.io.InputStream; import java.nio.ByteBuffer; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Phaser; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.reactivestreams.Subscription; import software.amazon.awssdk.core.ResponseInputStream; import software.amazon.awssdk.core.SdkResponse; +import software.amazon.awssdk.core.SplittingTransformerConfiguration; +import software.amazon.awssdk.core.async.AsyncRequestBody; import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.core.async.SdkPublisher; import software.amazon.awssdk.core.protocol.VoidSdkResponse; @@ -145,46 +149,117 @@ public void cancel() { } @Test - void onStream_whenGzipDetected_coercesAvailableAboveZeroWhileOpen() throws IOException { - ResponseInputStream stream = resultFuture.join(); + void onStream_whenCompatibilityDisabled_preservesZeroAvailable() throws IOException { + assertThat(availableAfterHeader(AsyncResponseTransformer.toBlockingInputStream(), gzipHeader())).isZero(); + } + + @Test + void onStream_whenCompatibilityEnabled_coercesAvailableAboveZeroWhileOpen() throws IOException { + assertThat(availableAfterHeader(AsyncResponseTransformer.toBlockingInputStream(true), gzipHeader())) + .isGreaterThanOrEqualTo(1); + } + + @Test + void onStream_whenCompatibilityEnabledAndContentIsNotGzip_preservesZeroAvailable() throws IOException { + assertThat(availableAfterHeader(AsyncResponseTransformer.toBlockingInputStream(true), new byte[] {1, 2, 3})) + .isZero(); + } - publisher.send(ByteBuffer.wrap(new byte[] {0x1f, (byte) 0x8b, 0x08})); // gzip magic + @Test + void onStream_whenCompatibilityEnabled_preservesAbort() { + AtomicBoolean cancelled = new AtomicBoolean(); + AsyncResponseTransformer> responseTransformer = + AsyncResponseTransformer.toBlockingInputStream(true); + CompletableFuture> future = responseTransformer.prepare(); + responseTransformer.onResponse(VoidSdkResponse.builder().build()); + responseTransformer.onStream(SdkPublisher.adapt(subscriber -> subscriber.onSubscribe(new Subscription() { + @Override + public void request(long n) { + } + + @Override + public void cancel() { + cancelled.set(true); + } + }))); + + future.join().abort(); + + assertThat(cancelled).isTrue(); + } + + @Test + void split_whenCompatibilityEnabled_preservesCompatibilityOnCombinedStream() throws Exception { + AsyncResponseTransformer> responseTransformer = + AsyncResponseTransformer.toBlockingInputStream(true); + AsyncResponseTransformer.SplitResult> splitResult = + responseTransformer.split(SplittingTransformerConfiguration.builder().bufferSizeInBytes(16L).build()); + AtomicBoolean failed = new AtomicBoolean(); + CountDownLatch partCompleted = new CountDownLatch(1); + + splitResult.publisher().subscribe(new org.reactivestreams.Subscriber< + AsyncResponseTransformer>() { + private Subscription subscription; + + @Override + public void onSubscribe(Subscription subscription) { + this.subscription = subscription; + subscription.request(1); + } + + @Override + public void onNext(AsyncResponseTransformer partTransformer) { + partTransformer.prepare().whenComplete((ignored, error) -> { + failed.set(error != null); + partCompleted.countDown(); + }); + partTransformer.onResponse(VoidSdkResponse.builder().build()); + partTransformer.onStream(AsyncRequestBody.fromBytes(gzipHeader())); + subscription.cancel(); + } + + @Override + public void onError(Throwable throwable) { + failed.set(true); + partCompleted.countDown(); + } + + @Override + public void onComplete() { + } + }); + + ResponseInputStream stream = splitResult.resultFuture().join(); + assertThat(partCompleted.await(5, TimeUnit.SECONDS)).isTrue(); + assertThat(failed).isFalse(); stream.read(); stream.read(); stream.read(); - // Nothing buffered now, so the raw stream would report 0; the gzip-aware wrapper coerces it to >= 1 so a - // wrapping GZIPInputStream does not truncate concatenated (multi-member) gzip at a member boundary. assertThat(stream.available()).isGreaterThanOrEqualTo(1); + stream.close(); } - @Test - void withConcatenatedGzipStreamSupportEnabled_whenDisabled_returnsImmutableCopyWithoutAvailableCoercion() - throws IOException { - InputStreamResponseTransformer original = new InputStreamResponseTransformer<>(); - AsyncResponseTransformer> disabled = - original.withConcatenatedGzipStreamSupportEnabled(false); - - assertThat(original.withConcatenatedGzipStreamSupportEnabled(true)).isSameAs(original); - assertThat(disabled).isNotSameAs(original); - assertThat(availableAfterGzipHeader(original)).isGreaterThanOrEqualTo(1); - assertThat(availableAfterGzipHeader(disabled)).isZero(); - } - - private static int availableAfterGzipHeader( - AsyncResponseTransformer> responseTransformer) throws IOException { + private static int availableAfterHeader( + AsyncResponseTransformer> responseTransformer, + byte[] header) throws IOException { SimplePublisher body = new SimplePublisher<>(); CompletableFuture> future = responseTransformer.prepare(); responseTransformer.onResponse(VoidSdkResponse.builder().build()); responseTransformer.onStream(SdkPublisher.adapt(body)); ResponseInputStream stream = future.join(); - body.send(ByteBuffer.wrap(new byte[] {0x1f, (byte) 0x8b, 0x08})); - stream.read(); - stream.read(); - stream.read(); - int result = stream.available(); + body.send(ByteBuffer.wrap(header)); + for (int i = 0; i < header.length; i++) { + stream.read(); + } + int available = stream.available(); body.complete(); stream.close(); - return result; + return available; } + + private static byte[] gzipHeader() { + return new byte[] {0x1f, (byte) 0x8b, 0x08}; + } + } diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/sync/ResponseTransformerToInputStreamTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/sync/ResponseTransformerToInputStreamTest.java new file mode 100644 index 000000000000..0db54e66fad0 --- /dev/null +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/sync/ResponseTransformerToInputStreamTest.java @@ -0,0 +1,146 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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. + */ + +package software.amazon.awssdk.core.sync; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.core.ResponseInputStream; +import software.amazon.awssdk.http.AbortableInputStream; + +class ResponseTransformerToInputStreamTest { + + @Test + void toInputStream_whenCompatibilityNotEnabled_preservesZeroAvailable() throws Exception { + assertThat(availableAfterHeader(ResponseTransformer.toInputStream(), gzipHeader())).isZero(); + } + + @Test + void toInputStreamWithTimeout_whenCompatibilityNotEnabled_preservesZeroAvailable() throws Exception { + assertThat(availableAfterHeader(ResponseTransformer.toInputStream(Duration.ZERO), gzipHeader())).isZero(); + } + + @Test + void toInputStream_whenCompatibilityEnabled_coercesAvailableForGzip() throws Exception { + assertThat(availableAfterHeader(ResponseTransformer.toInputStream(true), gzipHeader())).isEqualTo(1); + } + + @Test + void toInputStreamWithTimeout_whenCompatibilityEnabled_coercesAvailableForGzip() throws Exception { + assertThat(availableAfterHeader(ResponseTransformer.toInputStream(Duration.ZERO, true), gzipHeader())).isEqualTo(1); + } + + @Test + void toInputStream_whenCompatibilityEnabled_preservesZeroAvailableForNonGzip() throws Exception { + assertThat(availableAfterHeader(ResponseTransformer.toInputStream(true), new byte[] {1, 2, 3})).isZero(); + } + + @Test + void toInputStream_whenCompatibilityEnabled_preservesAbort() throws Exception { + AtomicBoolean aborted = new AtomicBoolean(); + AbortableInputStream content = AbortableInputStream.create(zeroAvailableStream(gzipHeader()), + () -> aborted.set(true)); + ResponseInputStream result = ResponseTransformer.toInputStream(true).transform("response", content); + + result.abort(); + + assertThat(aborted).isTrue(); + } + + @Test + void toInputStream_whenCompatibilityEnabled_decodesAllConcatenatedGzipMembers() throws Exception { + byte[] content = concatenatedGzip("member-1", "member-2", "member-3"); + AbortableInputStream body = AbortableInputStream.create(zeroAvailableStream(content)); + ResponseInputStream result = ResponseTransformer.toInputStream(true).transform("response", body); + + assertThat(readAllGzip(result)).isEqualTo("member-1member-2member-3"); + } + + private static int availableAfterHeader( + ResponseTransformer> transformer, byte[] header) throws Exception { + AbortableInputStream content = AbortableInputStream.create(zeroAvailableStream(header)); + try (ResponseInputStream result = transformer.transform("response", content)) { + for (int i = 0; i < header.length; i++) { + assertThat(result.read()).isNotEqualTo(-1); + } + return result.available(); + } + } + + private static byte[] gzipHeader() { + return new byte[] {0x1f, (byte) 0x8b, 0x08}; + } + + private static byte[] concatenatedGzip(String... members) throws IOException { + ByteArrayOutputStream output = new ByteArrayOutputStream(); + for (String member : members) { + ByteArrayOutputStream compressedMember = new ByteArrayOutputStream(); + try (GZIPOutputStream gzip = new GZIPOutputStream(compressedMember)) { + gzip.write(member.getBytes(StandardCharsets.UTF_8)); + } + output.write(compressedMember.toByteArray()); + } + return output.toByteArray(); + } + + private static String readAllGzip(InputStream inputStream) throws IOException { + try (GZIPInputStream gzip = new GZIPInputStream(inputStream); + ByteArrayOutputStream output = new ByteArrayOutputStream()) { + byte[] buffer = new byte[64]; + int read; + while ((read = gzip.read(buffer)) != -1) { + output.write(buffer, 0, read); + } + return new String(output.toByteArray(), StandardCharsets.UTF_8); + } + } + + private static InputStream zeroAvailableStream(byte[] data) { + return new InputStream() { + private int position; + + @Override + public int read() { + return position < data.length ? data[position++] & 0xff : -1; + } + + @Override + public int read(byte[] b, int off, int len) { + if (len == 0) { + return 0; + } + if (position >= data.length) { + return -1; + } + b[off] = data[position++]; + return 1; + } + + @Override + public int available() throws IOException { + return 0; + } + }; + } +} diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/ConcatenatedGzipStreamSupportS3AsyncClient.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/ConcatenatedGzipStreamSupportS3AsyncClient.java deleted file mode 100644 index 8343ddfc0c93..000000000000 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/ConcatenatedGzipStreamSupportS3AsyncClient.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://aws.amazon.com/apache2.0 - * - * or in the "license" file accompanying this file. This file 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. - */ - -package software.amazon.awssdk.services.s3.internal.client; - -import java.util.concurrent.CompletableFuture; -import software.amazon.awssdk.annotations.SdkInternalApi; -import software.amazon.awssdk.core.async.AsyncResponseTransformer; -import software.amazon.awssdk.core.internal.async.ConfigurableAsyncResponseTransformer; -import software.amazon.awssdk.services.s3.DelegatingS3AsyncClient; -import software.amazon.awssdk.services.s3.S3AsyncClient; -import software.amazon.awssdk.services.s3.model.GetObjectRequest; -import software.amazon.awssdk.services.s3.model.GetObjectResponse; -import software.amazon.awssdk.services.s3.presignedurl.AsyncPresignedUrlExtension; -import software.amazon.awssdk.services.s3.presignedurl.model.PresignedUrlDownloadRequest; - -/** - * Configures the caller's transformer before an S3 decorator can capture it by calling {@code split()}. - */ -@SdkInternalApi -final class ConcatenatedGzipStreamSupportS3AsyncClient extends DelegatingS3AsyncClient { - private final boolean enabled; - - ConcatenatedGzipStreamSupportS3AsyncClient(S3AsyncClient delegate, boolean enabled) { - super(delegate); - this.enabled = enabled; - } - - @Override - public CompletableFuture getObject( - GetObjectRequest request, AsyncResponseTransformer transformer) { - return super.getObject(request, ConfigurableAsyncResponseTransformer.configure(transformer, enabled)); - } - - @Override - public AsyncPresignedUrlExtension presignedUrlExtension() { - AsyncPresignedUrlExtension delegateExtension = super.presignedUrlExtension(); - return new AsyncPresignedUrlExtension() { - @Override - public CompletableFuture getObject( - PresignedUrlDownloadRequest request, - AsyncResponseTransformer transformer) { - return delegateExtension.getObject( - request, ConfigurableAsyncResponseTransformer.configure(transformer, enabled)); - } - }; - } -} diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java index 8e5316c494d5..90f07d42eace 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java @@ -20,7 +20,6 @@ import java.util.function.Predicate; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.checksums.RequestChecksumCalculation; -import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.config.SdkClientOption; import software.amazon.awssdk.services.s3.S3AsyncClient; @@ -57,14 +56,6 @@ public S3AsyncClient decorate(S3AsyncClient base, == RequestChecksumCalculation.WHEN_SUPPORTED; return MultipartS3AsyncClient.create(client, multipartConfiguration, checksumEnabled); })); - - boolean concatenatedGzipStreamSupportEnabled = !Boolean.FALSE.equals( - clientConfiguration.option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED)); - decorators.add(ConditionalDecorator.create( - client -> !concatenatedGzipStreamSupportEnabled, - client -> new ConcatenatedGzipStreamSupportS3AsyncClient( - client, concatenatedGzipStreamSupportEnabled))); - return ConditionalDecorator.decorate(base, decorators); } diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/functionaltests/GetObjectConcatenatedGzipTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/functionaltests/GetObjectConcatenatedGzipTest.java index 28009a1aaded..08dfbccf1648 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/functionaltests/GetObjectConcatenatedGzipTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/functionaltests/GetObjectConcatenatedGzipTest.java @@ -44,13 +44,13 @@ /** * End-to-end (through a real client over WireMock) verification that a concatenated (multi-member) gzip response body * with NO {@code Content-Encoding: gzip} header — i.e. the caller decodes it themselves — round-trips through both the - * sync {@code toInputStream()} and async {@code toBlockingInputStream()} paths and decodes to ALL members. + * sync {@code toInputStream(true)} and async {@code toBlockingInputStream(true)} paths and decodes to ALL members. * *

This is a happy-path regression guard: it proves the {@code GzipAvailabilityInputStream} wrap the SDK now applies * does not corrupt, stall, or truncate a streaming gzip download over the real HTTP stack. It does NOT reproduce the * underlying truncation — WireMock delivers the whole body into the socket buffer, so {@code available()} is never * transiently {@code 0} at a member boundary. Deterministic reproduction of the boundary condition lives in the - * lower-level stream and handler tests. + * lower-level stream and transformer tests. */ @WireMockTest public class GetObjectConcatenatedGzipTest { @@ -67,7 +67,7 @@ public void syncGetObject_whenBodyContainsConcatenatedGzip_decodesAllMembers(Wir try (S3Client s3 = syncClient(wm)) { ResponseInputStream body = - s3.getObject(r -> r.bucket(BUCKET).key(KEY), ResponseTransformer.toInputStream()); + s3.getObject(r -> r.bucket(BUCKET).key(KEY), ResponseTransformer.toInputStream(true)); assertThat(gunzip(body)).isEqualTo(EXPECTED); } } @@ -78,7 +78,7 @@ public void asyncGetObject_whenBodyContainsConcatenatedGzip_decodesAllMembers(Wi try (S3AsyncClient s3Async = asyncClient(wm)) { ResponseInputStream body = - s3Async.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBlockingInputStream()).join(); + s3Async.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBlockingInputStream(true)).join(); assertThat(gunzip(body)).isEqualTo(EXPECTED); } } diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecoratorGzipSupportTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecoratorGzipSupportTest.java deleted file mode 100644 index 2e13c859ab04..000000000000 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecoratorGzipSupportTest.java +++ /dev/null @@ -1,191 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://aws.amazon.com/apache2.0 - * - * or in the "license" file accompanying this file. This file 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. - */ - -package software.amazon.awssdk.services.s3.internal.client; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import java.net.MalformedURLException; -import java.net.URL; -import java.util.concurrent.CompletableFuture; -import org.junit.jupiter.api.Test; -import software.amazon.awssdk.core.SplittingTransformerConfiguration; -import software.amazon.awssdk.core.async.AsyncResponseTransformer; -import software.amazon.awssdk.core.checksums.RequestChecksumCalculation; -import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; -import software.amazon.awssdk.core.client.config.SdkClientConfiguration; -import software.amazon.awssdk.core.client.config.SdkClientOption; -import software.amazon.awssdk.core.internal.async.ConfigurableAsyncResponseTransformer; -import software.amazon.awssdk.services.s3.S3AsyncClient; -import software.amazon.awssdk.services.s3.endpoints.S3ClientContextParams; -import software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionAsyncClient; -import software.amazon.awssdk.services.s3.internal.multipart.MultipartS3AsyncClient; -import software.amazon.awssdk.services.s3.model.GetObjectRequest; -import software.amazon.awssdk.services.s3.model.GetObjectResponse; -import software.amazon.awssdk.services.s3.multipart.MultipartConfiguration; -import software.amazon.awssdk.services.s3.presignedurl.AsyncPresignedUrlExtension; -import software.amazon.awssdk.services.s3.presignedurl.model.PresignedUrlDownloadRequest; -import software.amazon.awssdk.utils.AttributeMap; - -class S3AsyncClientDecoratorGzipSupportTest { - - @Test - void decorate_whenSupportDefaultOrExplicitlyEnabled_addsNoGzipDecorator() { - S3AsyncClient base = mock(S3AsyncClient.class); - - assertThat(decorate(base, false, null, null)).isSameAs(base); - assertThat(decorate(base, false, null, true)).isSameAs(base); - } - - @Test - void decorate_whenSupportDisabled_addsGzipDecorator() { - S3AsyncClient decorated = decorate(mock(S3AsyncClient.class), false, null, false); - - assertThat(decorated).isInstanceOf(ConcatenatedGzipStreamSupportS3AsyncClient.class); - } - - @Test - void decorate_whenSupportDisabledAndMultipartEnabled_addsGzipDecoratorOutermost() { - S3AsyncClient decorated = decorate(mock(S3AsyncClient.class), true, null, false); - - assertThat(decorated).isInstanceOf(ConcatenatedGzipStreamSupportS3AsyncClient.class); - assertThat(((ConcatenatedGzipStreamSupportS3AsyncClient) decorated).delegate()) - .isInstanceOf(MultipartS3AsyncClient.class); - } - - @Test - void decorate_whenSupportDisabledAndCrossRegionEnabled_addsGzipDecoratorOutermost() { - S3AsyncClient decorated = decorate(mock(S3AsyncClient.class), false, true, false); - - assertThat(decorated).isInstanceOf(ConcatenatedGzipStreamSupportS3AsyncClient.class); - assertThat(((ConcatenatedGzipStreamSupportS3AsyncClient) decorated).delegate()) - .isInstanceOf(S3CrossRegionAsyncClient.class); - } - - @Test - void getObject_whenSupportDisabled_configuresTransformerBeforeDelegating() { - S3AsyncClient base = mock(S3AsyncClient.class); - ConfigurableAsyncResponseTransformer original = - mock(ConfigurableAsyncResponseTransformer.class); - AsyncResponseTransformer configured = mock(AsyncResponseTransformer.class); - GetObjectRequest request = GetObjectRequest.builder().bucket("bucket").key("key").build(); - CompletableFuture expected = CompletableFuture.completedFuture("result"); - when(original.withConcatenatedGzipStreamSupportEnabled(false)).thenReturn(configured); - when(base.getObject(request, configured)).thenReturn(expected); - S3AsyncClient decorated = decorate(base, false, null, false); - - assertThat(decorated.getObject(request, original)).isSameAs(expected); - - verify(original).withConcatenatedGzipStreamSupportEnabled(false); - verify(base).getObject(request, configured); - } - - @Test - void presignedGetObject_whenSupportDisabled_configuresTransformerBeforeDelegating() throws MalformedURLException { - S3AsyncClient base = mock(S3AsyncClient.class); - AsyncPresignedUrlExtension baseExtension = mock(AsyncPresignedUrlExtension.class); - when(base.presignedUrlExtension()).thenReturn(baseExtension); - ConfigurableAsyncResponseTransformer original = - mock(ConfigurableAsyncResponseTransformer.class); - AsyncResponseTransformer configured = mock(AsyncResponseTransformer.class); - PresignedUrlDownloadRequest request = - PresignedUrlDownloadRequest.builder() - .presignedUrl(new URL("https://s3.amazonaws.com/bucket/key?signature=abc")) - .build(); - CompletableFuture expected = CompletableFuture.completedFuture("result"); - when(original.withConcatenatedGzipStreamSupportEnabled(false)).thenReturn(configured); - when(baseExtension.getObject(request, configured)).thenReturn(expected); - S3AsyncClient decorated = decorate(base, false, null, false); - - assertThat(decorated.presignedUrlExtension().getObject(request, original)).isSameAs(expected); - - verify(original).withConcatenatedGzipStreamSupportEnabled(false); - verify(baseExtension).getObject(request, configured); - } - - @Test - void getObject_whenSupportDisabledAndMultipartEnabled_configuresTransformerBeforeSplit() { - S3AsyncClient base = mock(S3AsyncClient.class); - ConfigurableAsyncResponseTransformer original = - mock(ConfigurableAsyncResponseTransformer.class); - AsyncResponseTransformer configured = mock(AsyncResponseTransformer.class); - IllegalStateException splitFailure = new IllegalStateException("configured transformer split"); - when(original.withConcatenatedGzipStreamSupportEnabled(false)).thenReturn(configured); - when(configured.split(any(SplittingTransformerConfiguration.class))).thenThrow(splitFailure); - S3AsyncClient decorated = decorate(base, true, null, false); - - assertThatThrownBy(() -> decorated.getObject( - GetObjectRequest.builder().bucket("bucket").key("key").build(), original)).isSameAs(splitFailure); - - verify(original).withConcatenatedGzipStreamSupportEnabled(false); - verify(configured).split(any(SplittingTransformerConfiguration.class)); - verify(original, never()).split(any(SplittingTransformerConfiguration.class)); - } - - @Test - void presignedGetObject_whenSupportDisabledAndMultipartEnabled_configuresTransformerBeforeSplit() - throws MalformedURLException { - S3AsyncClient base = mock(S3AsyncClient.class); - when(base.presignedUrlExtension()).thenReturn(mock(AsyncPresignedUrlExtension.class)); - ConfigurableAsyncResponseTransformer original = - mock(ConfigurableAsyncResponseTransformer.class); - AsyncResponseTransformer configured = mock(AsyncResponseTransformer.class); - IllegalStateException splitFailure = new IllegalStateException("configured transformer split"); - when(original.withConcatenatedGzipStreamSupportEnabled(false)).thenReturn(configured); - when(configured.split(any(SplittingTransformerConfiguration.class))).thenThrow(splitFailure); - S3AsyncClient decorated = decorate(base, true, null, false); - PresignedUrlDownloadRequest request = - PresignedUrlDownloadRequest.builder() - .presignedUrl(new URL("https://s3.amazonaws.com/bucket/key?signature=abc")) - .build(); - - assertThatThrownBy(() -> decorated.presignedUrlExtension().getObject(request, original)).isSameAs(splitFailure); - - verify(original).withConcatenatedGzipStreamSupportEnabled(false); - verify(configured).split(any(SplittingTransformerConfiguration.class)); - verify(original, never()).split(any(SplittingTransformerConfiguration.class)); - } - - private static S3AsyncClient decorate(S3AsyncClient base, - boolean multipartEnabled, - Boolean crossRegionEnabled, - Boolean gzipSupportEnabled) { - AttributeMap.Builder context = AttributeMap.builder(); - if (multipartEnabled) { - context.put(S3AsyncClientDecorator.MULTIPART_ENABLED_KEY, true); - context.put(S3AsyncClientDecorator.MULTIPART_CONFIGURATION_KEY, - MultipartConfiguration.builder().build()); - } - if (crossRegionEnabled != null) { - context.put(S3ClientContextParams.CROSS_REGION_ACCESS_ENABLED, crossRegionEnabled); - } - SdkClientConfiguration.Builder configuration = - SdkClientConfiguration.builder() - .option(SdkClientOption.CLIENT_CONTEXT_PARAMS, context.build()) - .option(SdkClientOption.REQUEST_CHECKSUM_CALCULATION, - RequestChecksumCalculation.WHEN_SUPPORTED); - if (gzipSupportEnabled != null) { - configuration.option(SdkAdvancedClientOption.CONCATENATED_GZIP_STREAM_SUPPORT_ENABLED, - gzipSupportEnabled); - } - return new S3AsyncClientDecorator().decorate(base, configuration.build()); - } -}