380912 - (flock7662) "Get me out of here" link doesn't handle pipe-delimited home page

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Matthew (lilmatt) Willis]

Attachment: If uri has a pipe, only load the first page

Upstream fix from Flock:

The "Get me out of here!" link always tries to load the default home page URL using loadURI(), without regard for the fact that it could have been a pipe-delimited set of tabs.

See also:
https://bugzilla.flock.com/show_bug.cgi?id=7662

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 265029 [details] [diff] [review]
If uri has a pipe, only load the first page

>Index: phishing-afterload-displayer.js

>+    // In case 'url' is a pipe-delimited set of pages, just take the first one
>+    if (url.indexOf("|") != -1) {
>+      url = url.split("|")[0];
>+    }

Remove the unneeded parentheses and add a comment pointing out that this needs to be revisited if ever bug 221445 is fixed (to help possible fixers of that bug find places they need to change, if nothing else), and r=me. I kinda wish there was an easy to notify people changing default homepages of the potential issue here if they were to add a URL that had pipes in it for reasons other than to indicate multiple home pages, but a note in firefox.js probably wouldn't help, and I think that's probably a pretty unlikely scenario.

Comment 2

[Phil Ringnalda (:philor)]

Same privacy issue that made bug 343999 a branch blocker.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #2)
> Same privacy issue that made bug 343999 a branch blocker.

Not quite, since this only applies to default home page values (we don't use the user-specified homepage value for this dialog), and can't easily be triggered from content (unlike window.home())

Comment 4

[Phil Ringnalda (:philor)]

Some days, I can manage to imitate someone who knows what they're saying. Not today, apparently.

Comment 5

[Matthew (lilmatt) Willis]

Comment on attachment 265029 [details] [diff] [review]
If uri has a pipe, only load the first page

requesting a1815

Comment 6

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #1)
> Remove the unneeded parentheses

And of course by "parentheses", I meant "braces" (oops).

Comment 7

[Matthew (lilmatt) Willis]

Attachment: rev1 - patch as checked in on trunk

Carrying forward gavin's r.

Patch as checked in on trunk.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 265815 [details] [diff] [review]
rev1 - patch as checked in on trunk

approved for 1.8.1.5, a=dveditz for release-drivers

Comment 9

[Matthew (lilmatt) Willis]

Patch checked in on MOZILLA_1_8_BRANCH

-> fixed1.8.1.5

Comment 10

[Adam Guthrie]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.5pre) Gecko/20070709 BonEcho/2.0.0.5pre

So, on branch http://www.mozilla.com/en-US/firefox/ is always loaded when I click "Get me out of here" regardless of my homepage setting. I didn't feel like digging through the safe-browsing code to figure out why this is.

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #10)
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.5pre) Gecko/20070709
> BonEcho/2.0.0.5pre
> 
> So, on branch http://www.mozilla.com/en-US/firefox/ is always loaded when I
> click "Get me out of here" regardless of my homepage setting. I didn't feel
> like digging through the safe-browsing code to figure out why this is.

Isn't that covered by https://bugzilla.mozilla.org/show_bug.cgi?id=380912#c3 ? 

Comment 12

[Adam Guthrie]

Well, it sure looks like it's using the user-specified homepage preference: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/safebrowsing/content/phishing-afterload-displayer.js&rev=1.1.2.14&mark=173,174#161.
"browser.startup.homepage" is what contained the URLs that I inputed in the preferences.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

It always uses the default home page intentionally, see bug 339032.