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

feat: support custom formvalue function #1453

Merged

Conversation

li-jin-gou
Copy link
Contributor

@li-jin-gou li-jin-gou commented Dec 8, 2022

Some gin or net/http services have a different formvalue function when migrating to fasthttp.
it maybe causes some bugs.

net/http:

// FormValue returns the first value for the named component of the query.
// POST and PUT body parameters take precedence over URL query string values.
// FormValue calls ParseMultipartForm and ParseForm if necessary and ignores
// any errors returned by these functions.
// If key is not present, FormValue returns the empty string.
// To access multiple values of the same key, call ParseForm and
// then inspect Request.Form directly.

The user can customize a formvalue function to avoid this problem, but it would be better if the framework provided this switch.

@li-jin-gou li-jin-gou force-pushed the feat/add_custom_formvalue_func branch from 3a67d42 to efe5988 Compare December 8, 2022 12:23
Copy link
Collaborator

@erikdubbelboer erikdubbelboer left a comment

Choose a reason for hiding this comment

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

Do you have a real world example of this going wrong or is it something you think might go wrong?

server.go Outdated
return defaultFormValue(ctx, key)
}

type FormValue func(*RequestCtx, string) []byte
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
type FormValue func(*RequestCtx, string) []byte
type FormValueFunc func(*RequestCtx, string) []byte

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

server.go Outdated

// SetStandardFormValueFunc sets FormValue function implementation to get form value.
// Consistent behaviour with net/http. POST and PUT body parameters take precedence over URL query string values.
func SetStandardFormValueFunc() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func SetStandardFormValueFunc() {
func SetNetHttpFormValueFunc() {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@li-jin-gou
Copy link
Contributor Author

li-jin-gou commented Dec 10, 2022

Do you have a real world example of this going wrong or is it something you think might go wrong?

Yes, there was a problem with the code in the real environment and I noticed the difference. the front end passes the same parameters and different values in the http body and query and result in an unintended bug...😩

@li-jin-gou li-jin-gou force-pushed the feat/add_custom_formvalue_func branch 3 times, most recently from faf8119 to f58e4ab Compare December 15, 2022 15:02
server.go Outdated
@@ -412,6 +412,7 @@ type Server struct {
concurrencyCh chan struct{}
perIPConnCounter perIPConnCounter
serverName atomic.Value
FormValueFunc FormValueFunc
Copy link
Collaborator

Choose a reason for hiding this comment

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

This field needs some documentation since it's exposed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks and done

server.go Outdated
if len(vv) > 0 {
return []byte(vv[0])

// NetHttpFormValueFunc Consistent behaviour with net/http. POST and PUT body parameters take precedence over URL query string values.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// NetHttpFormValueFunc Consistent behaviour with net/http. POST and PUT body parameters take precedence over URL query string values.
// NetHttpFormValueFunc gives consistent behavior with net/http. POST and PUT body parameters take precedence over URL query string values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks

@li-jin-gou li-jin-gou force-pushed the feat/add_custom_formvalue_func branch 3 times, most recently from e8e52d2 to 56a135e Compare December 24, 2022 17:50
@erikdubbelboer erikdubbelboer merged commit b788e66 into valyala:master Dec 25, 2022
@erikdubbelboer
Copy link
Collaborator

Thanks!

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.

None yet

2 participants