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

Request body not parsed when using X-HTTP-Method-Override header #3688

Closed
netrounds-guillaume opened this issue Oct 23, 2023 · 1 comment · Fixed by #3918
Closed

Request body not parsed when using X-HTTP-Method-Override header #3688

netrounds-guillaume opened this issue Oct 23, 2023 · 1 comment · Fixed by #3918

Comments

@netrounds-guillaume
Copy link

netrounds-guillaume commented Oct 23, 2023

🐛 Bug Report

There is a bug around this code:

grpc-gateway/runtime/mux.go

Lines 324 to 330 in 1c444cc

r.Method = strings.ToUpper(override)
if err := r.ParseForm(); err != nil {
_, outboundMarshaler := MarshalerForRequest(s, r)
sterr := status.Error(codes.InvalidArgument, err.Error())
s.errorHandler(ctx, s, outboundMarshaler, w, r, sterr)
return
}

r.Method is overridden to GET before r.ParseForm() which causes ParseForm to skip parsing the request body and goes straight to parsing from the query string parameters. Since the whole point of using X-HTTP-Method-Override is to move the query string parameters to the request body, no query string parameters are present in the URL

One possible solution is to move r.Method = strings.ToUpper(override) after the r.ParseForm() block.

To Reproduce

Send a POST request to a GET endpoint with Content-Type: application/x-www-form-urlencoded and X-HTTP-Method-Override: GET headers. Set the request body with what would be query string parameters e.g. limit=15&page=1

Expected behavior

Form parameters are parsed and passed along to the API handler

Actual Behavior

Form parameters are not parsed and none of the parameters are populated when reaching the API handler

Your Environment

(no info)

@johanbrandhorst
Copy link
Collaborator

Hi, thanks for the detailed issue! The problem as you describe it seems indeed to be that we're mutating the request before parsing. Would you be willing to contribute a PR with a fix and a test confirming the fix?

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 a pull request may close this issue.

2 participants