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)
Firefox
General
Tracking
()
Tracking | Status | |
---|---|---|
firefox46 | --- | verified |
People
(Reporter: markh, Assigned: markh)
References
Details
(Whiteboard: [fxprivacy])
Attachments
(1 file)
(deleted),
patch
|
rbarnes
:
review+
rbarnes
:
feedback+
|
Details | Diff | Splinter Review |
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.
Updated•9 years ago
|
Whiteboard: [fxprivacy] [triage]
Assignee | ||
Comment 1•9 years ago
|
||
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•9 years ago
|
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Comment 2•9 years ago
|
||
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•9 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•9 years ago
|
Assignee: nobody → markh
Comment 4•9 years ago
|
||
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+
Assignee | ||
Comment 5•9 years ago
|
||
Comment 7•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Updated•9 years ago
|
Iteration: --- → 46.2 - Jan 11
Priority: -- → P1
Updated•9 years ago
|
Flags: qe-verify+
QA Contact: paul.silaghi
You need to log in
before you can comment on or make changes to this bug.
Description
•