uri: restrict setting protocol to file scheme

[watilde]

As file URLs cannot have username/password/port, we should not keep them when scheme is changed to file.

Refs: whatwg/url#259

[paddor]

Isn’t this dangerous? Allowing from another scheme to file:// and keeping the path? IMO this should fail with a SecurityError.

What’s the use case?

[watilde]

@paddor That's the current behaviour as you can try and this patch doesn't touch it.

url = URI.parse('http://user:pass@example.com')
url.scheme = 'file'
puts url.to_s
# => file://user:pass@example.com

File URL can not have username, password and port, and this patch is just about how we handle it. It is possible to raise an error when arg_check = true at _check methods, but this change might be the smallest impact.

[paddor]

I'm just wondering, what's the use case? With or without this patch, setting the scheme to file while keeping the path seems dangerous to me.

[watilde]

For example, it can use the path as file names when caching contents. If we specify the base URL, it is not dangerous so far. I'm also interested in discussing how the path should be handled between http and file, but I think it should happen at whatwg/url instead since this behaviour is what is assumed by the spec.

[watilde]

// We might also be possible to open a new issue for it on the tracker :)

[paddor]

This seems much safer and intuitive to me (path sanitizing not included):

[1] pry(main)> http_uri = URI 'http://example.com:8080/path/to/file.pdf'
=> #<URI::HTTP http://example.com:8080/path/to/file.pdf>
[2] pry(main)> file_uri = URI "file://#{http_uri.path}"
=> #<URI::Generic file:///path/to/file.pdf>

I do think it's dangerous, because the path could contain ../../ and cause your cache to serve some other, possibly sensitive file, if you don't properly sanitize the path of the HTTP URI.

[watilde]

In my understanding, that ../../ will be ignored if it's trying to go to the shallower directory than the base path, so it will not be that critical imo.

e.g.

new URL('http://example.com/../../../foo/bar')
// => http://example.com/foo/bar

Either way, that seems reasonable to ask at the specification repository about what properties the parser should keep when the scheme is changed between file and http. I'm also curious what @nurse think on this since he seems to work hard on the parser of URI.

[paddor]

It doesn't seem that way:

[1] pry(main)> URI 'http://example.com/../../../foo/bar'
=> #<URI::HTTP http://example.com/../../../foo/bar>

[watilde]

Wow really... Then I agree with you. Not a critical in a real world because of fewer use cases, but might not make sense to keep the path when the scheme is updated to file from http/https if we do not manage the relative path that can be shallower than the base path.

[k0kubun]

Hmm, 04883f1 says it fixes this but it failed due to typo. Let me close this.