From 915fc7a20fb882b974169d932e73210d5cebb0cb Mon Sep 17 00:00:00 2001 From: Tung Lam <53996158+tunglambk@users.noreply.github.com> Date: Mon, 21 Sep 2026 01:04:22 +0700 Subject: [PATCH] fix: return InvalidArgument for an empty website configuration * fix: return InvalidArgument for an empty website configuration `PutBucketWebsite` parsed an empty `` into a zero-value config, and `WebsiteConfiguration.Validate` reported the nil `IndexDocument` as `MalformedXML`. S3 reports the missing index document as an `InvalidArgument` request error naming the argument instead: InvalidArgument: A value for IndexDocument Suffix must be provided if RedirectAllRequestsTo is empty That branch now returns the error with `ArgumentName=IndexDocument` and `ArgumentValue=null`, matching the response in the report. Genuinely malformed XML, and a `RedirectAllRequestsTo` that conflicts with the other fields, still return `MalformedXML`. Fixes #2260 * test: cover an empty website configuration in the PutBucketWebsite integration suite Add a `PutBucketWebsite_empty_configuration` case that sends the empty `` payload from the report and asserts the `InvalidArgument` response, including `ArgumentName=IndexDocument` and `ArgumentValue=null`. The SDK collapses the error into a generic API error that drops the argument fields, so the request is signed by hand and the raw response checked with `checkHTTPResponseApiErr`, the same helper the other argument-field assertions use. The case fails against the previous behaviour with `expected error code to be InvalidArgument, instead got MalformedXML`. --------- Co-authored-by: Tung Lam --- s3api/controllers/bucket-put_test.go | 108 ++++++++++++++++++++++++++ s3err/invalid-argument.go | 5 ++ s3response/website.go | 3 +- s3response/website_test.go | 2 +- tests/integration/PutBucketWebsite.go | 30 +++++++ tests/integration/group-tests.go | 2 + 6 files changed, 148 insertions(+), 2 deletions(-) diff --git a/s3api/controllers/bucket-put_test.go b/s3api/controllers/bucket-put_test.go index 1cccf1ae..51f9c38f 100644 --- a/s3api/controllers/bucket-put_test.go +++ b/s3api/controllers/bucket-put_test.go @@ -1334,3 +1334,111 @@ func TestS3ApiController_PutBucketAcl(t *testing.T) { }) } } + +func TestS3ApiController_PutBucketWebsite(t *testing.T) { + validBody, err := xml.Marshal(s3response.WebsiteConfiguration{ + IndexDocument: &s3response.IndexDocument{Suffix: "index.html"}, + }) + assert.NoError(t, err) + + // The payload from https://github.com/versity/versitygw/issues/2260 + emptyBody := []byte(` +`) + + tests := []struct { + name string + input testInput + output testOutput + }{ + { + name: "verify access fails", + input: testInput{ + locals: accessDeniedLocals, + }, + output: testOutput{ + response: &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: "root", + }, + }, + err: s3err.GetAPIError(s3err.ErrAccessDenied), + }, + }, + { + name: "empty website configuration", + input: testInput{ + locals: defaultLocals, + body: emptyBody, + }, + output: testOutput{ + response: &Response{ + MetaOpts: &MetaOptions{BucketOwner: "root"}, + }, + err: s3err.GetInvalidArgumentErr(s3err.InvalidArgMissingIndexDocumentSuffix, "null"), + }, + }, + { + name: "malformed xml", + input: testInput{ + locals: defaultLocals, + body: []byte(""), + }, + output: testOutput{ + response: &Response{ + MetaOpts: &MetaOptions{BucketOwner: "root"}, + }, + err: s3err.GetAPIError(s3err.ErrMalformedXML), + }, + }, + { + name: "backend error", + input: testInput{ + locals: defaultLocals, + beErr: s3err.GetAPIError(s3err.ErrNoSuchBucket), + body: validBody, + }, + output: testOutput{ + response: &Response{ + MetaOpts: &MetaOptions{BucketOwner: "root"}, + }, + err: s3err.GetAPIError(s3err.ErrNoSuchBucket), + }, + }, + { + name: "success", + input: testInput{ + locals: defaultLocals, + body: validBody, + }, + output: testOutput{ + response: &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: "root", + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + be := &BackendMock{ + PutBucketWebsiteFunc: func(contextMoqParam context.Context, bucket string, website []byte) error { + return tt.input.beErr + }, + GetBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) { + return nil, s3err.GetAPIError(s3err.ErrAccessDenied) + }, + } + + ctrl := S3ApiController{ + be: be, + } + + testController(t, ctrl.PutBucketWebsite, tt.output.response, tt.output.err, ctxInputs{ + locals: tt.input.locals, + body: tt.input.body, + }) + }) + } +} diff --git a/s3err/invalid-argument.go b/s3err/invalid-argument.go index 1947c7e9..17a0e587 100644 --- a/s3err/invalid-argument.go +++ b/s3err/invalid-argument.go @@ -60,6 +60,7 @@ const ( InvalidArgOnlyAws4HmacSha256 InvalidArgDateHeader InvalidArgIndexDocumentSuffix + InvalidArgMissingIndexDocumentSuffix InvalidArgErrorDocumentKey ) @@ -208,6 +209,10 @@ var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{ Description: "The IndexDocument Suffix is not well formed", ArgumentName: "IndexDocument", }, + InvalidArgMissingIndexDocumentSuffix: { + Description: "A value for IndexDocument Suffix must be provided if RedirectAllRequestsTo is empty", + ArgumentName: "IndexDocument", + }, InvalidArgErrorDocumentKey: { Description: "The ErrorDocument Key is not well formed", ArgumentName: "ErrorDocument", diff --git a/s3response/website.go b/s3response/website.go index 153de9a7..da1848bd 100644 --- a/s3response/website.go +++ b/s3response/website.go @@ -91,7 +91,8 @@ func (c *WebsiteConfiguration) Validate() error { if c.IndexDocument == nil { debuglogger.Logf("website index document is missing") - return s3err.GetAPIError(s3err.ErrMalformedXML) + // S3 reports the absent suffix value as the literal "null". + return s3err.GetInvalidArgumentErr(s3err.InvalidArgMissingIndexDocumentSuffix, "null") } if c.IndexDocument.Suffix == "" { debuglogger.Logf("website index suffix is empty") diff --git a/s3response/website_test.go b/s3response/website_test.go index 1f3c7aaa..dc132cd0 100644 --- a/s3response/website_test.go +++ b/s3response/website_test.go @@ -70,7 +70,7 @@ func TestWebsiteConfiguration_Validate(t *testing.T) { name: "missing index document", config: WebsiteConfiguration{}, wantErr: true, - errCode: "MalformedXML", + errCode: "InvalidArgument", }, { name: "empty index suffix", diff --git a/tests/integration/PutBucketWebsite.go b/tests/integration/PutBucketWebsite.go index 1612866e..c256f364 100644 --- a/tests/integration/PutBucketWebsite.go +++ b/tests/integration/PutBucketWebsite.go @@ -17,7 +17,9 @@ package integration import ( "context" "fmt" + "net/http" "strings" + "time" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" @@ -60,6 +62,34 @@ func PutBucketWebsite_empty_suffix(s *S3Conf) error { }) } +func PutBucketWebsite_empty_configuration(s *S3Conf) error { + testName := "PutBucketWebsite_empty_configuration" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + // The payload from https://github.com/versity/versitygw/issues/2260 + body := []byte(` +`) + + // The SDK collapses the error into a generic API error that drops the + // ArgumentName and ArgumentValue fields, so the request is signed by + // hand and the raw error body checked. + req, err := createSignedReq(http.MethodPut, s.endpoint, + fmt.Sprintf("%v?website=", bucket), s.awsID, s.awsSecret, + "s3", s.awsRegion, "", body, time.Now(), + map[string]string{"Content-Type": "application/xml"}) + if err != nil { + return err + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return err + } + + return checkHTTPResponseApiErr(resp, + s3err.GetInvalidArgumentErr(s3err.InvalidArgMissingIndexDocumentSuffix, "null")) + }) +} + func PutBucketWebsite_suffix_with_slash(s *S3Conf) error { testName := "PutBucketWebsite_suffix_with_slash" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 78b7e4c5..8c932b7d 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -687,6 +687,7 @@ func TestDeleteBucketCors(ts *TestState) { func TestPutBucketWebsite(ts *TestState) { ts.Run(PutBucketWebsite_non_existing_bucket) ts.Run(PutBucketWebsite_empty_suffix) + ts.Run(PutBucketWebsite_empty_configuration) ts.Run(PutBucketWebsite_suffix_with_slash) ts.Run(PutBucketWebsite_invalid_redirect_protocol) ts.Run(PutBucketWebsite_redirectAll_index_error_routingRules) @@ -3235,6 +3236,7 @@ func GetIntTests() IntTests { "PutBucketCors_success": PutBucketCors_success, "PutBucketWebsite_non_existing_bucket": PutBucketWebsite_non_existing_bucket, "PutBucketWebsite_empty_suffix": PutBucketWebsite_empty_suffix, + "PutBucketWebsite_empty_configuration": PutBucketWebsite_empty_configuration, "PutBucketWebsite_suffix_with_slash": PutBucketWebsite_suffix_with_slash, "PutBucketWebsite_invalid_redirect_protocol": PutBucketWebsite_invalid_redirect_protocol, "PutBucketWebsite_redirectAll_index_error_routingRules": PutBucketWebsite_redirectAll_index_error_routingRules,