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/httptest: NewRequest panics whereas net/http NewRequest does not #26083

Closed
arthurgustin opened this issue Jun 27, 2018 · 4 comments
Closed

net/http/httptest: NewRequest panics whereas net/http NewRequest does not #26083

arthurgustin opened this issue Jun 27, 2018 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.11

Comments

@arthurgustin
Copy link

Please answer these questions before submitting your issue. Thanks!

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

go version go1.10.2 linux/amd64

Does this issue reproduce with the latest release?

Yes on the go playground

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/agustin/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/agustin/gocode"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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-build158217544=/tmp/go-build -gno-record-gcc-switches"

What did you do?

In unit tests, I use httptest to simulate user requests.

What did you expect to see?

The request properly created

What did you see instead?

Test Panicked
                invalid NewRequest arguments; malformed HTTP version "((0 0, 0 10, 10 10, 10 0, 0 0),(3 3, 3 7, 7 7, 7 3, 3 3))&relation=contains HTTP/1.0"
                /usr/local/go/src/runtime/panic.go:502

                Full Stack Trace
                	/usr/local/go/src/runtime/panic.go:502 +0x229
                net/http/httptest.NewRequest(0x94b42a, 0x3, 0xc42002c4d0, 0x6a, 0x7f81c8858bd0, 0xc4209a14a0, 0x8e6800)
                	/usr/local/go/src/net/http/httptest/httptest.go:47 +0x690

Playground link

https://play.golang.org/p/SB4a-aRS6xr
@agnivade agnivade changed the title net/http/httptest NewRequest panics whereas net/http NewRequest does not net/http/httptest: NewRequest panics whereas net/http NewRequest does not Jun 27, 2018
@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 27, 2018
@agnivade agnivade added this to the Go1.11 milestone Jun 27, 2018
@odeke-em
Copy link
Member

Thank you for the report @arthurgustin and thank you for the triage @agnivade!

@agnivade as we are quite late in the Go1.11 cycle, perhaps let's punt this off to Go1.12 or even make it Unreleased? This code has been like this for the past 2 years since March 2016 for Go1.7 as per 694eadc aka CL 21016

Paging @bradfitz too as in #14199 this was intended for outbound requests and using http.ReadRequest.

@odeke-em
Copy link
Member

*inbound requests

@bradfitz
Copy link
Contributor

This is working as intended. httptest.NewRequest is for incoming requests. And it's for tests, so panicking is fine, and the target represents the on-wire representation, which doesn't permit spaces. http.NewRequest is for outgoing requests, and represents a URL, not its wire representation.

@arthurgustin
Copy link
Author

arthurgustin commented Jun 27, 2018 via email

@golang golang locked and limited conversation to collaborators Jun 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
5 participants
@bradfitz @agnivade @odeke-em @gopherbot @arthurgustin