1815926 - URL host parser does not accept * or "

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[Valentin Gosu [:valentin]]

We're failing to parse the following URL
Origin parsing: http://!"$&'()*+,-.;=_`{}~/ against <about:blank>

Comment 1

[Oliver Medhurst [:canadahonk]]

Attachment: Bug 1815926 - Accept parsing * and " in URL hosts

• and " should be valid characters when parsing a host.

WIP. TODO:

• Find and retest relevant WPT tests to validate changes
• Update WPT expectations

Depends on D170528

Comment 2

[:Gijs (he/him)]

Oliver, are you still working on this?

Comment 3

[Oliver Medhurst [:canadahonk]]

Sorry, no, I abandoned the patch last week after Valentin said a while ago that there are many various internal changes needed as we do expect none of these characters in a few places currently.

Comment 4

[edgul]

This bug will not be handled as a part of interop-2024-url, but will likely need addressing for the purposes of URL Pattern.

Comment 6

[Henri Sivonen (:hsivonen)]

In order to keep the scope of bug 1889536 manageable, I intend not to fix this as part of that bug. However, for future reference, I'm in the process of moving the relevant ASCII deny list declaration from nsStandardURL.cpp to netwerk/base/idna_glue/src/lib.rs.

Comment 7

[Henri Sivonen (:hsivonen)]

Filed bug 1912011 about taking an already-possible step in this direction.

Comment 8

[Henri Sivonen (:hsivonen)]

Let's see the current status of test breakage if we did this:
https://treeherder.mozilla.org/jobs?repo=try&revision=1d9e2b73dc599341dc4997d04768dfbaba188bad

Comment 9

[Henri Sivonen (:hsivonen)]

Quota manager expects nsStandardURL host names to be usable as file names on Windows, so there's this assertion:
https://searchfox.org/mozilla-central/rev/891d104826fb0cfd5cbdd6128e2372ce62810028/caps/OriginAttributes.cpp#292
(See bug 1196371.)

Resolving the above in a way that doesn't break anything is probably the main task left here. bvandersloot, do you have advice on what the issues are here?

The cookie permission UI wants the asterisk to be rejected in URLs:
https://searchfox.org/mozilla-central/source/browser/components/preferences/tests/browser_permissions_checkPermissionsWereAdded.js

https://searchfox.org/mozilla-central/source/toolkit/components/extensions/test/xpcshell/test_ext_schemas.js also tests for asterisk invalidity in extension origin match, but to me it looks like a test-only issue.

Comment 10

[Henri Sivonen (:hsivonen)]

Added a comment on a relevant spec issue: https://github.com/whatwg/url/issues/815#issuecomment-2275009761

Comment 11

[Benjamin VanderSloot [:bvandersloot]]

Re quota manager (this isn't my area of expertise): we probably have to sanitize the url string for use as filename, since the acceptable characters of URLs are not a subset of that for files on windows. But I'd talk to :asuth for more info there.

Re cookie permission: we disallow the user from setting permission exemptions with a *, in order to prevent the confusion that they can set wildcards. This would require some UI change. Although we have a clean way to do this- strip the *. and it should work as the user expects. Then we can just permit * so foo*.example.com would be allowed. :pbz - does that sound reasonable as a solution here, since you know more about permissions than me?

I expect the last example to be a similar consideration, given this test line but I don't know enough about extensions to be certain.

Comment 12

[Henri Sivonen (:hsivonen)]

(In reply to Benjamin VanderSloot [:bvandersloot] from comment #11)

Re cookie permission: we disallow the user from setting permission exemptions with a *, in order to prevent the confusion that they can set wildcards. This would require some UI change. Although we have a clean way to do this- strip the *. and it should work as the user expects. Then we can just permit * so foo*.example.com would be allowed. :pbz - does that sound reasonable as a solution here, since you know more about permissions than me?

We could add a check that reads the host from the parsed URL in the cookie permission UI code and rejects it if it contains an asterisk even if nsStandardURL itself didn't reject the asterisk. After all, the asterisk is used as a wildcard on the TLS certificate layer, which means we've already rather committed to not supporting a literal asterisk in DNS naming.

Comment 13

[Emma Zühlcke [:emz]]

There is no explicit check for "*" in the frontend. The URI constructor here fails for inputs like "*.example.com" here: https://searchfox.org/mozilla-central/rev/10fcb8561f56db489a5a7a69e1f01392bcecd830/browser/components/preferences/dialogs/permissions.js#309

Comment 14

[:Gijs (he/him)]

(In reply to Paul Zühlcke [:pbz] from comment #13)

There is no explicit check for "*" in the frontend. The URI constructor here fails for inputs like "*.example.com" here: https://searchfox.org/mozilla-central/rev/10fcb8561f56db489a5a7a69e1f01392bcecd830/browser/components/preferences/dialogs/permissions.js#309

Yes, I think Benjamin's question (in comment #11, after comment #9) is, if we change the URI constructor to not throw, can we add frontend checks so we would do the right thing when the host contains *.

Comment 15

[Emma Zühlcke [:emz]]

Yes, we can! Checking for *. seems reasonable.

Comment 16

[Valentin Gosu [:valentin]]

Attachment: Bug 1815926 - URL host parser does not accept * or " r=#necko

Comment 17

[Valentin Gosu [:valentin]]

(In reply to Henri Sivonen (:hsivonen) from comment #9)

Quota manager expects nsStandardURL host names to be usable as file names on Windows, so there's this assertion:
https://searchfox.org/mozilla-central/rev/891d104826fb0cfd5cbdd6128e2372ce62810028/caps/OriginAttributes.cpp#292
(See bug 1196371.)

I still need to fix this bit.

Comment 18

[Valentin Gosu [:valentin]]

Attachment: Bug 1815926 - Fix serialization of originAttributes for host that contains * r=#necko

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/38264aa9ca96
https://hg.mozilla.org/integration/autoland/rev/32304afd4381
URL host parser does not accept * or " r=necko-reviewers,hjones,jesup,extension-reviewers,robwu
https://github.com/mozilla-firefox/firefox/commit/d81c9d78e8d7
https://hg.mozilla.org/integration/autoland/rev/afbc979cf372
Fix serialization of originAttributes for host that contains * r=necko-reviewers,jesup,ckerschb

Comment 20

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/32304afd4381
https://hg.mozilla.org/mozilla-central/rev/afbc979cf372