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.

Sign up for GitHub

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

Jump to bottom

net/http: FormValue doesn't take the body parameter precedence over the URL query string value #64575

Closed
stonezdj opened this issue Dec 6, 2023 · 2 comments
Closed

net/http: FormValue doesn't take the body parameter precedence over the URL query string value #64575

stonezdj opened this issue Dec 6, 2023 · 2 comments
Labels
Documentation NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.22

Comments

@stonezdj
Copy link

stonezdj commented Dec 6, 2023

Go version

go version go1.20.6 darwin/amd64

What operating system and processor architecture are you using (go env)?

GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/daojunz/Library/Caches/go-build"
GOENV="/Users/daojunz/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/daojunz/Documents/goworkdir/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/daojunz/Documents/goworkdir"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/daojunz/homebrew/Cellar/go/1.20.6/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/daojunz/homebrew/Cellar/go/1.20.6/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.20.6"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="cc"
CXX="c++"
CGO_ENABLED="1"
GOMOD="/Users/daojunz/Documents/goworkdir/src/github.com/goharbor/tracker/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/pr/bkvcjj5d6b55_7mzt5k8r6h00000gp/T/go-build3717534302=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://pkg.go.dev/net/http#Request.FormValue

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.

Sample application:
https://go.dev/play/p/B_9IyJ7YTEL

What did you expect to see?

curl --location 'http://127.0.0.1:8080/?name=potato' --form 'name="orange"'

Expected output should be:

Name: orange

Because the POST and PUT body parameters take precedence over URL query string values.

What did you see instead?

Server output:

Name: potato
@callthingsoff callthingsoff mentioned this issue Dec 6, 2023
Closed
@gopherbot
Copy link

Change https://go.dev/cl/547855 mentions this issue: net/http: query values from r.PostForm first in *Request.FormValue

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 6, 2023
@cagedmantis cagedmantis added this to the Backlog milestone Dec 6, 2023
@neild
Copy link
Contributor

neild commented Dec 11, 2023

It seems that the precedence order is:

  • application/x-www-form-urlencoded form body (POST, PUT, PATCH only)
  • query parameters (always)
  • multipart/form-data form body (always)

So far as I can tell, this has been the case since Request.PostForm was added in https://golang.org/cl/6210067. Prior to that CL, query parameters always took precedence over form bodies.

CL 6210067 added this line to the ParseForm documentation:

// POST and PUT body parameters take precedence over URL query string values.

This is only true for application/x-www-form-urlencoded body parameters. I don't know if this was intentional or not; it seems plausible that it was unintentional. Whether intentional or not, either the documentation or the function behavior needs to be updated.

@dmitshur dmitshur modified the milestones: Backlog, Go1.22 Jan 9, 2024
@dmitshur dmitshur added Documentation NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 9, 2024
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
@callthingsoff @ezz-no
net/http: clarify the precedence order for Request.FormValue
f8e3fd4 
Fixes golang#64575

Change-Id: I0eaec642a9dc8ae3b273a6d41131cc7cb8332947
GitHub-Last-Rev: 17aa517
GitHub-Pull-Request: golang#64578
Reviewed-on: https://go-review.googlesource.com/c/go/+/547855
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.22
Development

Successfully merging a pull request may close this issue.

net/http: clarify the precedence order for Request.FormValue callthingsoff/go
5 participants
@neild @cagedmantis @dmitshur @stonezdj @gopherbot