1643789 - Shouldn't use alternate fixup for links placed in-content when middle-clicking / opening links in new tabs

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P2)

Description

Attachment: firefoxbug.png

1. Browse https://4programmers.net/Forum/Spolecznosc/Projekty/294934-billy_bomber_moja_pierwsza_gra_w_sklepie_play_wnioski
2. Move cursor to "filmik promocyjny" post in first (appears to be https//www.youtube.com/watch?v=lgmWdObqh3Q) (attachment)
3. Check bottom bar
4. Click
5. Instead of youtube it actually send user to http://www.https.com//www.youtube.com/watch?v=lgmWdObqh3Q

HTML: <a href="http://https//www.youtube.com/watch?v=lgmWdObqh3Q">filmik promocyjny</a>

Windows 10

---

Firefox info:
plug-ins:
-privacy badger
-Facebook container

Copy data:

[Gijs' edit: moved to attachment 9156344 [details]]

Comment 1

[Mikołaj]

I'm sorry. I've send data before update to current version.
This is data after update, bug still reproduce.

[Gijs' edit: moved to attachment 9156345 [details]]

Comment 2

[:Gijs (he/him)]

The URL tooltip is the same in Chrome. Fundamentally, this is an issue in the web page, not in URL parsing. The web page should not omit the ":" between "https" and "//" from the link.

When I simply left-click the link in a private Firefox window (tested with Firefox 78 beta), and in Chrome, I get a network error page, not the site you mention.

The only way I can reproduce correcting https to www.https.com is when I middle-click to open the link in a new tab, or use the context menu.

Marco, would it make sense to stop passing the flag to try "alternate" fixup (ie adding the www/.com) for those cases?

Comment 3

[Marco Bonardo [:mak] (away 22-26 Sept)]

What's the security concern here? The tooltip looks correct, confusing because noticing the missing : is hard, nonetheless.

(In reply to :Gijs (he/him) from comment #2)

The only way I can reproduce correcting https to www.https.com is when I middle-click to open the link in a new tab, or use the context menu.

Marco, would it make sense to stop passing the flag to try "alternate" fixup (ie adding the www/.com) for those cases?

What are those cases? newtab&context, or you mean https -> https.com?
I think we should only try alternate when the source is the urlbar, not when the url comes from anywhere else. Not sure how to detect that, though.

Comment 4

[:Gijs (he/him)]

(In reply to Marco Bonardo [:mak] from comment #3)

What's the security concern here? The tooltip looks correct, confusing because noticing the missing : is hard, nonetheless.

I agree, but also, I don't really care about the tooltip anyway, an adversary that can run JS can make it say more or less whatever they like.

The only reason I haven't unhidden this is because it seems surprising to me that we behave differently for the left-mouse link click vs. context menu and middle-click. That seems like a bug to me, and it allows loading a "strange" website. But you're probably right that it's not security-sensitive...

(In reply to :Gijs (he/him) from comment #2)

The only way I can reproduce correcting https to www.https.com is when I middle-click to open the link in a new tab, or use the context menu.

Marco, would it make sense to stop passing the flag to try "alternate" fixup (ie adding the www/.com) for those cases?

What are those cases? newtab&context, or you mean https -> https.com?

middle-click + context menu. Though perhaps specialcasing the protocol strings themselves would not be a bad idea either...

I think we should only try alternate when the source is the urlbar, not when the url comes from anywhere else. Not sure how to detect that, though.

I thought it may be enough to not pass the flag "somewhere" in the path that we're taking for the context menu / middle-click. If that's somehow complicated, we could check for a system principal on the loadinfo, if we have access to that.

Comment 5

[Daniel Veditz [:dveditz]]

In your initial comment you say "click" in step 3 -- did you actually middle-click or Control-click to open it in a new tab? That's the only way we can reproduce this.

Comment 6

[Mikołaj]

I always use the middle-click to open hyperlinks.
I'm sure it was middle and checked it just now once again.

Comment 7

[:Gijs (he/him)]

Attachment: about:support data from comment 0

Comment 8

[:Gijs (he/him)]

Attachment: about:support data after the update

Comment 11

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #4)

(In reply to Marco Bonardo [:mak] from comment #3)

I think we should only try alternate when the source is the urlbar, not when the url comes from anywhere else. Not sure how to detect that, though.

I thought it may be enough to not pass the flag "somewhere" in the path that we're taking for the context menu / middle-click. If that's somehow complicated, we could check for a system principal on the loadinfo, if we have access to that.

I looked into this a bit; the current "alternate" logic in docshell restricts alternate fixes to "normal" loads, which is why left-clicking on links doesn't have this issue, but middle-clicks do.

But emulating this in the frontend for newly opened tabs, newly opened windows, etc., is tedious and requires funnelling yet more parameters through addTab, openLinkIn and all their friends (we don't currently have a single parameter for the load flags that ultimately are passed to the browsing context or webnavigation's loadURI method).

It seems like a much simpler restriction that would work would be to additionally require the triggering principal of the load to be system, inside the docshell code at AttemptURIFixup. Then we'd keep it for URL bar loads and things like that, as Marco suggested, but not for any of these loads (as openLinkIn and so on already pass triggering principals).

Comment 12

[:Gijs (he/him)]

Attachment: Bug 1643789 - fix use of alternate URI fixup for middle clicks, context menu clicks, etc., r?nika

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/babdc3b3a300
fix use of alternate URI fixup for middle clicks, context menu clicks, etc., r=nika

Comment 14

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/babdc3b3a300