Skip to content

Commit

Permalink
Fail in PutObject if invalid user metadata is passed
Browse files Browse the repository at this point in the history
This PR is a continuation of the work done in PR minio#886,
rather than silently skipping invalid names we simply
error out.
  • Loading branch information
harshavardhana authored and nitisht committed Jan 11, 2018
1 parent 2689d28 commit 63cc88e
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 66 deletions.
8 changes: 3 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
all: checks

checks:
@go get -u github.com/go-ini/ini/...
@go get -u github.com/mitchellh/go-homedir/...
@go get -u github.com/cheggaaa/pb/...
@go get -u github.com/sirupsen/logrus/...
@go get -u github.com/dustin/go-humanize/...
@go get -t ./...
@go vet ./...
@SERVER_ENDPOINT=play.minio.io:9000 ACCESS_KEY=Q3AM3UQ867SPQQA43P2F SECRET_KEY=zuf+tfteSlswRu7BJ86wekitnifILbZam1KYY3TG ENABLE_HTTPS=1 go test -race -v ./...
@go get github.com/dustin/go-humanize/...
@go get github.com/sirupsen/logrus/...
@SERVER_ENDPOINT=play.minio.io:9000 ACCESS_KEY=Q3AM3UQ867SPQQA43P2F SECRET_KEY=zuf+tfteSlswRu7BJ86wekitnifILbZam1KYY3TG ENABLE_HTTPS=1 go run functional_tests.go
@mkdir -p /tmp/examples && for i in $(echo examples/s3/*); do go build -o /tmp/examples/$(basename ${i:0:-3}) ${i}; done
@go get -u github.com/a8m/mark/...
Expand Down
10 changes: 7 additions & 3 deletions api-put-object.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

"github.com/minio/minio-go/pkg/encrypt"
"github.com/minio/minio-go/pkg/s3utils"
"golang.org/x/net/lex/httplex"
)

// PutObjectOptions represents options specified by user for PutObject call
Expand Down Expand Up @@ -94,9 +95,12 @@ func (opts PutObjectOptions) Header() (header http.Header) {
// validate() checks if the UserMetadata map has standard headers or client side
// encryption headers and raises an error if so.
func (opts PutObjectOptions) validate() (err error) {
for k := range opts.UserMetadata {
if isStandardHeader(k) || isCSEHeader(k) || isStorageClassHeader(k) {
return ErrInvalidArgument(k + " unsupported request parameter for user defined metadata from minio-go")
for k, v := range opts.UserMetadata {
if !httplex.ValidHeaderFieldName(k) || isStandardHeader(k) || isCSEHeader(k) || isStorageClassHeader(k) {
return ErrInvalidArgument(k + " unsupported user defined metadata name")
}
if !httplex.ValidHeaderFieldValue(v) {
return ErrInvalidArgument(v + " unsupported user defined metadata value")
}
}
return nil
Expand Down
86 changes: 31 additions & 55 deletions api-put-object_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,65 +22,41 @@ import (

func TestPutObjectOptionsValidate(t *testing.T) {
testCases := []struct {
metadata map[string]string
shouldPass bool
name, value string
shouldPass bool
}{
{map[string]string{"Content-Type": "custom/content-type"}, false},
{map[string]string{"content-type": "custom/content-type"}, false},
{map[string]string{"Content-Encoding": "gzip"}, false},
{map[string]string{"Cache-Control": "blah"}, false},
{map[string]string{"Content-Disposition": "something"}, false},
{map[string]string{"my-custom-header": "blah"}, true},
{map[string]string{"X-Amz-Iv": "blah"}, false},
{map[string]string{"X-Amz-Key": "blah"}, false},
{map[string]string{"X-Amz-Key-prefixed-header": "blah"}, false},
{map[string]string{"custom-X-Amz-Key-middle": "blah"}, true},
{map[string]string{"my-custom-header-X-Amz-Key": "blah"}, true},
{map[string]string{"X-Amz-Matdesc": "blah"}, false},
{map[string]string{"blah-X-Amz-Matdesc": "blah"}, true},
{map[string]string{"X-Amz-MatDesc-suffix": "blah"}, true},
{map[string]string{"x-amz-meta-X-Amz-Iv": "blah"}, false},
{map[string]string{"x-amz-meta-X-Amz-Key": "blah"}, false},
{map[string]string{"x-amz-meta-X-Amz-Matdesc": "blah"}, false},
// Invalid cases.
{"X-Amz-Matdesc", "blah", false},
{"x-amz-meta-X-Amz-Iv", "blah", false},
{"x-amz-meta-X-Amz-Key", "blah", false},
{"x-amz-meta-X-Amz-Matdesc", "blah", false},
{"It has spaces", "v", false},
{"It,has@illegal=characters", "v", false},
{"X-Amz-Iv", "blah", false},
{"X-Amz-Key", "blah", false},
{"X-Amz-Key-prefixed-header", "blah", false},
{"Content-Type", "custom/content-type", false},
{"content-type", "custom/content-type", false},
{"Content-Encoding", "gzip", false},
{"Cache-Control", "blah", false},
{"Content-Disposition", "something", false},

// Valid metadata names.
{"my-custom-header", "blah", true},
{"custom-X-Amz-Key-middle", "blah", true},
{"my-custom-header-X-Amz-Key", "blah", true},
{"blah-X-Amz-Matdesc", "blah", true},
{"X-Amz-MatDesc-suffix", "blah", true},
{"It-Is-Fine", "v", true},
{"Numbers-098987987-Should-Work", "v", true},
{"Crazy-!#$%&'*+-.^_`|~-Should-193832-Be-Fine", "v", true},
}
for i, testCase := range testCases {
err := PutObjectOptions{UserMetadata: testCase.metadata}.validate()

err := PutObjectOptions{UserMetadata: map[string]string{
testCase.name: testCase.value,
}}.validate()
if testCase.shouldPass && err != nil {
t.Errorf("Test %d - output did not match with reference results", i+1)
}
}
}

// test that metadata is properly converted to headers
func TestPutObjectOptionsHeaderMetadata(t *testing.T) {

// test that invalid header keys are not included
testCases := []struct {
metakey string
shouldPass bool
}{
{"It has spaces", false},
{"It:has@illegal=characters", false},
{"It-Is-Fine", true},
{"Numbers-098987987-Should-Work", true},
{"Crazy-!#$%&'*+-.^_`|~-Should-193832-Be-Fine", true},
}

// construct the meta map from test cases
meta := make(map[string]string)
for _, testCase := range testCases {
meta[testCase.metakey] = "somevalue"
}

opt := PutObjectOptions{UserMetadata: meta}
header := opt.Header()

// iterate through test cases, ensure header is correct
for _, testCase := range testCases {
_, ok := header["X-Amz-Meta-"+testCase.metakey]
if ok != testCase.shouldPass {
t.Errorf("Test case %s should be in headers %t, but was %t", testCase.metakey, testCase.shouldPass, ok)
t.Errorf("Test %d - output did not match with reference results, %s", i+1, err)
}
}
}
4 changes: 1 addition & 3 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@ install:
- go version
- go env
- go get -u github.com/golang/lint/golint
- go get -u github.com/go-ini/ini
- go get -u github.com/mitchellh/go-homedir
- go get -u github.com/remyoudompheng/go-misc/deadcode
- go get -u github.com/gordonklaus/ineffassign
- go get -u github.com/dustin/go-humanize
- go get -t ./...

# to run your custom scripts instead of automatic MSBuild
build_script:
Expand Down

0 comments on commit 63cc88e

Please sign in to comment.