From 046e233a4ed527abe49cec83d3110c407edbbf30 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Fri, 31 Mar 2023 16:28:11 -0400 Subject: [PATCH] Parse form-urlencoded, form-data body parameters Specifically, parseApiRequest now parses the bodies of POST requests with a Content-Type of either application/x-www-form-urlencoded or multipart/form-data. This is to support to primary use cases: 1. The typical /subscribe submission from a web form, which defaults to form-urlencoded, but can easily use form-data. It would require JavaScript to construct path parameters instead, or even a JSON payload. 2. The List-Unsubscribe=One-Click use case. The email address and UID are path parameters, but the List-Unsubscribe parameter must encoded in the body. See: https://www.rfc-editor.org/rfc/rfc8058 Performed slight refactoring and cleanup of parser.go and parser_test.go along the way. Of particular note is that parseApiRequest wraps all errors in ParseError, instead of having ParseErrors bubble up. --- handler/parser.go | 109 +++++++++++++----- handler/parser_test.go | 254 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 319 insertions(+), 44 deletions(-) diff --git a/handler/parser.go b/handler/parser.go index 46326a3..13c2d99 100644 --- a/handler/parser.go +++ b/handler/parser.go @@ -2,7 +2,12 @@ package handler import ( "fmt" + "io" + "mime" + "mime/multipart" + "net/http" "net/mail" + "net/url" "strings" "github.com/google/uuid" @@ -71,27 +76,22 @@ type apiRequest struct { } func parseApiRequest(req *apiRequest) (*eventOperation, error) { - if pi, err := newOpInfo(req.RawPath, req.Params); err != nil { - return nil, err - } else if email, err := pi.parseEmail(); err != nil { - return nil, err - } else if uid, err := pi.parseUid(); err != nil { - return nil, err + var info *opInfo = nil + + if optype, err := parseOperationType(req.RawPath); err != nil { + return nil, &ParseError{optype, err.Error()} + } else if err := parseParams(req); err != nil { + return nil, &ParseError{optype, err.Error()} } else { - return &eventOperation{pi.Type, email, uid}, nil + info = &opInfo{optype, req.Params} } -} - -type opInfo struct { - Type eventOperationType - Params map[string]string -} -func newOpInfo(endpoint string, params map[string]string) (*opInfo, error) { - if optype, err := parseOperationType(endpoint); err != nil { - return nil, err + if email, err := info.parseEmail(); err != nil { + return nil, &ParseError{info.Type, err.Error()} + } else if uid, err := info.parseUid(); err != nil { + return nil, &ParseError{info.Type, err.Error()} } else { - return &opInfo{optype, params}, nil + return &eventOperation{info.Type, email, uid}, nil } } @@ -103,9 +103,70 @@ func parseOperationType(endpoint string) (eventOperationType, error) { } else if strings.HasPrefix(endpoint, UnsubscribePrefix) { return UnsubscribeOp, nil } - return UndefinedOp, &ParseError{ - Type: UndefinedOp, Message: "unknown endpoint: " + endpoint, + return UndefinedOp, fmt.Errorf("unknown endpoint: %s", endpoint) +} + +func parseParams(req *apiRequest) error { + if req.Method != http.MethodPost { + return nil + } + + values, err := parseBody(req.ContentType, req.Body) + + if err != nil { + const errFmt = `failed to parse body params with Content-Type %q: %s` + return fmt.Errorf(errFmt, req.ContentType, err) + } + + for k, v := range values { + if len(v) != 1 { + values := strings.Join(v, ", ") + return fmt.Errorf("multiple values for %q: %s", k, values) + } else if _, exists := req.Params[k]; !exists { + req.Params[k] = v[0] + } + } + return nil +} + +func parseBody(contentType, body string) (url.Values, error) { + mediaType, params, err := mime.ParseMediaType(contentType) + + if err != nil { + const errFormat = "failed to parse %q: %s" + return url.Values{}, fmt.Errorf(errFormat, contentType, err) + } + + switch mediaType { + case "application/x-www-form-urlencoded": + return url.ParseQuery(body) + case "multipart/form-data": + return parseFormData(body, params) + } + return url.Values{}, fmt.Errorf("unknown media type: %s", mediaType) +} + +func parseFormData(body string, params map[string]string) (url.Values, error) { + reader := multipart.NewReader(strings.NewReader(body), params["boundary"]) + values := url.Values{} + + for { + if part, err := reader.NextPart(); err == io.EOF { + break + } else if err != nil { + return url.Values{}, err + } else if data, err := io.ReadAll(part); err != nil { + return url.Values{}, err + } else { + values.Add(part.FormName(), string(data)) + } } + return values, nil +} + +type opInfo struct { + Type eventOperationType + Params map[string]string } func (oi *opInfo) parseEmail() (string, error) { @@ -131,19 +192,15 @@ func parseParam[T string | uuid.UUID]( oi *opInfo, name string, nilValue T, parse func(string) (T, error), ) (T, error) { if value, ok := oi.Params[name]; !ok { - return nilValue, oi.parseError("missing " + name + " parameter") + return nilValue, fmt.Errorf("missing %s parameter", name) } else if v, err := parse(value); err != nil { - msg := fmt.Sprintf("invalid %s parameter: %s: %s", name, value, err) - return nilValue, oi.parseError(msg) + e := fmt.Errorf("invalid %s parameter: %s: %s", name, value, err) + return nilValue, e } else { return v, nil } } -func (oi *opInfo) parseError(message string) error { - return &ParseError{Type: oi.Type, Message: message} -} - type parsedSubject struct { Email string Uid uuid.UUID diff --git a/handler/parser_test.go b/handler/parser_test.go index 35ae32b..e06fc75 100644 --- a/handler/parser_test.go +++ b/handler/parser_test.go @@ -3,6 +3,10 @@ package handler import ( "errors" "fmt" + "mime" + "mime/multipart" + "net/http" + "net/url" "strings" "testing" @@ -22,7 +26,7 @@ func TestParseError(t *testing.T) { Message: "invalid email parameter: mbland acm.org", } - t.Run("StringIncludesOptypeMessageAndEndpoint", func(t *testing.T) { + t.Run("StringIncludesOptypeAndMessage", func(t *testing.T) { assert.Equal( t, "Subscribe: invalid email parameter: mbland acm.org", @@ -37,7 +41,7 @@ func TestParseError(t *testing.T) { t.Run("IsFalse", func(t *testing.T) { stringErr := fmt.Errorf( - "Subscribe: invalid email parameter: /subscribe/mbland acm.org", + "Subscribe: invalid email parameter: mbland acm.org", ) assert.Assert(t, !err.Is(stringErr), "string") @@ -45,9 +49,184 @@ func TestParseError(t *testing.T) { }) } +type testParams map[string]string + +func (tp testParams) urlencoded() (string, string) { + values := url.Values{} + + for k, v := range tp { + values.Add(k, v) + } + return "application/x-www-form-urlencoded", values.Encode() +} + +func (tp testParams) formData() (string, string) { + builder := strings.Builder{} + writer := multipart.NewWriter(&builder) + + for k, v := range tp { + writer.WriteField(k, v) + } + writer.Close() + return writer.FormDataContentType(), builder.String() +} + +func (tp testParams) urlValues() url.Values { + values := url.Values{} + + for k, v := range tp { + values.Add(k, v) + } + return values +} + +func TestParseFormData(t *testing.T) { + params := testParams{"email": "mbland@acm.org", "uid": "0123-456-789"} + contentType, body := params.formData() + _, mediaParams, err := mime.ParseMediaType(contentType) + + if err != nil { + t.Fatalf("Content-Type %q failed to parse: %s", contentType, err) + } + + t.Run("Success", func(t *testing.T) { + values, err := parseFormData(body, mediaParams) + + assert.NilError(t, err) + assert.DeepEqual(t, params.urlValues(), values) + }) + + t.Run("ErrorOnNextPart", func(t *testing.T) { + badBody := strings.ReplaceAll(body, mediaParams["boundary"], "") + + values, err := parseFormData(badBody, mediaParams) + + assert.ErrorContains(t, err, "multipart: NextPart: EOF") + assert.DeepEqual(t, url.Values{}, values) + }) + + t.Run("ErrorOnReadAll", func(t *testing.T) { + badBody := strings.Replace(body, mediaParams["boundary"]+"--", "", 1) + + values, err := parseFormData(badBody, mediaParams) + + assert.Error(t, err, "unexpected EOF") + assert.DeepEqual(t, url.Values{}, values) + }) +} + +func TestParseBody(t *testing.T) { + params := testParams{"email": "mbland@acm.org", "uid": "0123-456-789"} + + t.Run("ErrorIfMediaTypeFailsToParse", func(t *testing.T) { + _, body := params.urlencoded() + values, err := parseBody("foobar/", body) + + expected := `failed to parse "foobar/": ` + + "mime: expected token after slash" + assert.ErrorContains(t, err, expected) + assert.DeepEqual(t, url.Values{}, values) + }) + + t.Run("ErrorIfUnknownMediaType", func(t *testing.T) { + _, body := params.urlencoded() + values, err := parseBody("foobar", body) + + assert.ErrorContains(t, err, "unknown media type: foobar") + assert.DeepEqual(t, url.Values{}, values) + }) + + t.Run("ParseUrlencoded", func(t *testing.T) { + contentType, body := params.urlencoded() + values, err := parseBody(contentType, body) + + assert.NilError(t, err) + assert.DeepEqual(t, params.urlValues(), values) + }) + + t.Run("ParseFormData", func(t *testing.T) { + contentType, body := params.formData() + values, err := parseBody(contentType, body) + + assert.NilError(t, err) + assert.DeepEqual(t, params.urlValues(), values) + }) +} + +func TestParseParams(t *testing.T) { + newRequest := func() *apiRequest { + return &apiRequest{ + Method: http.MethodPost, + ContentType: "application/x-www-form-urlencoded", + Params: map[string]string{}, + Body: "email=mbland%40acm.org&uid=0123-456-789", + } + } + parsedParams := map[string]string{ + "email": "mbland@acm.org", "uid": "0123-456-789", + } + + t.Run("IgnoreIfNotPostRequest", func(t *testing.T) { + req := newRequest() + req.Method = http.MethodGet + + err := parseParams(req) + + assert.NilError(t, err) + assert.DeepEqual(t, map[string]string{}, req.Params) + }) + + t.Run("ParseError", func(t *testing.T) { + req := newRequest() + req.Body = "email=mbland@acm.org;uid=0123-456-789" + + err := parseParams(req) + + expected := fmt.Sprintf( + `failed to parse body params with Content-Type "%s": `, + req.ContentType, + ) + assert.ErrorContains(t, err, expected) + assert.DeepEqual(t, map[string]string{}, req.Params) + }) + + t.Run("Success", func(t *testing.T) { + req := newRequest() + + err := parseParams(req) + + assert.NilError(t, err) + assert.DeepEqual(t, parsedParams, req.Params) + }) + + t.Run("PreferIncomingParamsOverBodyParams", func(t *testing.T) { + req := newRequest() + req.Params["email"] = "foo@bar.com" + + err := parseParams(req) + + assert.NilError(t, err) + expected := map[string]string{ + "email": "foo@bar.com", "uid": parsedParams["uid"], + } + assert.DeepEqual(t, expected, req.Params) + }) + + t.Run("ErrorIfParamHasMultipleValues", func(t *testing.T) { + req := newRequest() + req.Body = "email=mbland%40acm.org&email=foo%40bar.com" + + err := parseParams(req) + + expected := `multiple values for "email": mbland@acm.org, foo@bar.com` + assert.ErrorContains(t, err, expected) + assert.DeepEqual(t, map[string]string{}, req.Params) + }) +} + func TestParseOperationType(t *testing.T) { t.Run("Subscribe", func(t *testing.T) { - result, err := parseOperationType(SubscribePrefix + "/foobar") + result, err := parseOperationType(SubscribePrefix) assert.NilError(t, err) assert.Equal(t, "Subscribe", result.String()) @@ -71,9 +250,7 @@ func TestParseOperationType(t *testing.T) { result, err := parseOperationType("/foobar/baz") assert.Equal(t, "Undefined", result.String()) - assert.DeepEqual( - t, err, &ParseError{UndefinedOp, "unknown endpoint: /foobar/baz"}, - ) + assert.ErrorContains(t, err, "unknown endpoint: /foobar/baz") }) } @@ -85,9 +262,7 @@ func TestParseEmail(t *testing.T) { result, err := pi.parseEmail() assert.Equal(t, "", result) - assert.DeepEqual( - t, err, &ParseError{VerifyOp, "missing email parameter"}, - ) + assert.ErrorContains(t, err, "missing email parameter") }) t.Run("ParamInvalid", func(t *testing.T) { @@ -95,10 +270,9 @@ func TestParseEmail(t *testing.T) { result, err := pi.parseEmail() assert.Equal(t, "", result) - assert.DeepEqual(t, err, &ParseError{ - VerifyOp, - "invalid email parameter: bazquux: mail: missing '@' or angle-addr", - }) + expected := "invalid email parameter: bazquux: " + + "mail: missing '@' or angle-addr" + assert.ErrorContains(t, err, expected) }) t.Run("ParamValid", func(t *testing.T) { @@ -132,6 +306,7 @@ func TestParseUid(t *testing.T) { } func TestParseApiEvent(t *testing.T) { + t.Run("Unknown", func(t *testing.T) { result, err := parseApiRequest(&apiRequest{ RawPath: "/foobar", Params: map[string]string{}, @@ -142,9 +317,26 @@ func TestParseApiEvent(t *testing.T) { assert.ErrorContains(t, err, "unknown endpoint: /foobar") }) + t.Run("ErrorWhileParsingParams", func(t *testing.T) { + req := &apiRequest{ + RawPath: SubscribePrefix, + Params: map[string]string{}, + Method: http.MethodPost, + ContentType: "application/x-www-form-urlencoded", + Body: "email=mbland%40acm.org&email=foo%40bar.com", + } + + result, err := parseApiRequest(req) + + assert.Assert(t, is.Nil(result)) + assert.Assert(t, errors.Is(err, &ParseError{Type: SubscribeOp})) + expected := `multiple values for "email": mbland@acm.org, foo@bar.com` + assert.ErrorContains(t, err, expected) + }) + t.Run("InvalidEmail", func(t *testing.T) { result, err := parseApiRequest(&apiRequest{ - RawPath: SubscribePrefix + "foobar", + RawPath: SubscribePrefix, Params: map[string]string{"email": "foobar"}, }) @@ -166,19 +358,45 @@ func TestParseApiEvent(t *testing.T) { assert.ErrorContains(t, err, "invalid uid parameter: 0123456789") }) - t.Run("Success", func(t *testing.T) { - uidStr := "00000000-1111-2222-3333-444444444444" - result, err := parseApiRequest(&apiRequest{ + t.Run("SuccessfulSubscribe", func(t *testing.T) { + req := &apiRequest{ + RawPath: SubscribePrefix, + Params: map[string]string{}, + Method: http.MethodPost, + ContentType: "application/x-www-form-urlencoded", + Body: "email=mbland%40acm.org", + } + + result, err := parseApiRequest(req) + + assert.NilError(t, err) + assert.DeepEqual( + t, result, &eventOperation{SubscribeOp, "mbland@acm.org", uuid.Nil}, + ) + }) + + t.Run("SuccessfulOneClickUnsubscribe", func(t *testing.T) { + // The "email" and "uid" are path parameters. "List-Unsubscribe" is + // parsed from the body. + const uidStr = "00000000-1111-2222-3333-444444444444" + + req := &apiRequest{ RawPath: UnsubscribePrefix + "/mbland@acm.org/" + uidStr, Params: map[string]string{ "email": "mbland@acm.org", "uid": uidStr, }, - }) + Method: http.MethodPost, + ContentType: "application/x-www-form-urlencoded", + Body: "List-Unsubscribe=One-Click", + } + + result, err := parseApiRequest(req) assert.NilError(t, err) assert.DeepEqual(t, result, &eventOperation{ UnsubscribeOp, "mbland@acm.org", uuid.MustParse(uidStr), }) + assert.Equal(t, "One-Click", req.Params["List-Unsubscribe"]) }) }