Files
versitygw/s3api/utils/content-length-reader_test.go
T
DavvyyandGitHub d03ac3c299 fix: reject a PutObject whose body ends before Content-Length
* fix(s3api): reject a PUT whose body ends before Content-Length

With a plain (non-aws-chunked) request body, a PutObject whose body ends
before Content-Length bytes have arrived is committed as a complete but
shorter object. Real S3 rejects this with IncompleteBody and the object
never becomes visible.

Nothing on that path compares bytes received against Content-Length:
fasthttp reports a connection closed mid-body as a plain io.EOF (the
conversion to io.ErrUnexpectedEOF exists only on the chunked
transfer-encoding branch), the authentication middleware leaves an
UNSIGNED-PAYLOAD body unwrapped, and io.Copy treats io.EOF as a normal
end of stream.

ErrIncompleteBody already exists and is enforced by the chunk readers.
This adds the equivalent check for plain bodies, in the controller so
that every backend is covered, and only when the body is not already an
aws-chunked reader.

POST-Object is deliberately untouched: it is a separate handler whose
ContentLength is an upper bound, so a byte-count check there would
break browser form uploads.

* test(s3api): cover ContentLengthReader

Complete and empty bodies, truncated bodies (including an EOF delivered
together with the final bytes, and one byte per Read), a body longer
than announced, and pass-through of a non-EOF error.

* fix(s3api): check plain bodies against Content-Length, not the decoded length

Review catch: the controller replaces contentLength with
X-Amz-Decoded-Content-Length whenever that header is present, regardless
of payload type, and the previous commit fed that value to
ContentLengthReader. A complete plain upload whose decoded header is
larger than Content-Length was then rejected with IncompleteBody, where
it had succeeded before.

AWS S3 ignores X-Amz-Decoded-Content-Length on a plain body and stores
Content-Length bytes. The decoded length only describes aws-chunked
payloads, and those skip this wrapper anyway, so the check now reads the
raw Content-Length header.

If the header is missing or unparseable the body is left unwrapped: an
aws-chunked request without Content-Length is already rejected earlier
with ErrMissingContentLength, so there is nothing to check here.

* test(integration): cover aborted uploads and the decoded-length case

Three tests, as requested in review.

PutObject_aborted_plain_body       plain PUT, body ends early -> no object
PutObject_aborted_streaming_body   same for an aws-chunked upload
PutObject_plain_body_with_decoded_length
                                   complete plain PUT carrying
                                   X-Amz-Decoded-Content-Length still
                                   succeeds and stores Content-Length bytes

The abort is driven by a reader that fails partway, so the transport
tears the connection down mid-body - that is what a client that dies or
cancels looks like on the wire. Sending a short body with a normal
reader would not reproduce it: net/http would simply report the
mismatch itself.

The third test is the regression the review found. It fails without the
accompanying fix.

* test(integration): replace the bogus streaming abort test with a real one

The reviewer is right: putObjectAborted always sends
x-amz-content-sha256: UNSIGNED-PAYLOAD, and the gateway decides aws-chunked
from that header alone, so PutObject_aborted_streaming_body took the same
plain path as the test above it. It duplicated the plain test instead of
covering the streaming one.

Dropped it and added UnsignedStreamingPayloadTrailer_aborted_connection in
tests/integration/unsigned_streaming_payload_trailer.go, where it belongs.

UnsignedStreamingPayloadTrailer_incomplete_body already covers malformed and
truncated framing, but every case there is a COMPLETE request. The new test is
the other shape: valid framing whose bytes simply stop arriving.
2026-09-20 11:18:15 -07:00

171 lines
4.3 KiB
Go

// Copyright 2026 Versity Software
// This file is licensed under the Apache License, Version 2.0
// (the "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License 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 utils
import (
"errors"
"io"
"strings"
"testing"
"github.com/versity/versitygw/s3err"
)
// eofWithDataReader returns its payload and reports io.EOF together with the
// final bytes, the way a connection that was closed mid-body surfaces through
// fasthttp's request stream.
type eofWithDataReader struct {
data []byte
pos int
}
func (r *eofWithDataReader) Read(p []byte) (int, error) {
n := copy(p, r.data[r.pos:])
r.pos += n
if r.pos >= len(r.data) {
return n, io.EOF
}
return n, nil
}
// oneByteReader hands out a single byte per call so the shortfall is only
// known on the very last Read.
type oneByteReader struct {
data []byte
pos int
}
func (r *oneByteReader) Read(p []byte) (int, error) {
if r.pos >= len(r.data) {
return 0, io.EOF
}
p[0] = r.data[r.pos]
r.pos++
return 1, nil
}
func TestContentLengthReader(t *testing.T) {
incomplete := s3err.GetAPIError(s3err.ErrIncompleteBody)
for _, tt := range []struct {
name string
body string
contentLength int64
newReader func(string) io.Reader
wantErr error
}{
{
name: "complete body",
body: "hello world",
contentLength: 11,
wantErr: nil,
},
{
name: "empty body",
body: "",
contentLength: 0,
wantErr: nil,
},
{
name: "truncated body",
body: "hel",
contentLength: 11,
wantErr: incomplete,
},
{
name: "truncated body, EOF with final bytes",
body: "hel",
contentLength: 11,
newReader: func(s string) io.Reader { return &eofWithDataReader{data: []byte(s)} },
wantErr: incomplete,
},
{
name: "truncated body, one byte per read",
body: "hel",
contentLength: 11,
newReader: func(s string) io.Reader { return &oneByteReader{data: []byte(s)} },
wantErr: incomplete,
},
{
name: "empty body but length announced",
body: "",
contentLength: 5,
wantErr: incomplete,
},
{
// Should not be reachable through fasthttp, but the reader must
// not invent an error when more arrives than was announced.
name: "body longer than Content-Length",
body: "hello world",
contentLength: 5,
wantErr: nil,
},
} {
t.Run(tt.name, func(t *testing.T) {
newReader := tt.newReader
if newReader == nil {
newReader = func(s string) io.Reader { return strings.NewReader(s) }
}
got, err := io.ReadAll(NewContentLengthReader(newReader(tt.body), tt.contentLength))
if string(got) != tt.body {
t.Errorf("data: got %q, want %q", string(got), tt.body)
}
if tt.wantErr == nil {
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
return
}
var apiErr s3err.APIError
if !errors.As(err, &apiErr) {
t.Fatalf("got %v, want %v", err, tt.wantErr)
}
if apiErr.Code != "IncompleteBody" {
t.Fatalf("got code %q, want IncompleteBody", apiErr.Code)
}
})
}
}
// A non-EOF error from the wrapped reader must reach the caller unchanged:
// a signed payload whose body ends early already fails with
// ContentSHA256Mismatch from the checksum reader, and that error is the one
// the client should see.
func TestContentLengthReaderPassesThroughOtherErrors(t *testing.T) {
want := s3err.GetAPIError(s3err.ErrContentSHA256Mismatch)
r := NewContentLengthReader(&failingReader{err: want}, 100)
_, err := io.ReadAll(r)
var apiErr s3err.APIError
if !errors.As(err, &apiErr) {
t.Fatalf("got %v, want %v", err, want)
}
if apiErr.Code != want.Code {
t.Fatalf("got code %q, want %q", apiErr.Code, want.Code)
}
}
type failingReader struct {
err error
}
func (r *failingReader) Read(p []byte) (int, error) {
return 0, r.err
}