1319533 - Location bar does not trim on paste

Status: RESOLVED DUPLICATE of bug 1460097

(Firefox :: Address Bar, defect, P2)

Description

[WBT]

Copying a URL from an e-mail, such as "https://bugzilla.mozilla.org/page.cgi?id=persona_deprecated.html " and pasting it in the address bar, results in a 404 error because the whitespace is included in the request.  
The expected result is that Firefox will trim leading and trailing whitespace before making the request.  
The workaround is to realize that this is the issue instead of assuming it's a bad link, and manually trim that whitespace.

Comment 1

[Marco Bonardo [:mak]]

Both Chrome and Edge seem to do what you suggest, so it sounds like something users are used to, and also useful.

Comment 2

[Jonathan Watt [:jwatt]]

Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)

Comment 3

[Harry Twyford]

I'm working on this one. As it stands, my fix allows a user to paste a URL containing a trailing space as above. The browser will strip the space before loading the URL, thus fixing the bug. 

However, I have a question about autocomplete behaviour. Pasting the URL from the initial bug into the URL bar shows that the space at the end isn't a "space bar space", but an em space (U+2003). `%2003` is put in the autocomplete entry to show this. This behaviour is in the current version on Firefox; it's not new to my fix. I'll attach a screenshot.

I was looking into this a little more and found this is intended behaviour dating back to 2005. It's an anti-phishing practice to escape potential phishing characters with their punycode versions. It's addressed on this page complete with Bugzilla bug: https://www.mozilla.org/en-US/security/advisories/mfsa2005-29/

My question: should the fix here continue to show `%2003` in the autocomplete result? Currently, Firefox escapes every IDN phishing character with its punycode with the exception of the regular space. It's common that a user might accidentally type a space in a URL, so showing a punycode in autocomplete would probably just confuse. However, it's much less common that an em space (U+2003) would just "wind up" in a URL and so its a nice subtle indicator to the user that there's something suspicious in the URL.

Personally, I think showing the `%2003` (or any of the other space-derived IDN phishing punycodes) in autocomplete but still stripping the space out when the URL is actually loaded is the ideal solution. I wanted to get others' thoughts before submitting to review, though.

Comment 4

[Harry Twyford]

Attachment: Autocomplete behaviour

Comment 5

[Marco Bonardo [:mak]]

let's dupe this to bug 1460097 that has a bit more discussion. You can obviously continue your work there, no problem with that.

(In reply to Harry Twyford [:harry] from comment #3)
> However, I have a question about autocomplete behaviour. Pasting the URL
> from the initial bug into the URL bar shows that the space at the end isn't
> a "space bar space", but an em space (U+2003). `%2003` is put in the
> autocomplete entry to show this. This behaviour is in the current version on
> Firefox; it's not new to my fix. I'll attach a screenshot.

We must change autocomplete to use the trimmed string rather than the original string.
It should only trim, shouldn't do any other encoding/decoding.
There's no big risk of phishing here since the locationbar loads the trimmed url, and the popup would show the same.
Most of the logic is in unifiedcomplete.js, in the _matchUnknownUrl method, where we use the original string instead of the trimmed string. I didn't check details but likely some of that should be changed.

That should already cover most cases where we search through autocomplete. Additionally, there should also be a fix here for the Paste&Go case:
https://searchfox.org/mozilla-central/source/browser/base/content/urlbarBindings.xml#794

Comment 6

[Harry Twyford]

Attachment: Bug 1319533 - Autocompleted URLs are trimmed.

Review commit: https://reviewboard.mozilla.org/r/245880/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245880/

Comment 7

[Marco Bonardo [:mak]]

please move the patch to bug 1460097.

Comment 8

[Harry Twyford]

Moved