1273255 - Redirected loads should not keep using uri fixup to "fix" URIs provided by webpages

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[:Gijs (he/him)]

STR:

1. open a page that does a 302 to "http://mozilla"

(for now: http://gijsk.com/temp/location-header.php )

ER: 'mozilla' loads, if defined on your local network, or an error page if it isn't.

AR: www.mozilla.org loads

Comment 1

[Boris Zbarsky [:bzbarsky]]

The right way to fix this is to just check for the nsIChannel::LOAD_REPLACE flag on the channel and skip the fixup.

Gijs, how easy is it to write a test for this, do you think?  I can write the code pretty easily, but don't have a day or so to figure out the test....

Comment 2

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bz] from comment #1)
> The right way to fix this is to just check for the nsIChannel::LOAD_REPLACE
> flag on the channel and skip the fixup.
> 
> Gijs, how easy is it to write a test for this, do you think?  I can write
> the code pretty easily, but don't have a day or so to figure out the test....

I think DNS lookups from tests are OK, but connections are not, so it should be a question of having a plain mochitest that sticks an sjs file in a frame that does a redirect to e.g. http://mozilla and verifying that an error page gets loaded. If we contact www.mozilla.org the browser will exit. A slightly less harsh method would be using http://example and checking if www.example.com gets loaded instead (which is redirected to the mochitest server in tests).

Would that work or am I missing something? I can see if I can write the test if necessary, but it seems like with this recipe it shouldn't take you a day? Maybe I overestimate your familiarity with mochitests or my assumptions about the test are broken?

Comment 3

[Boris Zbarsky [:bzbarsky]]

We don't do fixup for frames, so sticking something in a frame would not work.

We could try window.open(), but arguably that should not fixup at all.

So this needs to be a browser test that emulates what happens when the user types in the URL bar, I think.  And the problem is my lack of familiarity with browser tests...

Comment 4

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bz] from comment #3)
> We don't do fixup for frames, so sticking something in a frame would not
> work.
> 
> We could try window.open(), but arguably that should not fixup at all.
> 
> So this needs to be a browser test that emulates what happens when the user
> types in the URL bar, I think.  And the problem is my lack of familiarity
> with browser tests...

Ah, OK. Yeah, I can help. In a meeting right now, I'll try to whip something up in an hour or so.

Comment 5

[:Gijs (he/him)]

Attachment: test for url rewriting on redirects

This seems to work in my testing. It times out right now, but it works if it loads a real error page. It's a bit annoying because error page loads are special, but I don't really have a better idea about how to simulate this without going through the faff of registering 2 separate test domains in the mochitest test server. I guess we could use a domain we know doesn't exist... that should be a pretty trivial change to this patch. Boris, let me know if there's issues with this.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch with the fix in addition to the test

Comment 7

[:Gijs (he/him)]

Did you intend to ask for review? :-)

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8754631 [details] [diff] [review]
Patch with the fix in addition to the test

I wasn't quite sure what to do with it...

Olli, please feel free to tell me to go pick on someone else?

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc6334aea116

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bc6334aea116