1873976 - Incorrect parsing: <file:c:\foo\bar.html> against <file:///tmp/mock/path>

Status: RESOLVED FIXED

(Core :: Networking, defect, P3)

Description

[Valentin Gosu [:valentin]]

https://jsdom.github.io/whatwg-url/#url=ZmlsZTpjOlxmb29cYmFyLmh0bWw=&base=ZmlsZTovLy90bXAvbW9jay9wYXRo

It seems the parser is thrown off by file:c: file:/c: works fine

Comment 1

[Valentin Gosu [:valentin]]

I'm also using this bug to track other relative file: failures

<..> against <file:///C:/>
<file:..> against http://www.example.com/test
etc

Comment 3

[Sean Kim [:sekim]]

Attachment: Bug 1873976 - Fix incorrect parsing: <file:c:\foo\bar.html> against <file:///tmp/mock/path> and <..> against <file:///C:/> r?valentin

Comment 4

[Sean Kim [:sekim]]

<..> against <file:///C:/> might need to be a separate patch, but I incorporated it in this patch.

Comment 5

[edgul]

I did a quick pass over the tests we expect to be fixed by this bug. I'm pretty sure these ones should be addressed here:

Parsing: < File:c|////foo\bar.html> against <file:///tmp/mock/path>
Parsing: <C|/foo/bar> against <file:///tmp/mock/path>
Parsing: <..> against <file:///C:/>
Parsing: </> against <file:///C:/a/b>
Parsing: </> against <file://h/C:/a/b>

@valentin, I noticed that these ones were attributed to Bug 1507354, however they don't seem to be addressed by the current patch we have. Can you comment on whether they should be included in this bug, Bug 1507354, or maybe we need a new bug?:

Parsing: <C|> against <file://host/D:/dir1/dir2/file>
Parsing: <C|#> against <file://host/dir/file>
Parsing: <C|?> against <file://host/dir/file>
Parsing: <C|/> against <file://host/dir/file>
Parsing: <C| /> against <file://host/dir/file>
Parsing: <C|\> against <file://host/dir/file>
Parsing: <file://[1::8]/C:/> against <about:blank>```

Comment 6

[Valentin Gosu [:valentin]]

(In reply to Ed Guloien [:edgul] from comment #5)

I did a quick pass over the tests we expect to be fixed by this bug. I'm pretty sure these ones should be addressed here:

Parsing: < File:c|////foo\bar.html> against <file:///tmp/mock/path>
Parsing: <C|/foo/bar> against <file:///tmp/mock/path>
Parsing: <..> against <file:///C:/>
Parsing: </> against <file:///C:/a/b>
Parsing: </> against <file://h/C:/a/b>

Right, these should be fixed here, except the last one which has a host and should go to bug 1507354.

@valentin, I noticed that these ones were attributed to Bug 1507354, however they don't seem to be addressed by the current patch we have. Can you comment on whether they should be included in this bug, Bug 1507354, or maybe we need a new bug?:

Parsing: <C|> against <file://host/D:/dir1/dir2/file>
Parsing: <C|#> against <file://host/dir/file>
Parsing: <C|?> against <file://host/dir/file>
Parsing: <C|/> against <file://host/dir/file>
Parsing: <C| /> against <file://host/dir/file>
Parsing: <C|\> against <file://host/dir/file>
Parsing: <file://[1::8]/C:/> against <about:blank>```

We can check whether applying the patches in bug 1507354 and the patches here fixes these test cases too, but we can't fix these without allowing a host in file URLs.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52a75a6d604a
Fix incorrect parsing: <file:c:\foo\bar.html> against <file:///tmp/mock/path> and <..> against <file:///C:/> r=necko-reviewers,valentin

Comment 8

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/52a75a6d604a