Last Comment Bug 781617 - http is given from History even when https is explicitly typed in address bar
: http is given from History even when https is explicitly typed in address bar
Status: VERIFIED FIXED
: regression, reproducible, sec-want
Product: Firefox
Classification: Client Software
Component: Location Bar (show other bugs)
: 14 Branch
: x86 All
: -- normal with 3 votes (vote)
: Firefox 18
Assigned To: Marco Bonardo [::mak]
:
Mentors:
: 792969 801247 806143 806323 807292 (view as bug list)
Depends on:
Blocks: 720081 769348
  Show dependency treegraph
 
Reported: 2012-08-09 12:35 PDT by Benno Schulenberg
Modified: 2012-11-27 12:54 PST (History)
26 users (show)
mak77: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
wontfix
+
wontfix
+
verified


Attachments
patch v1.0 (3.92 KB, patch)
2012-09-20 10:07 PDT, Marco Bonardo [::mak]
bmcbride: review+
gavin.sharp: approval‑mozilla‑aurora+
Details | Diff | Review

Description Benno Schulenberg 2012-08-09 12:35:16 PDT
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:14.0) Gecko/20100101 Firefox/14.0.1
Build ID: 20120713225625

Steps to reproduce:

I typed an explicit https URL in the address bar.


Actual results:

FF showed the page using http instead.  (The page was visited many days before using http, and apparently FF recognizes the typed URL as equivalent to the one in the history, ignoring the all-important https component.)

For example, visit:  http://de.wiktionary.org/wiki/Augenbraue
Then type  https://  at the start of the address bar en hit Enter.
No go.  The https part is discarded.

(Clicking a link works fine, of course: https://de.wiktionary.org/wiki/Augenbraue)


Expected results:

FF should honour the typed URL, consider http and https as fundamentally different, and refetch the requested page using https.
Comment 1 Alice0775 White 2012-08-09 12:54:22 PDT
Regression window
Good:
http://hg.mozilla.org/mozilla-central/rev/0e4f8e1a141b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120603134204
Bad:
http://hg.mozilla.org/mozilla-central/rev/dd6ec482a85d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120603154604
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0e4f8e1a141b&tochange=dd6ec482a85d

Regressed by
dd6ec482a85d	Marco Bonardo — Bug 720081 - Part 1: backportable solution for autocomplete controller to provide a different final defaultComplete value for typeAheadResults r=gavin
Comment 2 Loic 2012-08-10 10:02:56 PDT
It's already reported here https://bugzilla.mozilla.org/show_bug.cgi?id=769994#c5 in scenario #2. And this comment from Marco Bonardo too: https://bugzilla.mozilla.org/show_bug.cgi?id=769994#c8
Comment 3 Lukas Blakk [:lsblakk] use ?needinfo 2012-08-13 16:46:52 PDT

*** This bug has been marked as a duplicate of bug 769994 ***
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-08-14 08:13:14 PDT
Not a dupe - this bug is the opposite of bug 769994, per the summary. IIRC, preferring https is intentional, and we should never override that with "http".
Comment 5 Lukas Blakk [:lsblakk] use ?needinfo 2012-08-14 09:53:55 PDT
Marco can you look into backing out bug 720081 in that case so we don't ship 15 with this regression?
Comment 6 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-08-14 12:36:34 PDT
We can't back out the patch, it's critical to the functionality of inline autocomplete. The "regression" shipped in 14, and doesn't seem to be affecting all cases, so if it comes down to it we can ship with this again, but we should definitely make an effort to investigate and correct it.
Comment 7 Marco Bonardo [::mak] 2012-08-17 07:59:25 PDT
I tried the steps in comment 0, but I cannot reproduce, it goes to the https version for me. Can anyone else confirm steps?
Comment 8 Loic 2012-08-17 08:07:35 PDT
(In reply to Marco Bonardo [:mak] from comment #7)
> I tried the steps in comment 0, but I cannot reproduce, it goes to the https
> version for me. Can anyone else confirm steps?

See https://bugzilla.mozilla.org/show_bug.cgi?id=781617#c2
I think it's the same scenario as this bug.
Comment 9 Lukas Blakk [:lsblakk] use ?needinfo 2012-08-20 16:48:38 PDT
At this point in FF15 cycle we must wontfix for 15 but please keep this as a priority for 16.
Comment 10 [:Aleksej] 2012-08-21 03:09:05 PDT
*** Bug 784248 has been marked as a duplicate of this bug. ***
Comment 11 Rainer Rillke 2012-09-11 08:06:04 PDT
When prefixing https in address bar or changing https to http, I still remain at the same protocol that I used before. 
Steps:
* Enter http://commons.wikimedia.org/wiki/Special:Contributions/Republicoat with C&Past in address bar
* Press enter key. "http://" disappears.
* Wait until page is loaded
* Clicking before the first char in the address bar in order to prepend "https://". Enter "https://" using the keyboard.
* Clicking enter
* Firefox, again loads the page, but *not using HTTPS*, but instead *HTTP*

Workaround:
* Add or modify a query-parameter so the URL is a different one

Expected behaviour:
* Allow me switching to HTTPS
Comment 12 Marco Bonardo [::mak] 2012-09-19 13:13:06 PDT
(In reply to Rainer Rillke from comment #11)
> * Clicking before the first char in the address bar in order to prepend
> "https://". Enter "https://" using the keyboard.
> * Clicking enter

hm this is strange cause inline ac is active only when the mouse cursor is at the end :(
Btw, looking into this.
Comment 13 Marco Bonardo [::mak] 2012-09-19 13:51:37 PDT
so, looks like there is a missing condition for typeAhead results in the controller, the suggested string should always contain the input value.
The problem comes from the fact you search for
"https://something", the code strips the prefix and searches for "something", it finds "http://something" and suggests it, at this point we lack the check.
So this could either be fixed:
- in the controller, this would be sort of a firewall for broken searches
- in the search, makes sense that a search can do whatever on the input string but should not suggest nonsense results

Or just in both places, I'd probably do the latter but put an aborting assert in the controller...
Comment 14 Marco Bonardo [::mak] 2012-09-20 10:07:19 PDT
Created attachment 663058 [details] [diff] [review]
patch v1.0

Blair, if you're too busy please let me know, since this is tracked and I'm late already :)
Comment 15 Alex Keybl [:akeybl] 2012-09-20 15:50:25 PDT
FF15 was affected, so given where we are in the cycle, we're going to leave this unfixed. Please nominate for FF17 Aurora approval when ready.
Comment 16 Loic 2012-09-20 16:17:27 PDT
*** Bug 792969 has been marked as a duplicate of this bug. ***
Comment 17 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2012-09-21 04:34:29 PDT
Comment on attachment 663058 [details] [diff] [review]
patch v1.0

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

Seems reasonable :)

::: toolkit/components/places/nsPlacesAutoComplete.js
@@ +1521,5 @@
>  
>      // Add the result.
> +    // If the untrimmed value doesn't preserve the user's input just
> +    // ignore it and complete to the found url.
> +    let untrimmedURL = prefix + url;    

Nit: trailing whitespace.
Comment 18 Marco Bonardo [::mak] 2012-09-21 07:58:01 PDT
thank you, I had fixed the trailing spaces locally already.
https://hg.mozilla.org/integration/mozilla-inbound/rev/9efb8c43ee8a
Comment 19 Ryan VanderMeulen [:RyanVM] 2012-09-21 19:56:19 PDT
https://hg.mozilla.org/mozilla-central/rev/9efb8c43ee8a
Comment 20 Marco Bonardo [::mak] 2012-09-24 06:59:03 PDT
Comment on attachment 663058 [details] [diff] [review]
patch v1.0

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 720081
User impact if declined: we are not respecting what the user types, so he may end up to the unwanted page
Testing completed (on m-c, etc.): m-c, automated test
Risk to taking this patch (and alternatives if risky): regressions in autoFill, but it's quite well covered by tests, and the change is quite contained
String or UUID changes made by this patch: none
Comment 22 Wout de Zeeuw 2012-10-03 03:45:05 PDT
ARGH! This just cost me half a day try to figure out whether I had done something wrong in my new ASP.NET MVC application. Broken in v15 indeed, very very inconvenient! Sorry, I just had to complain.
Comment 23 Matthias Versen [:Matti] 2012-10-13 05:35:45 PDT
*** Bug 801247 has been marked as a duplicate of this bug. ***
Comment 24 Matthias Versen [:Matti] 2012-10-27 18:25:51 PDT
*** Bug 806143 has been marked as a duplicate of this bug. ***
Comment 25 Petr Kadlec 2012-10-29 03:37:14 PDT
*** Bug 806323 has been marked as a duplicate of this bug. ***
Comment 26 Marco Bonardo [::mak] 2012-10-31 06:43:58 PDT
*** Bug 807292 has been marked as a duplicate of this bug. ***
Comment 27 Ioana (away) 2012-11-15 07:24:30 PST
Verified as fixed on Firefox 17 beta 6 - 20121113065533.

I wasn't redirected to http for any pages in the above comments, nor other web pages.
Comment 28 Benno Schulenberg 2012-11-27 12:54:20 PST
Verified as fixed in FF 17.

Note You need to log in before you can comment on or make changes to this bug.