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

net/http: Request.Cookie returns first cookie when name is empty #53181

Closed
muyizixiu opened this issue Jun 1, 2022 · 7 comments
Closed

net/http: Request.Cookie returns first cookie when name is empty #53181

muyizixiu opened this issue Jun 1, 2022 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.

Comments

@muyizixiu
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.17.9 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/data/root/.cache/go-build"
GOENV="/data/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/go/pkg/mod"
GONOPROXY=""
GONOSUMDB="github.com/galeone/tensorflow"
GOOS="linux"
GOPATH="/go"
GOPRIVATE=""
GOPROXY=""
GOROOT="/usr/local/go"
GOSUMDB="sum.woa.com+643d7a06+Ac5f5VOC4N8NUXdmhbm8pZSXIWfhek5JSmWdWrq7pLX4"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.9"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/data/micro/admin/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2273884838=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.17.9 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.17.9
uname -sr: Linux 3.10.107-1-tlinux2_kvm_guest-0052
/lib64/libc.so.6: GNU C Library (GNU libc) development release version 2.34.9000.

What did you do?

see link blow
https://go.dev/play/p/svsT3co5Y_X

What did you expect to see?

it suppose to return http.ErrNoCookie

What did you see instead?

unexpected cookie

@kamly
Copy link

kamly commented Jun 1, 2022

the same thing happened to me !!

@muyizixiu
Copy link
Contributor Author

readCookies will return all cookie when cookie name is empty
企业微信截图_1d1c56ab-ee6f-4132-811b-4253c7fc5c11
it only return matched cookie when filter is not empty
企业微信截图_e69f1333-7af2-4388-bf56-3d5f8aa56d11

should be fixed by follow

func (r *Request) Cookie(name string) (*Cookie, error) {
	for _, c := range readCookies(r.Header, name) {
		if c.Name == name {
			return c, nil
		}
	}
	return nil, ErrNoCookie
}

muyizixiu pushed a commit to muyizixiu/go that referenced this issue Jun 1, 2022
…hen cookie name is empty

http.Cookie(name string) will return the first cookie when name is a empty string.
since readCookies in file net/http/cookie.go at 247 will return all cookies when second parameter is a empty string
and http.Cookie don`t check whether cookie name match the parameter.
so solution is to check whether cookie name match the specific parameter.

Fixes golang#53181
muyizixiu pushed a commit to muyizixiu/go that referenced this issue Jun 1, 2022
…hen cookie name is empty

http.Cookie(name string) will return the first cookie when name is a empty string.
since readCookies in file net/http/cookie.go at 247 will return all cookies when second parameter is a empty string
and http.Cookie don`t check whether cookie name match the parameter.
so solution is to check whether cookie name match the specific parameter.

Fixes golang#53181
muyizixiu added a commit to muyizixiu/go that referenced this issue Jun 1, 2022
…hen cookie name is empty

http.Cookie(name string) will return the first cookie when name is a empty string.
since readCookies in file net/http/cookie.go at 247 will return all cookies when second parameter is a empty string
and http.Cookie don`t check whether cookie name match the parameter.
so solution is to check whether cookie name match the specific parameter.

Fixes golang#53181
@gopherbot
Copy link

Change https://go.dev/cl/409754 mentions this issue: net/http: fix unexpected cookie returned by Request.Cookie function w…

@seankhliao seankhliao changed the title affected/package: net/http wrong cookie value returned when cookie name is empty net/http: Request.Cookie returns first cookie when name is empty Jun 1, 2022
@seankhliao seankhliao added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 1, 2022
@earthboundkid
Copy link
Contributor

"" is not a valid cookie name. A) It's not clear if this behavior should change. Is anyone relying on this undocumented behavior? B) If it should change, the fix should be

func (r *Request) Cookie(name string) (*Cookie, error) {
	if name == "" {
		return nil, ErrNoCookie
	}
	for _, c := range readCookies(r.Header, name) {
		return c, nil
	}
	return nil, ErrNoCookie
}

@seankhliao
Copy link
Member

cc @neild

@kamly
Copy link

kamly commented Jun 1, 2022

I think cookie's lack of key is unreasonable and should be pointed out.

@neild
Copy link
Contributor

neild commented Jun 1, 2022

Given that the current behavior is undocumented, surprising, and doesn't seem to be useful, I think Request.Cookie("") should return ErrNoCookie.

muyizixiu added a commit to muyizixiu/go that referenced this issue Jun 2, 2022
…hen cookie name is empty

http.Cookie(name string) will return the first cookie when name is a empty string.
since readCookies in file net/http/cookie.go at 247 will return all cookies when second parameter is a empty string
and http.Cookie don`t check whether cookie name match the parameter.
so solution is to check whether cookie name match the specific parameter.

Fixes golang#53181
muyizixiu added a commit to muyizixiu/go that referenced this issue Jun 2, 2022
…hen cookie name is empty

Request.Cookie(name string) will return the first cookie when name is "".
since readCookies in file net/http/cookie.go at line 247 will return all cookies when second parameter is a empty string
and http.Cookie don`t check whether cookie name match the parameter.
so solution is to return ErrNoCookie if cookie name is "".

Fixes golang#53181
muyizixiu added a commit to muyizixiu/go that referenced this issue Jun 8, 2022
Request.Cookie(name string) will return the first cookie
when cookie name is "". Since readCookies in
file net/http/cookie.go at line 247 return all cookies
when second parameter is a empty string.

To fix it, Return ErrNoCookie from Request.Cookie(""),
instead of the first cookie in the request.

Fixes golang#53181

Change-Id: Ie623ca4c53da64ef7623a7863292a2d771f76832
GitHub-Last-Rev: 1a6903f
GitHub-Pull-Request: golang#53183
muyizixiu added a commit to muyizixiu/go that referenced this issue Jun 8, 2022
Request.Cookie(name string) will return the first cookie
when cookie name is "". Since readCookies in
file net/http/cookie.go at line 247 return all cookies
when second parameter is a empty string.

To fix it, Return ErrNoCookie from Request.Cookie(""),
instead of the first cookie in the request.

Fixes golang#53181

Change-Id: Ie623ca4c53da64ef7623a7863292a2d771f76832
GitHub-Last-Rev: 1a6903f
GitHub-Pull-Request: golang#53183
muyizixiu added a commit to muyizixiu/go that referenced this issue Aug 17, 2022
Request.Cookie(name string) will return the first cookie
when cookie name is "". Since readCookies in
file net/http/cookie.go at line 247 return all cookies
when second parameter is a empty string.

To fix it, Return ErrNoCookie from Request.Cookie(""),
instead of the first cookie in the request.

Fixes golang#53181

Change-Id: Ie623ca4c53da64ef7623a7863292a2d771f76832
GitHub-Last-Rev: 1a6903f
GitHub-Pull-Request: golang#53183
@golang golang locked and limited conversation to collaborators Aug 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants