Control centre may crop the important part of nsSimpleURIs (eg, about: URLs)

VERIFIED FIXED in Firefox 46

Status

()

Firefox
General
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: markh, Assigned: markh)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 46
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox46 verified)

Details

(Whiteboard: [fxprivacy])

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
If you sign in to Sync, you end up on a page with URL about:accounts?action=signup&entrypoint=menupanel. If you view the control centre for that page, you get https://bugzilla.mozilla.org/attachment.cgi?id=8683343 - ie, the url is shown as "...on=signup&entrypoint=menupanel" - so the important part of the URL (ie, the about:accounts part) has been cropped.

MattN said this might be a regression from bug 1204616. A quick look at the code shows that because we have a simpleURI, the getEffectiveHost() call in browser.js will return an emptry string (as uri.host throws for simple URIs), so the code then falls back to uri.specIgnoringRef, which returns the entire URL.

I guess we either want to treat about: URIs as a special case and manually trim the query portion, or at least get it to crop the end rather than the start.
Whiteboard: [fxprivacy] [triage]
(Assignee)

Comment 1

2 years ago
Created attachment 8683399 [details] [diff] [review]
0001-Bug-1221808-have-the-control-centre-crop-the-end-for.patch

Hi Richard,
  This problem seems to be related to bug 1204616, hence I'm asking you for some advice.

The simplest fix is as attached - it arranges to crop the end of the URL when the URI has no host. Another option might be to try and remove the querystring, but (a) this means doing it manually on the string representation of the URI as SimpleURIs don't know about query strings, and (b) about:averylongaboutstring might still trim the "about:" portion if we did that.

Thoughts?
Attachment #8683399 - Flags: feedback?(rlb)

Updated

2 years ago
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Comment on attachment 8683399 [details] [diff] [review]
0001-Bug-1221808-have-the-control-centre-crop-the-end-for.patch

Review of attachment 8683399 [details] [diff] [review]:
-----------------------------------------------------------------

This seems like it's probably OK.  IIRC, the risks we were trying to address in Bug 1204616 were related to an attacker using a name like "facebook.com.alotofextrapaddingtogetcropped.nefarious.com" and that getting displayed as "facebook.com..." instead of "...nefarious.com".

If there's no host, then it seems like you're not going to be in a case where you have that kind of spoofing risk, so it's better to start at the beginning.  In fact, it seems kind of nice to do that, because it highlights the unusual URL scheme.
Attachment #8683399 - Flags: feedback?(rlb) → feedback+
(Assignee)

Comment 3

2 years ago
Comment on attachment 8683399 [details] [diff] [review]
0001-Bug-1221808-have-the-control-centre-crop-the-end-for.patch

Thanks for the comment. It appears there are no tests for this, so best I can see, this patch is all that is necessary?
Attachment #8683399 - Flags: review?(rlb)
(Assignee)

Updated

2 years ago
Assignee: nobody → markh
Comment on attachment 8683399 [details] [diff] [review]
0001-Bug-1221808-have-the-control-centre-crop-the-end-for.patch

Review of attachment 8683399 [details] [diff] [review]:
-----------------------------------------------------------------

Upgrading f+ to r+
Attachment #8683399 - Flags: review?(rlb) → review+

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/83cb1c2445fa
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46

Updated

2 years ago
Iteration: --- → 46.2 - Jan 11
Priority: -- → P1
Flags: qe-verify+
QA Contact: paul.silaghi
Verified fixed FF 46.0a1 (2016-01-04) Win 7
Status: RESOLVED → VERIFIED
status-firefox46: fixed → verified

Updated

2 years ago
Blocks: 1216897
You need to log in before you can comment on or make changes to this bug.