Closed Bug 1221808 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox :: General, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 46
Iteration:
46.2 - Jan 11
Tracking Status
firefox46 --- verified

People

(Reporter: markh, Assigned: markh)

References

Details

(Whiteboard: [fxprivacy])

Attachments

(1 file)

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]
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)
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+
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: 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+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
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
Blocks: 1216897
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: