Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions s3api/utils/csum-reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (

"github.com/aws/aws-sdk-go-v2/service/s3/types"
"github.com/cespare/xxhash/v2"
"github.com/versity/versitygw/debuglogger"
"github.com/versity/versitygw/s3err"
"github.com/zeebo/xxh3"
)
Expand Down Expand Up @@ -127,6 +128,13 @@ func NewHashReader(r io.Reader, expectedSum string, ht HashType) (*HashReader, e
// Read allows *HashReader to be used as an io.Reader
func (hr *HashReader) Read(p []byte) (int, error) {
n, readerr := hr.r.Read(p)
// Treat ErrUnexpectedEOF as EOF so a truncated body triggers checksum
// validation (which will fail on partial data) rather than leaking a
// raw Go error as an internal server error.
if readerr == io.ErrUnexpectedEOF {
debuglogger.Infof("client connection terminated early")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think Infof is meant to provide additional information in complex success paths ? This is more like an unexpected behavior from client side. Maybe it would be better to use logf and print out with [DEBUG] prefix ?

readerr = io.EOF
}
_, err := hr.hash.Write(p[:n])
if err != nil {
return n, err
Expand Down
8 changes: 8 additions & 0 deletions s3api/utils/signed-chunk-reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ func NewSignedChunkReader(r io.Reader, authdata AuthData, canonicalString, secre
// Read satisfies the io.Reader for this type
func (cr *ChunkReader) Read(p []byte) (int, error) {
n, err := cr.r.Read(p)
// Treat ErrUnexpectedEOF as EOF so a connection that closes before
// all Content-Length bytes arrive follows the normal EOF path and
// returns a proper S3 error (e.g. ErrContentLengthMismatch or
// SignatureDoesNotMatch) instead of leaking an internal Go error.
if err == io.ErrUnexpectedEOF {
debuglogger.Infof("client connection terminated early")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same here

err = io.EOF
}
if err != nil && err != io.EOF {
return 0, err
}
Expand Down
Loading