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: URL.String can return invalid string if given invalid URL.Path #5927

Closed
eaigner opened this issue Jul 21, 2013 · 7 comments
Closed

net/url: URL.String can return invalid string if given invalid URL.Path #5927

eaigner opened this issue Jul 21, 2013 · 7 comments
Labels
FrozenDueToAge
Milestone
Go1.2

Comments

@eaigner
Copy link
Contributor

eaigner commented Jul 21, 2013 

When a path is set on a URL without a leading slash, String() does not add one

http://play.golang.org/p/AQR3kXpXrf

I don't know if is intended behavior, but it I guess it would be better handled with
automatically pre-pending the slash since the documentation for this function also states

    "String reassembles the URL into a valid URL string."

which is obviously not the case here.
@scottferg
Copy link
Contributor

Comment 1:

Seems like String() is ignoring the lack of a slash when it writes the path to the
output. In your provided case, if you add the leading slash when setting u.Path it works
fine. I've attached a proposed fix for the issue in url.go, as well as a source file
that shows which cases work and which do not.  I can push up a CL if there's interest.

Attachments:

  1. slash.go (338 bytes)
  2. urlfix.diff (423 bytes)

@scottferg
Copy link
Contributor

Comment 2:

CL submitted as https://golang.org/cl/11698045/

@mitchellh
Copy link

Comment 3:

I just ran into this as well and would consider this a bug. 
According to the URL RFC (1738), not section 3.1 (and all following schemas), the
prefixed "/" is expected to exist and must be there if there is a path.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 4:

Still not sure whether this is just a doc fix.

Labels changed: added priority-later, go1.2, removed priority-triage.

Status changed to Accepted.

@mitchellh
Copy link

Comment 5:

I would consider this a bug solely based on this Go Playground example:
http://play.golang.org/p/XWK4tgBqYo
I would consider parsing the String() version of a *URL to turn back into an identical
representation of the URL that made the string. However, due to this bug, it turns into
a completely different (and wrong) URL object.

@rsc
Copy link
Contributor

rsc commented Aug 1, 2013

Comment 6:

@xmitchx, the input you are passing to u.String is not a valid URL. No valid URL can
decode back to that identical struct, because any decoded URL will put a leading / on
the path.

@bradfitz
Copy link
Contributor

bradfitz commented Aug 1, 2013

Comment 7:

This issue was closed by revision 39679ca.

Status changed to Fixed.

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.2 label Apr 14, 2015
cbednarski added a commit to hashicorp/packer that referenced this issue Jul 31, 2015
@cbednarski
Remove code to work around a URL bug in go core
f787193 
- Found while grepping for code.google.com
- This was fixed here golang/go#5927

Pretty sure we don't support whatever version of go that was written for anymore.
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.2
Development

No branches or pull requests
6 participants
@mitchellh @bradfitz @scottferg @rsc @eaigner @gopherbot