Skip to content

Commit

Permalink
Allow passing custom get query params (#1905)
Browse files Browse the repository at this point in the history
Allow checking if a query param starts with "x-" which is documented by
Amazon as a parameter S3 will ignore and recommended as a way to add
custom info to server access logs.

Allow GetObject calls to set custom query parameters if they want.
  • Loading branch information
ashmrtn committed Dec 5, 2023
1 parent db8fd75 commit a9b7701
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 4 deletions.
8 changes: 4 additions & 4 deletions api-get-options.go
Expand Up @@ -87,10 +87,10 @@ func (o *GetObjectOptions) Set(key, value string) {
}

// SetReqParam - set request query string parameter
// supported key: see supportedQueryValues.
// supported key: see supportedQueryValues and allowedCustomQueryPrefix.
// If an unsupported key is passed in, it will be ignored and nothing will be done.
func (o *GetObjectOptions) SetReqParam(key, value string) {
if !isStandardQueryValue(key) {
if !isCustomQueryValue(key) && !isStandardQueryValue(key) {
// do nothing
return
}
Expand All @@ -101,10 +101,10 @@ func (o *GetObjectOptions) SetReqParam(key, value string) {
}

// AddReqParam - add request query string parameter
// supported key: see supportedQueryValues.
// supported key: see supportedQueryValues and allowedCustomQueryPrefix.
// If an unsupported key is passed in, it will be ignored and nothing will be done.
func (o *GetObjectOptions) AddReqParam(key, value string) {
if !isStandardQueryValue(key) {
if !isCustomQueryValue(key) && !isStandardQueryValue(key) {
// do nothing
return
}
Expand Down
42 changes: 42 additions & 0 deletions get-options_test.go
Expand Up @@ -57,3 +57,45 @@ func TestSetHeader(t *testing.T) {
}
}
}

func TestCustomQueryParameters(t *testing.T) {
var (
paramKey = "x-test-param"
paramValue = "test-value"

invalidParamKey = "invalid-test-param"
invalidParamValue = "invalid-test-param"
)

testCases := []struct {
setParamsFunc func(o *GetObjectOptions)
}{
{func(o *GetObjectOptions) {
o.AddReqParam(paramKey, paramValue)
o.AddReqParam(invalidParamKey, invalidParamValue)
}},
{func(o *GetObjectOptions) {
o.SetReqParam(paramKey, paramValue)
o.SetReqParam(invalidParamKey, invalidParamValue)
}},
}

for i, testCase := range testCases {
opts := GetObjectOptions{}
testCase.setParamsFunc(&opts)

// This and the following checks indirectly ensure that only the expected
// valid header is added.
if len(opts.reqParams) != 1 {
t.Errorf("Test %d: Expected 1 kv-pair in query parameters, got %v", i+1, len(opts.reqParams))
}

if v, ok := opts.reqParams[paramKey]; !ok {
t.Errorf("Test %d: Expected query parameter with key %s missing", i+1, paramKey)
} else if len(v) != 1 {
t.Errorf("Test %d: Expected 1 value for query parameter with key %s, got %d values", i+1, paramKey, len(v))
} else if v[0] != paramValue {
t.Errorf("Test %d: Expected query value %s for key %s, got %s", i+1, paramValue, paramKey, v[0])
}
}
}
8 changes: 8 additions & 0 deletions utils.go
Expand Up @@ -528,6 +528,14 @@ func isStandardQueryValue(qsKey string) bool {
return supportedQueryValues[qsKey]
}

// Per documentation at https://docs.aws.amazon.com/AmazonS3/latest/userguide/LogFormat.html#LogFormatCustom, the
// set of query params starting with "x-" are ignored by S3.
const allowedCustomQueryPrefix = "x-"

func isCustomQueryValue(qsKey string) bool {
return strings.HasPrefix(qsKey, allowedCustomQueryPrefix)
}

var (
md5Pool = sync.Pool{New: func() interface{} { return md5.New() }}
sha256Pool = sync.Pool{New: func() interface{} { return sha256.New() }}
Expand Down
21 changes: 21 additions & 0 deletions utils_test.go
Expand Up @@ -408,3 +408,24 @@ func TestIsAmzHeader(t *testing.T) {
}
}
}

// Tests if query parameter starts with "x-" and will be ignored by S3.
func TestIsCustomQueryValue(t *testing.T) {
testCases := []struct {
// Input.
queryParamKey string
// Expected result.
expectedValue bool
}{
{"x-custom-key", true},
{"xcustom-key", false},
{"random-header", false},
}

for i, testCase := range testCases {
actual := isCustomQueryValue(testCase.queryParamKey)
if actual != testCase.expectedValue {
t.Errorf("Test %d: Expected to pass, but failed", i+1)
}
}
}

0 comments on commit a9b7701

Please sign in to comment.