1922062 - make nsStandardURL::GetDisplayHostPort() always return "" for file URLs

Status: RESOLVED WONTFIX

(Core :: Networking, task, P2)

Description

[edgul]

Several functions already handle that fine (they build a display string from a scheme + "//" + displayHostPort.).
Otherwise, files with hosts will start showing up in new places. Internally, nsStandardURL still needs to keep mDisplayHost for file URLs because GetDisplaySpec and GetDisplayPrePath need to be correct.
Or maybe the "display" PrePath for file should be "file://" ?

nsGlobalWindowOuter::MakeMessageWithPrincipal() uses uri->GetDisplayHostPort() and would be fixed if that returns "" for file. Otherwise we'll have to special-case file URLs in this method itself.

Comment 1

[edgul]

I'm thinking we should just add mScheme check for file here instead of waiting for Bug 1922064 to implement similar just in case this code appends something to the empty string.

Still not sure about this part though:

Or maybe the "display" PrePath for file should be "file://" ?

Comment 2

[Valentin Gosu [:valentin] (he/him)]

nsGlobalWindowOuter::MakeMessageWithPrincipal() uses uri->GetDisplayHostPort()

I am in favor of special-casing these, but I'm not 100% convinced we need to if we don't allow opening channels to file URLs with a host.

Comment 3

[edgul]

If we choose to special-case for MakeMessageWithPrincipal, do we also need to cover Navigator::RegisterProtocolHandler? If not, why not?

We also have some jar code that uses it, but I'm guessing only jar schemes will use this.

URL.webidl doesn't provide access to this function, so we only need to worry about xpcom, which we are exposed to, though only fuzzing tests seem to use. For future uses of this function, I suppose we just make a strongly worded comment about how to use?

Comment 4

[Valentin Gosu [:valentin] (he/him)]

(In reply to edgul from comment #3)

If we choose to special-case for MakeMessageWithPrincipal, do we also need to cover Navigator::RegisterProtocolHandler? If not, why not?

Probably not, because you're not allowed to call registerProtocolHandler for the file scheme.

Comment 5

[edgul]

It seems like a file: url could trigger MakeMessageWithPrincipal to call GetDisplayHostPort if it contained javascript with alert(). That is unless the Principal is Null or there is an add-on policy?

https://searchfox.org/mozilla-central/rev/fb8a1b0beee7dbc1b0a35050ec5c0345f0b09173/dom/base/nsGlobalWindowOuter.cpp#4570,4575,4581,4584

Comment 6

[edgul]

Bump on comment 5.

Also, what exactly do we mean by special-casing on MakeMessageWithPrincipal?

Comment 7

[Valentin Gosu [:valentin] (he/him)]

Not sure what I meant by special-casing.
The gist is that if we don't allow opening file channels with a host, then MakeMessageWithPrincipal should never be called with a principal URL that contains file://host. Hoever, even if it would, the at worst we show an alert dialog with the file's host - which seems benign to me.
Let's close as wontfix. GetDisplayHostPort should always return the correct host and port.