Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When c.bind() fails with a type error on POST'd form data the inputfieldname is not available #2629

Open
3 tasks done
oschonrock opened this issue Apr 24, 2024 · 3 comments
Open
3 tasks done

Comments

@oschonrock
Copy link

Issue Description

When using echo for a web application sending HTML and receiving MIMEApplicationForm POST requests, it seems very difficult / impossible to show helpful validation error messages to the user, when using echo.context.bind().

For any string data coming from the MIMEApplicationForm submit which cannot be converted to the target type in the "DTO" struct provided, echo.context.bind() returns a generic error that gives no information about which struct/form field failed validation.

This makes it very difficult or impossible to render a helpful message to the user like "The number field should contain an integer". when the failed form is re-rendered in the browser.

This issue is also discussed here:
https://stackoverflow.com/a/77712569/1087626

The issue apparently does not exist when dealing with JSON data rather than MIMEApplicationForm

Checklist

  • Dependencies installed
  • No typos
  • Searched existing issues and docs

Expected behaviour

For the error returned from context.bind() to return some information about which field failed type conversion

Actual behaviour

The error returned from context.bind() contains no information about the form field which failed type conversion.

Steps to reproduce

server as below, then POST a non-integer to be bound into an integer field

$ curl localhost:8080/submit -d"number=10a"
code=400, message=strconv.ParseInt: parsing "10a": invalid syntax, internal=strconv.ParseInt: parsing "10a": invalid syntax

Working code to debug

package main

import (
	"fmt"
	"net/http"

	"github.com/labstack/echo/v4"
)

type DTO struct {
	Number int     `form:"number"`
}

func main() {
	e := echo.New()
	e.POST("/submit", func(c echo.Context) error {
		dto := DTO{}
		if err := c.Bind(&dto); err != nil {
			return c.String(http.StatusBadRequest, err.Error() + "\n")
		}
		return c.String(http.StatusOK, fmt.Sprint(dto) + "\n")
	})
	addr := "127.0.0.1:8080"
	e.Logger.Fatal(e.Start(addr))
}

The following diff shows that the desired "fieldname" information is available in the relevant part of bind.go and could return that info with the error:

--- /home/oliver/bind.go	2024-04-24 17:01:48.429244184 +0100
+++ /home/oliver/go/pkg/mod/github.com/labstack/echo/[email protected]/bind.go	2024-04-24 17:09:46.881545243 +0100
@@ -263,7 +263,7 @@
 		}
 
 		if err := setWithProperType(structFieldKind, inputValue[0], structField); err != nil {
-			return err
+			return fmt.Errorf("%s: %w", inputFieldName, err)
 		}
 	}
 	return nil

This may not be the preferred way to fix it (and would require some string parsing of the error message to get the desired info) but it shows a proof of concept solution.

The result with the above fix is:

$ curl localhost:8080/submit -d"number=10a"
code=400, message=number: strconv.ParseInt: parsing "10a": invalid syntax, internal=number: strconv.ParseInt: parsing "10a": invalid syntax

Version/commit

v4.12

@oschonrock oschonrock changed the title When e.bind() fails with a type error on posted form data the inputfieldname is not available When c.bind() fails with a type error on posted form data the inputfieldname is not available Apr 24, 2024
@oschonrock oschonrock changed the title When c.bind() fails with a type error on posted form data the inputfieldname is not available When c.bind() fails with a type error on POST'd form data the inputfieldname is not available Apr 24, 2024
@oschonrock
Copy link
Author

oschonrock commented Apr 24, 2024

to make this work for time.Time fields we also need this:

$ diff -u ~/bind.go ~/go/pkg/mod/github.com/labstack/echo/[email protected]/bind.go 
--- /home/oliver/bind.go	2024-04-24 17:01:48.429244184 +0100
+++ /home/oliver/go/pkg/mod/github.com/labstack/echo/[email protected]/bind.go	2024-04-24 20:16:16.370829698 +0100
@@ -237,7 +237,7 @@
 
 		if ok, err := unmarshalInputToField(typeField.Type.Kind(), inputValue[0], structField); ok {
 			if err != nil {
-				return err
+				return fmt.Errorf("%s: %w", inputFieldName, err)
 			}
 			continue
 		}

no doubt there are few more.

If this type of "error wrapping" approach to providing failure context to the caller is what is wanrted, I am happy to create a pull request for the fullest possible set of cases.

@aldas
Copy link
Contributor

aldas commented Apr 24, 2024

If you are in a hurry you could use different binder that does not use struct tags and has errors containing field names. See this example line 34 bErr.Field:

b := echo.QueryParamsBinder(c)
errs := b.Int64("length", &length).
Int64s("ids", &opts.IDs).
Bool("active", &opts.Active).
BindErrors() // returns all errors
if errs != nil {
for _, err := range errs {
bErr := err.(*echo.BindingError)
log.Printf("in case you want to access what field: %s values: %v failed", bErr.Field, bErr.Values)
}
return fmt.Errorf("%v fields failed to bind", len(errs))
}
fmt.Printf("active = %v, length = %v, ids = %v", opts.Active, length, opts.IDs)

@oschonrock
Copy link
Author

oschonrock commented Apr 24, 2024

Thanks.

Yes, hadn't tried that. A little more verbose, but very powerful. Supports time format, arrays and required/must. Nice.

echo.BindingError Missing the type information though?

I can't construct an error message for the user saying "we were expecting an integer for field X but you gave us Y"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants