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/url: add test of "Windows" file URL #6027

Closed
gopherbot opened this issue Aug 3, 2013 · 11 comments
Closed

net/url: add test of "Windows" file URL #6027

gopherbot opened this issue Aug 3, 2013 · 11 comments
Milestone

Comments

@gopherbot
Copy link

by mitchell.hashimoto:

Go version: go version go1.1.1 darwin/amd64

Playground example showcasing bug:
http://play.golang.org/p/XjdafRyjCH

When parsing valid Windows `file:///` URLs, Go prefixes the path with a "/".
While this is easy to workaround, this makes for an invalid path on Windows, such that
the slash must always be eliminated prior to doing any file work.

While the RFC doesn't provide any concrete information on how URLs are formed, Windows
itself predictably handles file URLs in a specific way, and even documented some of the
ways on the official IE blog[1].

[1]: http://blogs.msdn.com/b/ie/archive/2006/12/06/file-uris-in-windows.aspx
@robpike
Copy link
Contributor

robpike commented Aug 4, 2013

Comment 1:

Labels changed: added priority-later, os-windows, removed priority-triage.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Sep 16, 2013

Comment 2:

We're not going to get to this for Go 1.2.
It's not obvious to me what the fix is either. Should net/url's behavior really depend
on which OS the program runs on? Does that mean you cannot manipulate Windows file URLs
on Linux machines or vice versa?

Labels changed: added go1.3, removed go1.2maybe.

Status changed to Thinking.

@alexbrainman
Copy link
Member

Comment 3:

I didn't have chance to look into this properly. But I think we need to do something
here. Like you suggested, lets leave for after Go 1.2. 
Alex

@gopherbot
Copy link
Author

Comment 4 by mitchell.hashimoto:

Yeah after more thought the unfortunate thing is that it just isn't clear whether the
URL is a Windows file URL or not. Changing the behavior based on the OS that Go runs on
is kind of silly I think. 
I've worked around this at the moment by just stripping the first character and checking
if it exists. 
I'm not sure what Go should do here.

@minux
Copy link
Member

minux commented Sep 17, 2013

Comment 5:

what do other libraries for other languages do on the problem?

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 6:

Labels changed: added release-go1.3, removed go1.3.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 7:

Labels changed: added repo-main.

@mattn
Copy link
Member

mattn commented Feb 13, 2014

Comment 8:

I think this is not a bug. For example, we can create directory like follow on unix.
$ mkdir -p 'C:/Program Files/foo/bar'
Whether the url is treated as windows path or not, it depend on the OS which treat the
URL.

@mariosnikolaou
Copy link

Comment 9:

Leaving it as is results in easy to reason behaviour and avoids overcomplicated edge
cases, as can be seen by the mkdir example above.
I tested some other libraries handling of URIs/URLs (on Mac OS X 10.9)
Python 2 and 3
    >>> urlparse('http:///C:/Directory/')
    ParseResult(scheme='http', netloc='', path='/C:/Directory/', params='', query='', fragment='')
    >>> urlparse('http:///Directory/')
    ParseResult(scheme='http', netloc='', path='/Directory/', params='', query='', fragment='')
    >>> urlparse('http://www.google.com/')
    ParseResult(scheme='http', netloc='www.google.com', path='/', params='', query='', fragment='')
Node.JS 0.10.26
    > url.parse('file:///C:/Directory/').pathname
    '/C:/Directory/'
    > url.parse('http:///Directory/').pathname
    '/Directory/'
    > url.parse('http://www.google.com/').pathname
    '/'
Java 1.6.0 (java.net.URL and getPath())
    URL("file:///C:/Directory/");   // => /C:/Directory/
    URL("file:///Directory/");      // => /Directory/
    URL("http://www.google.com/");  // => /
Mono 3.2.7 (System.Uri and AbsolutePath)
    uri = new Uri("file:///C:/Directory/");   // => C:/Directory/
    uri = new Uri("file:///Directory/");      // => /Directory/
    uri = new Uri("http://www.google.com/");  // => /
Of these, only Mono (.Net in general?) will parse the URI differently, removing the
leading '/'' infront of a drive letter.
Given this and reading RFC3986 for Generic syntax for URI
(http://tools.ietf.org/html/rfc3986), it is best to just treat the path as Go currently
does. In addition, reading the file-scheme RFC draft
(http://www.ietf.org/id/draft-kerwin-file-scheme or
https://github.com/phluid61/file-uri-scheme) shows how many edge cases there can be if
we start treating file-schemes, so it might be hard to draw the line for file-scheme
Path parsing? Can we just let the Path be the path as specified by RFC 3986 (in decoded
form as the documentation in net/url states)?
In my view, the current behaviour matches Go's least-surprise property.

@rsc
Copy link
Contributor

rsc commented Apr 3, 2014

Comment 10:

Thank you for the analysis in comment 9. You've convinced me the parsing is correct as
is.
CL 83930046 will add a test that we preserve the current behavior.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Apr 3, 2014

Comment 11:

This issue was closed by revision 844b625.

Status changed to Fixed.

@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@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.
Projects
None yet
Development

No branches or pull requests

7 participants