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

Joining relative path onto a file:// url with a hostname is non spec compliant #504

Open
piscisaureus opened this issue Jul 8, 2019 · 8 comments

Comments

@piscisaureus
Copy link

use url::Url;
fn main() {
    let u = Url::parse("file://server/some/dir/file")
        .unwrap()
        .join("/another/path/at/the/same/server")
        .unwrap();
    println!("{}", u);
}

Expected output: file://server/another/path/at/the/same/server
Actual output file:///another/path/at/the/same/server

Compare also the output from Firefox developer tools:

> new URL("/another/path/at/the/same/server", "file://server/some/dir/file")
URL {href: "file://server/another/path/at/the/same/server", origin: "file://", protocol: "file:", username: "", password: "", …}
@CYBAI
Copy link
Member

CYBAI commented Jul 8, 2019

new URL("/another/path/at/the/same/server", "file://server/some/dir/file")

I tried this in Firefox devtools and I get file:///another/path/at/the/same/server 👀

Firefox Nightly: 69.0a1 (2019-07-02) (64-bit)
image

Safari: Version 12.1 (14607.1.40.1.4)
image

also, I'm on macOS 10.14.4

@piscisaureus
Copy link
Author

piscisaureus commented Jul 8, 2019

Well your firefox doesn't agree with mine, then...
Let's look at some others:

Here's chrome: (Version 75.0.3770.100 (Official Build) (windows, 64-bit)
image

Node.js (v10.16.0, 64-bit, linux):
image

Edge (Microsoft Edge 42.17134.1.0; Microsoft EdgeHTML 17.17134):
image

@SimonSapin
Copy link
Member

Consider including the version numbers of all tools involved when testing. Note that some Firefox versions actually use rust-url for some things.

Also, the title of the issue mentions spec compliance but the description does not point to specification text at all, only the behavior of a specific implementation.

@piscisaureus
Copy link
Author

piscisaureus commented Jul 8, 2019

@SimonSapin
I added version numbers to #504 (comment)

W.r.t. referencing the spec: the whatwg url parsing spec is described as a state machine which makes it difficult to point at a specific "rule".
The W3C web platform test suite however contains a test for this specific case:

https://github.com/web-platform-tests/wpt/blob/255b99e5c2e0f5f8010adf6b69e3c1e0b73f7652/url/resources/urltestdata.json#L5554-L5556
and similar cases:
https://github.com/web-platform-tests/wpt/blob/255b99e5c2e0f5f8010adf6b69e3c1e0b73f7652/url/resources/urltestdata.json#L5525-L5527
https://github.com/web-platform-tests/wpt/blob/255b99e5c2e0f5f8010adf6b69e3c1e0b73f7652/url/resources/urltestdata.json#L5568-L5570

[
  {
    "input": "/..//localhost//pig",
    "base": "file://lion/",
    "href": "file://lion/localhost//pig",
    "protocol": "file:",
    "username": "",
    "password": "",
    "host": "lion",
    "hostname": "lion",
    "port": "",
    "pathname": "/localhost//pig",
    "search": "",
    "hash": ""
  },
  {
    "input": "/rooibos",
    "base": "file://tea/",
    "href": "file://tea/rooibos",
    "protocol": "file:",
    "username": "",
    "password": "",
    "host": "tea",
    "hostname": "tea",
    "port": "",
    "pathname": "/rooibos",
    "search": "",
    "hash": ""
  },
  {
    "input": "/?chai",
    "base": "file://tea/",
    "href": "file://tea/?chai",
    "protocol": "file:",
    "username": "",
    "password": "",
    "host": "tea",
    "hostname": "tea",
    "port": "",
    "pathname": "/",
    "search": "?chai",
    "hash": ""
  }
]

@CYBAI
Copy link
Member

CYBAI commented Jul 8, 2019

@piscisaureus I also updated my comment in #504 (comment). Looks like Safari currently agrees with Firefox.
But, after reading spec, I think the main issue here is if we should see the first path as host and I think it should be correct to see server in your example code as an opaque host.

Currently, Firefox, Safari and Servo failed on these 3 tests you mentioned.
Ref: https://wpt.fyi/results/url/url-constructor.html?label=master&label=experimental

@SimonSapin what do you think 👀?

@piscisaureus
Copy link
Author

I must admit, it's not as cut-and-dry as I made it sound,
Spec discussion: whatwg/url#405

@djc
Copy link
Contributor

djc commented Aug 25, 2020

Current rust-url passes all the tests in web-platform-tests. Is there still something to do here?

@annevk
Copy link

annevk commented Oct 8, 2021

Any remaining work can probably be tackled as part of #642.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants