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/url: no way to parse absolute URLs #16822

Closed
ThomasHabets opened this issue Aug 21, 2016 · 4 comments
Closed

net/url: no way to parse absolute URLs #16822

ThomasHabets opened this issue Aug 21, 2016 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.8Maybe

Comments

@ThomasHabets
Copy link
Contributor

#1. What version of Go are you using (go version)?

go version go1.7 linux/arm
#2. What operating system and processor architecture are you using (go env)?

GOARCH="arm"
GOBIN=""
GOEXE=""
GOHOSTARCH="arm"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/blaha/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_arm"
CC="gcc"
GOGCCFLAGS="-fPIC -marm -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build859569037=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

#3. What did you do?

https://play.golang.org/p/W4T12uL3_L

"cache_object" scheme is used by the squid proxy.
#4. What did you expect to see?

RFC3986 section 3.1:

   When presented with a URI that violates one or more scheme-specific
   restrictions, the scheme-specific resolution process should flag the
   reference as an error rather than ignore the unused parts; doing so
   reduces the number of equivalent URIs and helps detect abuses of the
   generic syntax, which might indicate that the URI has been
   constructed to mislead the user (Section 7.6).

I expect to get an error. I.e. for the play.golang.org snippet to panic.
#5. What did you see instead?

"Path" is set to the whole input.
@bradfitz
Copy link
Contributor

I'm not sure there's anything to fix here.

Go interprets the scheme per the spec;

      scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )

Which doesn't include "_".

But the part of the spec you cite regarding not ignoring parts on error would apply if we had a function to just parse an absolute URL only. We don't, though: our Parse function is defined (for better or worse) to parse either absolute or relative paths. And if it's not a valid absolute URL, we have no choice but to interpret it as a possible valid relative path.

Maybe we should add a way to parse in absolute mode only, in which case we could return an error.

It still doesn't help you parse "cache_object" bogus schemes.

@bradfitz bradfitz changed the title net/url: Invalid schemes transparently ignored counter to RFC net/url: no way to parse absolute URLs Aug 21, 2016
@ThomasHabets
Copy link
Contributor Author

Like I said I expect an error, because of the underscore.

Ah. That it would parse relative URLs did not occur to me, but it's even documented.

I'm not sure I'm reading this section 3.3 right:

   In addition, a URI reference
   (Section 4.1) may be a relative-path reference, in which case the
   first path segment cannot contain a colon (":") character

The way I read this "cache_object:" is the first path segment, and thus violates section 3.3. (without throwing an error in the Go implementation). Apologies if I'm misreading.

ThomasHabets added a commit to google/squidwarden that referenced this issue Aug 21, 2016
@ThomasHabets
Don't log cache_object:// URLs.
dcfe9a9 
They are an artefact of squid and can't be parsed by net/url.
Filed golang/go#16822 about it.
@bradfitz bradfitz added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 22, 2016
@bradfitz bradfitz added this to the Go1.8Maybe milestone Aug 22, 2016
@quentinmit quentinmit removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 13, 2016
@rsc
Copy link
Contributor

rsc commented Oct 20, 2016

Looking at the code, (*URL).String gets this right, so we should probably fix Parse. Sent CL 31582.

@rsc rsc added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 20, 2016
@gopherbot
Copy link

CL https://golang.org/cl/31582 mentions this issue.

timoreimann added a commit to gambol99/go-marathon that referenced this issue Feb 17, 2017
@timoreimann
Support parsing scheme-less URLs in Go 1.8.
c4308a3 
The behavior changed in 1.8.

See also the 1.8 release notes[1], this Go issue[2], and these
examples[3].

[1]https://golang.org/doc/go1.8#minor_library_changes
[2]golang/go#16822
[3]https://play.golang.org/p/EKOP85WqLd
timoreimann added a commit to gambol99/go-marathon that referenced this issue Feb 17, 2017
@timoreimann
Support parsing scheme-less URLs in Go 1.8.
12e256f 
The behavior changed in 1.8.

See also the 1.8 release notes[1], this Go issue[2], and these
examples[3].

[1]https://golang.org/doc/go1.8#minor_library_changes
[2]golang/go#16822
[3]https://play.golang.org/p/EKOP85WqLd
timoreimann added a commit to gambol99/go-marathon that referenced this issue Feb 17, 2017
@timoreimann
Support parsing scheme-less URLs in Go 1.8.
c0423ee 
The behavior changed in 1.8.

See also the 1.8 release notes[1], this Go issue[2], and these
examples[3].

[1]https://golang.org/doc/go1.8#minor_library_changes
[2]golang/go#16822
[3]https://play.golang.org/p/EKOP85WqLd
timoreimann added a commit to gambol99/go-marathon that referenced this issue Feb 20, 2017
@timoreimann
Fix URL parsing for Go 1.8. (#261)
38bd622 
The behavior changed in 1.8.

See also the 1.8 release notes[1], this Go issue[2], and these examples[3].

[1]https://golang.org/doc/go1.8#minor_library_changes
[2]golang/go#16822
[3]https://play.golang.org/p/EKOP85WqLd
jangie pushed a commit to jangie/go-marathon that referenced this issue May 6, 2017
@timoreimann @jangie
Fix URL parsing for Go 1.8. (gambol99#261)
d5c9ca0 
The behavior changed in 1.8.

See also the 1.8 release notes[1], this Go issue[2], and these examples[3].

[1]https://golang.org/doc/go1.8#minor_library_changes
[2]golang/go#16822
[3]https://play.golang.org/p/EKOP85WqLd
@golang golang locked and limited conversation to collaborators Oct 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.8Maybe
Development

No branches or pull requests
5 participants
@bradfitz @ThomasHabets @rsc @quentinmit @gopherbot