Skip to content

Commit

Permalink
refactor: improve decoderx wording for parse errors (#187)
Browse files Browse the repository at this point in the history
  • Loading branch information
@aeneasr
aeneasr authored Aug 13, 2020
1 parent e97db59 commit 77a1151
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 28 deletions.
55 changes: 33 additions & 22 deletions decoderx/http.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,28 @@ const (
)

const (
// ParseErrorIgnore will ignore any parse errors caused by strconv.Parse* and use the
// ParseErrorIgnoreConversionErrors will ignore any errors caused by strconv.Parse* and use the
// raw form field value, which is a string, when such a parse error occurs.
ParseErrorIgnore parseErrorStrategy = iota + 1
//
// If the JSON Schema defines `{"ratio": {"type": "number"}}` but `ratio=foobar` then field
// `ratio` will be handled as a string. If the destination struct is a `json.RawMessage`, then
// the output will be `{"ratio": "foobar"}`.
ParseErrorIgnoreConversionErrors parseErrorStrategy = iota + 1

// ParseErrorDefault will ignore any parse errors caused by strconv.Parse* and use the
// ParseErrorUseEmptyValueOnConversionErrors will ignore any parse errors caused by strconv.Parse* and use the
// default value of the type to be casted, e.g. float64(0), string("").
ParseErrorDefault
//
// If the JSON Schema defines `{"ratio": {"type": "number"}}` but `ratio=foobar` then field
// `ratio` will receive the default value for the primitive type (here `0.0` for `number`).
// If the destination struct is a `json.RawMessage`, then the output will be `{"ratio": 0.0}`.
ParseErrorUseEmptyValueOnConversionErrors

// ParseErrorReturn will abort and return with an error if strconv.Parse* returns
// ParseErrorReturnOnConversionErrors will abort and return with an error if strconv.Parse* returns
// an error.
ParseErrorReturn
//
// If the JSON Schema defines `{"ratio": {"type": "number"}}` but `ratio=foobar` the parser aborts
// and returns an error, here: `strconv.ParseFloat: parsing "foobar"`.
ParseErrorReturnOnConversionErrors
)

// HTTPFormDecoder configures the HTTP decoder to only accept form-data
Expand Down Expand Up @@ -87,11 +98,11 @@ func HTTPDecoderSetValidatePayloads(validate bool) HTTPDecoderOption {

// HTTPDecoderSetIgnoreParseErrorsStrategy sets a strategy for dealing with strconv.Parse* errors:
//
// - decoderx.ParseErrorIgnore will ignore any parse errors caused by strconv.Parse* and use the
// - decoderx.ParseErrorIgnoreConversionErrors will ignore any parse errors caused by strconv.Parse* and use the
// raw form field value, which is a string, when such a parse error occurs. (default)
// - decoderx.ParseErrorDefault will ignore any parse errors caused by strconv.Parse* and use the
// - decoderx.ParseErrorUseEmptyValueOnConversionErrors will ignore any parse errors caused by strconv.Parse* and use the
// default value of the type to be casted, e.g. float64(0), string("").
// - decoderx.ParseErrorReturn will abort and return with an error if strconv.Parse* returns
// - decoderx.ParseErrorReturnOnConversionErrors will abort and return with an error if strconv.Parse* returns
// an error.
func HTTPDecoderSetIgnoreParseErrorsStrategy(strategy parseErrorStrategy) HTTPDecoderOption {
return func(o *httpDecoderOptions) {
Expand Down Expand Up @@ -152,7 +163,7 @@ func newHTTPDecoderOptions(fs []HTTPDecoderOption) *httpDecoderOptions {
},
allowedHTTPMethods: []string{"POST", "PUT", "PATCH"},
maxCircularReferenceDepth: 5,
handleParseErrors: ParseErrorIgnore,
handleParseErrors: ParseErrorIgnoreConversionErrors,
}

for _, f := range fs {
Expand Down Expand Up @@ -251,11 +262,11 @@ func (t *HTTP) decodeForm(r *http.Request, destination interface{}, o *httpDecod
for k, v := range r.PostForm[key] {
if f, err := strconv.ParseFloat(v, 64); err != nil {
switch o.handleParseErrors {
case ParseErrorIgnore:
case ParseErrorIgnoreConversionErrors:
raw, err = sjson.SetBytes(raw, path.Name+"."+strconv.Itoa(k), v)
case ParseErrorDefault:
case ParseErrorUseEmptyValueOnConversionErrors:
raw, err = sjson.SetBytes(raw, path.Name+"."+strconv.Itoa(k), f)
case ParseErrorReturn:
case ParseErrorReturnOnConversionErrors:
return errors.WithStack(herodot.ErrBadRequest.WithReasonf("Expected value to be a number.").
WithDetail("parse_error", err.Error()).
WithDetail("name", key).
Expand All @@ -270,11 +281,11 @@ func (t *HTTP) decodeForm(r *http.Request, destination interface{}, o *httpDecod
for k, v := range r.PostForm[key] {
if f, err := strconv.ParseBool(v); err != nil {
switch o.handleParseErrors {
case ParseErrorIgnore:
case ParseErrorIgnoreConversionErrors:
raw, err = sjson.SetBytes(raw, path.Name+"."+strconv.Itoa(k), v)
case ParseErrorDefault:
case ParseErrorUseEmptyValueOnConversionErrors:
raw, err = sjson.SetBytes(raw, path.Name+"."+strconv.Itoa(k), f)
case ParseErrorReturn:
case ParseErrorReturnOnConversionErrors:
return errors.WithStack(herodot.ErrBadRequest.WithReasonf("Expected value to be a boolean.").
WithDetail("parse_error", err.Error()).
WithDetail("name", key).
Expand All @@ -291,11 +302,11 @@ func (t *HTTP) decodeForm(r *http.Request, destination interface{}, o *httpDecod
v := r.PostForm[key][len(r.PostForm[key])-1]
if f, err := strconv.ParseBool(v); err != nil {
switch o.handleParseErrors {
case ParseErrorIgnore:
case ParseErrorIgnoreConversionErrors:
raw, err = sjson.SetBytes(raw, path.Name, v)
case ParseErrorDefault:
case ParseErrorUseEmptyValueOnConversionErrors:
raw, err = sjson.SetBytes(raw, path.Name, f)
case ParseErrorReturn:
case ParseErrorReturnOnConversionErrors:
return errors.WithStack(herodot.ErrBadRequest.WithReasonf("Expected value to be a boolean.").
WithDetail("parse_error", err.Error()).
WithDetail("name", key).
Expand All @@ -308,11 +319,11 @@ func (t *HTTP) decodeForm(r *http.Request, destination interface{}, o *httpDecod
v := r.PostForm.Get(key)
if f, err := strconv.ParseFloat(v, 64); err != nil {
switch o.handleParseErrors {
case ParseErrorIgnore:
case ParseErrorIgnoreConversionErrors:
raw, err = sjson.SetBytes(raw, path.Name, v)
case ParseErrorDefault:
case ParseErrorUseEmptyValueOnConversionErrors:
raw, err = sjson.SetBytes(raw, path.Name, f)
case ParseErrorReturn:
case ParseErrorReturnOnConversionErrors:
return errors.WithStack(herodot.ErrBadRequest.WithReasonf("Expected value to be a number.").
WithDetail("parse_error", err.Error()).
WithDetail("name", key).
Expand Down
20 changes: 14 additions & 6 deletions decoderx/http_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,33 +143,41 @@ func TestHTTPFormDecoder(t *testing.T) {
}`,
},
{
d: "should work with ParseErrorIgnore",
d: "should work with ParseErrorIgnoreConversionErrors",
request: newRequest(t, "POST", "/", bytes.NewBufferString(url.Values{
"ratio": {"foobar"},
}.Encode()), httpContentTypeURLEncodedForm),
options: []HTTPDecoderOption{
HTTPJSONSchemaCompiler("stub/person.json", nil),
HTTPDecoderSetIgnoreParseErrorsStrategy(ParseErrorIgnore),
HTTPDecoderSetIgnoreParseErrorsStrategy(ParseErrorIgnoreConversionErrors),
HTTPDecoderSetValidatePayloads(false),
},
expected: `{"ratio": "foobar"}`,
},
{
d: "should work with ParseErrorIgnore",
d: "should work with ParseErrorIgnoreConversionErrors",
request: newRequest(t, "POST", "/", bytes.NewBufferString(url.Values{
"ratio": {"foobar"},
}.Encode()), httpContentTypeURLEncodedForm),
options: []HTTPDecoderOption{HTTPJSONSchemaCompiler("stub/person.json", nil), HTTPDecoderSetIgnoreParseErrorsStrategy(ParseErrorDefault)},
options: []HTTPDecoderOption{HTTPJSONSchemaCompiler("stub/person.json", nil), HTTPDecoderSetIgnoreParseErrorsStrategy(ParseErrorUseEmptyValueOnConversionErrors)},
expected: `{"ratio": 0.0}`,
},
{
d: "should work with ParseErrorIgnore",
d: "should work with ParseErrorIgnoreConversionErrors",
request: newRequest(t, "POST", "/", bytes.NewBufferString(url.Values{
"ratio": {"foobar"},
}.Encode()), httpContentTypeURLEncodedForm),
options: []HTTPDecoderOption{HTTPJSONSchemaCompiler("stub/person.json", nil), HTTPDecoderSetIgnoreParseErrorsStrategy(ParseErrorReturn)},
options: []HTTPDecoderOption{HTTPJSONSchemaCompiler("stub/person.json", nil), HTTPDecoderSetIgnoreParseErrorsStrategy(ParseErrorReturnOnConversionErrors)},
expectedError: `strconv.ParseFloat: parsing "foobar"`,
},
{
d: "should interpret numbers as string if mandated by the schema",
request: newRequest(t, "POST", "/", bytes.NewBufferString(url.Values{
"name.first": {"12345"},
}.Encode()), httpContentTypeURLEncodedForm),
options: []HTTPDecoderOption{HTTPJSONSchemaCompiler("stub/person.json", nil), HTTPDecoderSetIgnoreParseErrorsStrategy(ParseErrorUseEmptyValueOnConversionErrors)},
expected: `{"name": {"first": "12345"}}`,
},
} {
t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) {
dec := NewHTTP()
Expand Down

0 comments on commit 77a1151

Please sign in to comment.