Kai Engert (:KaiE:) Assignee
	Description • 23 years ago

This is a constructed test scenario I'm using to test my code.

Prepare a web page B, that gives out a HTTP code 302 and redirects to a new
document location https://C

Prepare a page A that has a link to http://B

Go to https://A
When you click the link to B, it will load a http page, and immediately redirect
to https://C

Expected behaviour: Because the intermediate loading of a http page could be
used to transfer data unencrypted, the browser should show warnings. A warning
should first be shown that an unencrypted web site is loaded. Then another
warning should be shown, that a secure web site is loaded.

Actual behaviour: No warning is shown.

	Kai Engert (:KaiE:) Assignee
	Comment 1 • 23 years ago

A test case is available at
  https://www.kuix.de/misc/test26/redirect302.php

	Paul Wyskoczka
	Comment 4 • 23 years ago

Adding [adt2 rtm] and nsbeta1+

	Kai Engert (:KaiE:) Assignee
	Comment 5 • 23 years ago

This patch fixes the problem.

When I originally rewrote the lock icon behaviour with bug 130949, I did not
test the redirect at the protocol level. This problem has been in since 130949
landed.

Luckily, now that bug 148610 has landed, it is possible to remove some code
from the tracking code, that is no longer needed.

When I wrote 130949, I tried to stick with the "wait with updating until the
request is finished" scheme.

But now, we are updating the lock icon as early as possible. Therefore, it is
ok to check the lock icon state each time a top level document finishes
loading. A top level document does finish loading when doing 302 redirects.

The attached patch simply removes a restriction to test only in some cases, and
makes us check more often. By doing so, we are now detecting the insecure
redirect correctly.

	Javier Delgadillo
	Comment 6 • 23 years ago

Comment on attachment 89167 [details] [diff] [review]
Patch v2

r=javi looks good.

	Alec Flett
	Comment 7 • 23 years ago

Comment on attachment 89167 [details] [diff] [review]
Patch v2

sr=alecf

	Kai Engert (:KaiE:) Assignee
	Comment 8 • 23 years ago

Checked in to trunk around 7 am this morning.

Should be fixed in today's builds appearing after 8 am.

	John Unruh
	Comment 9 • 23 years ago

Verified fixed on the 9:30 AM 6/26 trunk. 
https://www.kuix.de/misc/test26/many.php

	scottputterman
	Comment 10 • 23 years ago

adding adt1.0.1+.  Please get drivers approval (cc'ing valeski) before checking
into the branch.

	Stephane Saux
	Comment 11 • 23 years ago

John,
  If you haven't done so already, can you run the full lock icon regression
testing with this additional patch?  We want to make sure that any work on the
lock icon is tested against both the original bug report, but also for any
additional regressions.
  If you have done so already, can you just state so?
thanks.

	John Unruh
	Comment 12 • 23 years ago

I did full lock icon testing with the trunk build, and all is as expected. The 
patch should be checked into the branch.

	Judson Valeski
	Comment 13 • 23 years ago

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

	Stephane Saux
	Comment 14 • 23 years ago

Kai will check this in tomorrow.

	Kai Engert (:KaiE:) Assignee
	Comment 15 • 23 years ago

Checked in to branch.

	Kai Engert (:KaiE:) Assignee
	Comment 16 • 23 years ago

It's very good that we fixed this bug.
Somebody @netscape.com just pinged me and showed me a site where this
redirection is indeed used:

http://www.tropilab.com/checkout2.html

And click on the "Click here" link. https link redirecting to http.
The now shown page claims it is on secure site, although it is not!

Luckily, we behave correctly on that site.

	John Unruh
	Comment 17 • 23 years ago

Verified on branch.