Skip to content

Commit

Permalink
Consume incoming streams fully before processing
Browse files Browse the repository at this point in the history
Some http clients (like the AWS SDKs) do not cope well if we return
errors from APIs before consuming the incoming stream.
Make sure we always consume all bytes before processing the streams.

#1662
  • Loading branch information
afranken committed Apr 12, 2024
1 parent fe9341c commit 8fb575b
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ public ResponseEntity<Void> uploadPart(@PathVariable String bucketName,
@RequestHeader(value = X_AMZ_CONTENT_SHA256, required = false) String sha256Header,
@RequestHeader HttpHeaders httpHeaders,
InputStream inputStream) {

final var input = multipartService.toTempFile(inputStream);

bucketService.verifyBucketExists(bucketName);
multipartService.verifyMultipartUploadExists(uploadId);
multipartService.verifyPartNumberLimits(partNumber);
Expand All @@ -220,7 +223,7 @@ public ResponseEntity<Void> uploadPart(@PathVariable String bucketName,
key.key(),
uploadId,
partNumber,
inputStream,
input,
isV4ChunkedWithSigningEnabled(sha256Header),
encryptionHeadersFrom(httpHeaders));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,9 +597,9 @@ public ResponseEntity<Void> putObject(@PathVariable String bucketName,
StorageClass storageClass,
@RequestHeader HttpHeaders httpHeaders,
InputStream inputStream) {
var input = objectService.toTempFile(inputStream);
bucketService.verifyBucketExists(bucketName);

var stream = objectService.verifyMd5(inputStream, contentMd5, sha256Header);
var stream = objectService.verifyMd5(input, contentMd5, sha256Header);
//TODO: need to extract owner from headers
var owner = Owner.DEFAULT_OWNER;
var s3ObjectMetadata =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.adobe.testing.s3mock.service;

import static com.adobe.testing.s3mock.S3Exception.BAD_REQUEST_CONTENT;
import static com.adobe.testing.s3mock.S3Exception.ENTITY_TOO_SMALL;
import static com.adobe.testing.s3mock.S3Exception.INVALID_PART;
import static com.adobe.testing.s3mock.S3Exception.INVALID_PART_NUMBER;
Expand All @@ -35,7 +36,9 @@
import com.adobe.testing.s3mock.dto.StorageClass;
import com.adobe.testing.s3mock.store.BucketStore;
import com.adobe.testing.s3mock.store.MultipartStore;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.util.Collections;
import java.util.Date;
import java.util.List;
Expand Down Expand Up @@ -334,4 +337,14 @@ public void verifyMultipartUploadExists(String uploadId) throws S3Exception {
throw NO_SUCH_UPLOAD_MULTIPART;
}
}

public InputStream toTempFile(InputStream inputStream) {
try {
var tempFile = Files.createTempFile("tempPart", "");
inputStream.transferTo(Files.newOutputStream(tempFile));
return Files.newInputStream(tempFile);
} catch (IOException e) {
throw BAD_REQUEST_CONTENT;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -256,17 +257,24 @@ public void verifyRetention(Retention retention) {
}
}

public InputStream verifyMd5(InputStream inputStream, String contentMd5,
String sha256Header) {
public Path toTempFile(InputStream inputStream) {
try {
var tempFile = Files.createTempFile("md5Check", "");
var tempFile = Files.createTempFile("tempObject", "");
inputStream.transferTo(Files.newOutputStream(tempFile));
return tempFile;
} catch (IOException e) {
throw BAD_REQUEST_CONTENT;
}
}

public InputStream verifyMd5(Path input, String contentMd5,
String sha256Header) {
try {
try (var stream = isV4ChunkedWithSigningEnabled(sha256Header)
? new AwsChunkedDecodingInputStream(Files.newInputStream(tempFile))
: Files.newInputStream(tempFile)) {
? new AwsChunkedDecodingInputStream(Files.newInputStream(input))
: Files.newInputStream(input)) {
verifyMd5(stream, contentMd5);
return Files.newInputStream(tempFile);
return Files.newInputStream(input);
}
} catch (IOException e) {
throw BAD_REQUEST_CONTENT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,9 @@ void testPutObject_md5_BadRequest() throws Exception {
var testFile = new File(UPLOAD_FILE_NAME);
var base64Digest = DigestUtil.base64Digest(FileUtils.openInputStream(testFile));

when(objectService.toTempFile(any(InputStream.class))).thenReturn(testFile.toPath());
doThrow(BAD_REQUEST_MD5).when(
objectService).verifyMd5(any(InputStream.class), eq(base64Digest + 1), isNull());
objectService).verifyMd5(any(Path.class), eq(base64Digest + 1), isNull());

var key = "sampleFile.txt";
var headers = new HttpHeaders();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017-2023 Adobe.
* Copyright 2017-2024 Adobe.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -108,7 +108,7 @@ void testVerifyMd5_success() throws IOException {
var sourceFile = new File(TEST_FILE_PATH);
var path = sourceFile.toPath();
var md5 = base64Digest(Files.newInputStream(path));
var inputStream = iut.verifyMd5(Files.newInputStream(path), md5, null);
var inputStream = iut.verifyMd5(path, md5, null);
assertThat(base64Digest(inputStream)).isEqualTo(md5);
}

Expand All @@ -118,7 +118,7 @@ void testVerifyMd5_failure() {
var path = sourceFile.toPath();
var md5 = "wrong-md5";
assertThatThrownBy(() ->
iut.verifyMd5(Files.newInputStream(path), md5, null)
iut.verifyMd5(path, md5, null)
).isEqualTo(BAD_REQUEST_MD5);
}

Expand Down

0 comments on commit 8fb575b

Please sign in to comment.