diff --git a/s3api/utils/unsigned-chunk-reader.go b/s3api/utils/unsigned-chunk-reader.go index d17bb38..d7067f1 100644 --- a/s3api/utils/unsigned-chunk-reader.go +++ b/s3api/utils/unsigned-chunk-reader.go @@ -111,6 +111,12 @@ func (ucr *UnsignedChunkReader) Read(p []byte) (int, error) { // Read and cache the payload _, err = io.ReadFull(rdr, payload) if err != nil { + // the chunk size is not 0 and if io.EOF is returned + // it means the body is incomplete + if errors.Is(err, io.EOF) { + debuglogger.Logf("unexpected EOF when reading chunk data") + return 0, s3err.GetAPIError(s3err.ErrIncompleteBody) + } debuglogger.Logf("failed to read chunk data: %v", err) return 0, err } @@ -149,14 +155,11 @@ func (ucr *UnsignedChunkReader) readAndSkip(data ...byte) error { for _, d := range data { b, err := ucr.reader.ReadByte() if err != nil { - if err == io.EOF { - return io.ErrUnexpectedEOF - } - return err + return s3err.GetAPIError(s3err.ErrIncompleteBody) } if b != d { - return errMalformedEncoding + return s3err.GetAPIError(s3err.ErrIncompleteBody) } } @@ -165,17 +168,24 @@ func (ucr *UnsignedChunkReader) readAndSkip(data ...byte) error { // Extracts the chunk size from the payload func (ucr *UnsignedChunkReader) extractChunkSize() (int64, error) { - line, err := ucr.reader.ReadString('\n') + line, err := ucr.reader.ReadString('\r') if err != nil { debuglogger.Logf("failed to parse chunk size: %v", err) - return 0, errMalformedEncoding + return 0, s3err.GetAPIError(s3err.ErrIncompleteBody) } + + err = ucr.readAndSkip('\n') + if err != nil { + debuglogger.Logf("failed to read the second byte (\\n) after chunk size") + return 0, err + } + line = strings.TrimSpace(line) chunkSize, err := strconv.ParseInt(line, 16, 64) if err != nil { debuglogger.Logf("failed to convert chunk size: %v", err) - return 0, errMalformedEncoding + return 0, s3err.GetAPIError(s3err.ErrIncompleteBody) } debuglogger.Infof("chunk size extracted: %v", chunkSize) @@ -191,11 +201,7 @@ func (ucr *UnsignedChunkReader) readTrailer() error { for { v, err := ucr.reader.ReadByte() if err != nil { - debuglogger.Logf("failed to read byte: %v", err) - if err == io.EOF { - return io.ErrUnexpectedEOF - } - return err + return s3err.GetAPIError(s3err.ErrIncompleteBody) } if v != '\r' { hasChecksum = true @@ -210,10 +216,7 @@ func (ucr *UnsignedChunkReader) readTrailer() error { err := ucr.readAndSkip('\n') if err != nil { debuglogger.Logf("failed to read chunk last byte: \\n: %v", err) - if err == io.EOF { - return io.ErrUnexpectedEOF - } - return err + return s3err.GetAPIError(s3err.ErrIncompleteBody) } break @@ -223,14 +226,11 @@ func (ucr *UnsignedChunkReader) readTrailer() error { _, err = io.ReadFull(ucr.reader, tmp[:]) if err != nil { debuglogger.Logf("failed to read chunk ending: \\n\\r\\n: %v", err) - if err == io.EOF { - return io.ErrUnexpectedEOF - } - return err + return s3err.GetAPIError(s3err.ErrIncompleteBody) } if !bytes.Equal(tmp[:], trailerDelim) { debuglogger.Logf("incorrect trailer delimiter: (expected): \\n\\r\\n, (got): %q", tmp[:]) - return errMalformedEncoding + return s3err.GetAPIError(s3err.ErrIncompleteBody) } break } diff --git a/s3err/s3err.go b/s3err/s3err.go index 2673e89..6936a48 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -88,6 +88,7 @@ const ( ErrInvalidCompleteMpPartNumber ErrInternalError ErrNonEmptyRequestBody + ErrIncompleteBody ErrInvalidCopyDest ErrInvalidCopySourceRange ErrInvalidCopySourceBucket @@ -328,6 +329,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "The request included a body. Requests of this type must not include a non-empty body.", HTTPStatusCode: http.StatusBadRequest, }, + ErrIncompleteBody: { + Code: "IncompleteBody", + Description: "The request body terminated unexpectedly", + HTTPStatusCode: http.StatusBadRequest, + }, ErrInvalidPart: { Code: "InvalidPart", Description: "One or more of the specified parts could not be found. The part may not have been uploaded, or the specified entity tag may not match the part's entity tag.", diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 5ae82e1..9e3976d 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -1100,6 +1100,7 @@ func TestUnsignedStreaminPayloadTrailer(ts *TestState) { ts.Run(UnsignedStreamingPayloadTrailer_incorrect_trailing_checksum) ts.Run(UnsignedStreamingPayloadTrailer_multiple_checksum_headers) ts.Run(UnsignedStreamingPayloadTrailer_sdk_algo_and_trailer_mismatch) + ts.Run(UnsignedStreamingPayloadTrailer_incomplete_body) ts.Run(UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme) ts.Run(UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers) ts.Run(UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer) @@ -1752,6 +1753,7 @@ func GetIntTests() IntTests { "UnsignedStreamingPayloadTrailer_incorrect_trailing_checksum": UnsignedStreamingPayloadTrailer_incorrect_trailing_checksum, "UnsignedStreamingPayloadTrailer_multiple_checksum_headers": UnsignedStreamingPayloadTrailer_multiple_checksum_headers, "UnsignedStreamingPayloadTrailer_sdk_algo_and_trailer_mismatch": UnsignedStreamingPayloadTrailer_sdk_algo_and_trailer_mismatch, + "UnsignedStreamingPayloadTrailer_incomplete_body": UnsignedStreamingPayloadTrailer_incomplete_body, "UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme": UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme, "UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers": UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers, "UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer": UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer, diff --git a/tests/integration/unsigned_streaming_payload_trailer.go b/tests/integration/unsigned_streaming_payload_trailer.go index afc27ec..39741a2 100644 --- a/tests/integration/unsigned_streaming_payload_trailer.go +++ b/tests/integration/unsigned_streaming_payload_trailer.go @@ -175,6 +175,42 @@ func UnsignedStreamingPayloadTrailer_sdk_algo_and_trailer_mismatch(s *S3Conf) er }) } +func UnsignedStreamingPayloadTrailer_incomplete_body(s *S3Conf) error { + testName := "UnsignedStreamingPayloadTrailer_incomplete_body" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + object := "my-object" + for i, body := range [][]byte{ + []byte("A\ndummy data\r\n0\r\n\r\n"), + []byte("A\r\ndummy data0\r\n\r\n"), + []byte("A\r\nXYZ\r\ndummy data\r\n0\r\n\r\n"), + []byte("B\r\ndummy data\r\n0\r\n\r\n"), + []byte("A\r\ndummy data\r\n0\r\n"), + []byte("A\r\ndummy data\r\n0\r\nx-amz-checksum-crc64nvme:dPVWc2vU1+Q=\r\n"), + []byte("A\r\n"), + []byte("A\r\nA\r\ndummy data\r\n0\r\n\r\n"), + // invalid chunk size + []byte("invalid_chunk_size\r\ndummy data\r\n0\r\nx-amz-checksum-crc64nvme:dPVWc2vU1+Q=\r\n\r\n"), + []byte("A\r\ndummy data\r\nJ\r\nx-amz-checksum-crc64nvme:dPVWc2vU1+Q=\r\n\r\n"), + } { + reqHeaders := map[string]string{ + "x-amz-decoded-content-length": "10", + "x-amz-trailer": "x-amz-checksum-crc64nvme", + } + + _, apiErr, err := testUnsignedStreamingPayloadTrailerObjectPut(s, bucket, object, body, reqHeaders) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + if err := compareS3ApiError(s3err.GetAPIError(s3err.ErrIncompleteBody), apiErr); err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + } + + return nil + }) +} + func UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme(s *S3Conf) error { testName := "UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {