mirror of
https://github.com/versity/versitygw.git
synced 2026-09-21 07:24:29 +00:00
fix: return InvalidArgument for an empty website configuration
* fix: return InvalidArgument for an empty website configuration
`PutBucketWebsite` parsed an empty `<WebsiteConfiguration/>` 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 `<WebsiteConfiguration/>` 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 <lamphamabtung96@gmail.com>
This commit is contained in:
@@ -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(`<?xml version="1.0" encoding="UTF-8"?>
|
||||
<WebsiteConfiguration xmlns="https://s3.amazonaws.com/doc/2006-03-01/"></WebsiteConfiguration>`)
|
||||
|
||||
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("<WebsiteConfiguration><IndexDocument>"),
|
||||
},
|
||||
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,
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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(`<?xml version="1.0" encoding="UTF-8"?>
|
||||
<WebsiteConfiguration xmlns="https://s3.amazonaws.com/doc/2006-03-01/"></WebsiteConfiguration>`)
|
||||
|
||||
// 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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user