diff --git a/cmd/postpolicyform.go b/cmd/postpolicyform.go index 69889b4e1..6abdc7305 100644 --- a/cmd/postpolicyform.go +++ b/cmd/postpolicyform.go @@ -17,14 +17,18 @@ package cmd import ( + "bytes" "encoding/json" "errors" "fmt" + "io" "net/http" "reflect" "strconv" "strings" "time" + + "github.com/bcicen/jstream" ) // startWithConds - map which indicates if a given condition supports starts-with policy operator @@ -110,8 +114,32 @@ type PostPolicyForm struct { } } +// implemented to ensure that duplicate keys in JSON +// are merged together into a single JSON key, also +// to remove any extraneous JSON bodies. +// +// Go stdlib doesn't support parsing JSON with duplicate +// keys, so we need to use this technique to merge the +// keys. +func sanitizePolicy(policy string) (io.Reader, error) { + var buf bytes.Buffer + e := json.NewEncoder(&buf) + d := jstream.NewDecoder(strings.NewReader(policy), 0) + for mv := range d.Stream() { + e.Encode(mv.Value) + } + return &buf, d.Err() +} + // parsePostPolicyForm - Parse JSON policy string into typed PostPolicyForm structure. -func parsePostPolicyForm(policy string) (ppf PostPolicyForm, e error) { +func parsePostPolicyForm(policy string) (PostPolicyForm, error) { + preader, err := sanitizePolicy(policy) + if err != nil { + return PostPolicyForm{}, err + } + + d := json.NewDecoder(preader) + // Convert po into interfaces and // perform strict type conversion using reflection. var rawPolicy struct { @@ -119,9 +147,9 @@ func parsePostPolicyForm(policy string) (ppf PostPolicyForm, e error) { Conditions []interface{} `json:"conditions"` } - err := json.Unmarshal([]byte(policy), &rawPolicy) - if err != nil { - return ppf, err + d.DisallowUnknownFields() + if err := d.Decode(&rawPolicy); err != nil { + return PostPolicyForm{}, err } parsedPolicy := PostPolicyForm{} @@ -129,7 +157,7 @@ func parsePostPolicyForm(policy string) (ppf PostPolicyForm, e error) { // Parse expiry time. parsedPolicy.Expiration, err = time.Parse(time.RFC3339Nano, rawPolicy.Expiration) if err != nil { - return ppf, err + return PostPolicyForm{}, err } // Parse conditions. diff --git a/cmd/postpolicyform_test.go b/cmd/postpolicyform_test.go index 5ac451140..c1c299248 100644 --- a/cmd/postpolicyform_test.go +++ b/cmd/postpolicyform_test.go @@ -25,6 +25,48 @@ import ( minio "github.com/minio/minio-go/v7" ) +func TestParsePostPolicyForm(t *testing.T) { + testCases := []struct { + policy string + success bool + }{ + // missing expiration, will fail. + { + policy: `{"conditions":[["eq","$bucket","asdf"],["eq","$key","hello.txt"]],"conditions":[["eq","$success_action_status","201"],["eq","$Content-Type","plain/text"],["eq","$success_action_status","201"],["eq","$x-amz-algorithm","AWS4-HMAC-SHA256"],["eq","$x-amz-credential","Q3AM3UQ867SPQQA43P2F/20210315/us-east-1/s3/aws4_request"],["eq","$x-amz-date","20210315T091621Z"]]}`, + success: false, + }, + // invalid json. + { + policy: `{"conditions":[["eq","$bucket","asdf"],["eq","$key","hello.txt"]],"conditions":[["eq","$success_action_status","201"],["eq","$Content-Type","plain/text"],["eq","$success_action_status","201"],["eq","$x-amz-algorithm","AWS4-HMAC-SHA256"],["eq","$x-amz-credential","Q3AM3UQ867SPQQA43P2F/20210315/us-east-1/s3/aws4_request"],["eq","$x-amz-date","20210315T091621Z"]]`, + success: false, + }, + // duplicate conditions, shall be parsed properly + { + policy: `{"expiration":"2021-03-22T09:16:21.310Z","conditions":[["eq","$bucket","asdf"],["eq","$key","hello.txt"]],"conditions":[["eq","$success_action_status","201"],["eq","$Content-Type","plain/text"],["eq","$success_action_status","201"],["eq","$x-amz-algorithm","AWS4-HMAC-SHA256"],["eq","$x-amz-credential","Q3AM3UQ867SPQQA43P2F/20210315/us-east-1/s3/aws4_request"],["eq","$x-amz-date","20210315T091621Z"]]}`, + success: true, + }, + // no duplicates, shall be parsed properly. + { + policy: `{"expiration":"2021-03-27T20:35:28.458Z","conditions":[["eq","$bucket","testbucket"],["eq","$key","wtf.txt"],["eq","$x-amz-date","20210320T203528Z"],["eq","$x-amz-algorithm","AWS4-HMAC-SHA256"],["eq","$x-amz-credential","Q3AM3UQ867SPQQA43P2F/20210320/us-east-1/s3/aws4_request"]]}`, + success: true, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run("", func(t *testing.T) { + _, err := parsePostPolicyForm(testCase.policy) + fmt.Println(err) + if testCase.success && err != nil { + t.Errorf("Expected success but failed with %s", err) + } + if !testCase.success && err == nil { + t.Errorf("Expected failed but succeeded") + } + }) + } +} + // Test Post Policy parsing and checking conditions func TestPostPolicyForm(t *testing.T) { pp := minio.NewPostPolicy() diff --git a/go.mod b/go.mod index 8d313eb7e..3490df1d2 100644 --- a/go.mod +++ b/go.mod @@ -74,7 +74,7 @@ require ( github.com/shirou/gopsutil/v3 v3.21.1 github.com/spaolacci/murmur3 v1.1.0 // indirect github.com/streadway/amqp v1.0.0 - github.com/tidwall/gjson v1.6.7 + github.com/tidwall/gjson v1.6.8 github.com/tidwall/sjson v1.0.4 github.com/tinylib/msgp v1.1.3 github.com/valyala/tcplisten v0.0.0-20161114210144-ceec8f93295a diff --git a/go.sum b/go.sum index 7edc8448c..f7a4d9661 100644 --- a/go.sum +++ b/go.sum @@ -584,8 +584,8 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/tidwall/gjson v1.6.7 h1:Mb1M9HZCRWEcXQ8ieJo7auYyyiSux6w9XN3AdTpxJrE= -github.com/tidwall/gjson v1.6.7/go.mod h1:zeFuBCIqD4sN/gmqBzZ4j7Jd6UcA2Fc56x7QFsv+8fI= +github.com/tidwall/gjson v1.6.8 h1:CTmXMClGYPAmln7652e69B7OLXfTi5ABcPPwjIWUv7w= +github.com/tidwall/gjson v1.6.8/go.mod h1:zeFuBCIqD4sN/gmqBzZ4j7Jd6UcA2Fc56x7QFsv+8fI= github.com/tidwall/match v1.0.3 h1:FQUVvBImDutD8wJLN6c5eMzWtjgONK9MwIBCOrUJKeE= github.com/tidwall/match v1.0.3/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= github.com/tidwall/pretty v1.0.2 h1:Z7S3cePv9Jwm1KwS0513MRaoUe3S01WPbLNV40pwWZU=