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

Schema Decoder: Ignore unknown fields by default #308

Merged
merged 1 commit into from
Apr 27, 2020

Conversation

kabychow
Copy link
Contributor

@kabychow kabychow commented Apr 26, 2020

Hi Guys,

Thanks for your great work! It seems like BodyParser is generating error when there are unknown fields in the form/query. For example:

type User struct {
	Email string `form:"email"`
	Password string `form:"password"`
}

If I send the request with these parameters:
email: abc@gmail.com
password: abc
token_type: jwt

It will generate error schema: invalid path "token_type"

I think it should be ignored peacefully. Let me know how you think :)

@welcome
Copy link

welcome bot commented Apr 26, 2020

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you want to chat with us, join ons on Telegram https://t.me/gofiber

@Fenny
Copy link
Member

Fenny commented Apr 26, 2020

Welcome @Khaibin, thanks for opening your first issue.

I invite the @gofiber/maintainers to also give their feedback on whether we should enable IgnoreUnknownKeys by default in our schema decoder

@kabychow
Copy link
Contributor Author

kabychow commented Apr 26, 2020

Hi @Fenny, thanks for your reply :)

Consider this:

user := &User{}
err := c.BodyParser(user)
if err != nil {
  c.SendStatus(400)
}

By default, the server should not return with 400 (Bad request) when additional fields are present. There are cases where we need some extra fields to do further processing.

Gorilla schema returns the error instance as MultiError and it will complicate the process of handling unknown fields.

Gin are ignoring the fields by default for all Binding methods:
https://github.com/gin-gonic/gin#model-binding-and-validation

Copy link
Member

@Fenny Fenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Khaibin if you could add testing for this by adding another field to the struct in the ctx_test.go#L156 that would be great.

@Fenny Fenny merged commit eda372d into gofiber:master Apr 27, 2020
@welcome
Copy link

welcome bot commented Apr 27, 2020

Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you want to chat with us, join ons on Telegram https://t.me/gofiber

@Fenny Fenny mentioned this pull request Apr 28, 2020
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

Successfully merging this pull request may close these issues.

5 participants