1196882 - Firefox not following 303/7 Status Code after 302

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Benny M]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.155 Safari/537.36

Steps to reproduce:

I'm a developer with spideroak.com. The OMVA we have built for our SMB and Enterprise customers has an option to `View User's Data` from a user's detail page.

The process for linking to the user's data follows a 4 step process wherein the page is directed through a couple of channels, landing at the user's url:
- Click the button. This kicks off the process of grabbing user's validation through an escrow process.
- The process returns a 302 redirect with credentials to our internal processor.
- The internal processor validates and returns a 303 with the supplied destination url.
- Manager can then view user's data.

Due to certain policies, I cannot provide a reduced test case. However, I can direct you to a secured a minimal as possible version of our product upon request for reproduction.


Actual results:

From the 303, the final supplied url should load. But Firefox spins for a while then just times out.


Expected results:

Firefox should follow 303 to the final destination.

Note: This works in Firefox 38.0.5 but causes the above issue in 39.0.3 to Nightly 43.0a1

Comment 1

[Loic]

Maybe there is a regression in FF39. A dev will probably ask you an access to the testcase.

Comment 2

[Patrick McManus [:mcmanus]]

please supply an http log of the test case. When you upload the attachment you can mark it private.

https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging

Comment 3

[Benny M]

(In reply to Patrick McManus [:mcmanus] from comment #2)
> please supply an http log of the test case. When you upload the attachment
> you can mark it private.
> 
> https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging

The log files generated from this exceed the upload limit. I can attach them offsite in a spideroak.com ShareRoom. Alternatively can I trim the logs at a certain point?
Currently the log generated from the 38.0.5 test case is 37.2MB and the one from 40.0.2 is 51.1MB

Also - I didn't see an option in the Create New Attachment view for marking the files as private.

Comment 4

[Loic]

You can upload the log file to a third-party cloud service like Dropbox, Mediafire etc. and post the download link here.

Comment 5

[Patrick McManus [:mcmanus]]

on the upload screen there is a checkbox under privacy.. but if you're just going to include a link in a comment (which is fine) then you want the "make comment private" checkbox next to "additional comments"

Comment 6

[Patrick McManus [:mcmanus]]

oh - and those logs are text and compress very well..

Comment 7

[Benny M]

(In reply to Patrick McManus [:mcmanus] from comment #6)
> oh - and those logs are text and compress very well..

Here you go. This ShareRoom will be available till the conclusion of this ticket.

https://spideroak.com/browse/share/Rustic/firefox-logs

Comment 8

[Patrick McManus [:mcmanus]]

if you find the 303 response in the log (see other is a good string to search for) it has a content-length of 246 but there are no body bytes sent after the headers and the server closes the connection. We detect that, correctly, as a malformed response. (and this is a new behavior)

The fastest fix for you would be to make it well formed.. (c-l 0, or send a body, or just omit c-l as long as you close the connection)

Given that this is a redirect and the response body is not  all that important, I think we should also consider the firefox behavior an overreaction and let this slide on non 2xx cases for webcompat reasons.

I'll attach a patch soon

bagder ni just fyi

Comment 9

[Daniel Stenberg [:bagder]]

Yeah, seeing that there are now more than one case of this and they're rather obscure to the users I can see how we might need to backpedal once again. Do you plan on loosening the check for redirects in particular or do it for all non-2xx status responses?

Comment 10

[Patrick McManus [:mcmanus]]

(In reply to Daniel Stenberg [:bagder] from comment #9)
> Yeah, seeing that there are now more than one case of this and they're
> rather obscure to the users I can see how we might need to backpedal once
> again. Do you plan on loosening the check for redirects in particular or do
> it for all non-2xx status responses?

can you include other bug numbers here?

Comment 11

[Patrick McManus [:mcmanus]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=50fd184e6bc1

Comment 12

[Patrick McManus [:mcmanus]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ed45de3ad71e

Comment 13

[Daniel Stenberg [:bagder]]

I was thinking about bug 1186830 in addition to this.

Comment 15

[Patrick McManus [:mcmanus]]

https://tbpl.mozilla.org/?tree=Try&rev=a6aa4f93d812

Comment 16

[Patrick McManus [:mcmanus]]

Attachment: dont enforce h1 framing on non 2xx

Comment 17

[Daniel Stenberg [:bagder]]

Comment on attachment 8655007 [details] [diff] [review]
dont enforce h1 framing on non 2xx

Looking good!

Comment 18

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9620db5a619d

Comment 19

[Patrick McManus [:mcmanus]]

Comment on attachment 8655007 [details] [diff] [review]
dont enforce h1 framing on non 2xx

Approval Request Comment
[Feature/regressing bug #]:237623 - gecko 39
[User impact if declined]: minor webcompat issue with non compliant http responses can lead to failed navigations. We can loosen the rules a little bit with this patch and still obtain the goal of 237623.
[Describe test coverage new/current, TreeHerder]: new xpcshell test
[Risks and why]: extremely low risk - 237623 tightened handling of non-compliant responses, and this change brings us back to where we were for a small subset of them for reasons of web compat.
[String/UUID change made/needed]: none

Comment 20

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Benny, would you be able to verify the fix on Nightly build from 09-02 or later? I am hoping the fix will be in the next Nightly build. Thanks!

Comment 21

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Adding the keyword regression and setting status affected for 41+

Comment 22

[Benny M]

(In reply to Ritu Kothari (:ritu) from comment #20)
> Benny, would you be able to verify the fix on Nightly build from 09-02 or
> later? I am hoping the fix will be in the next Nightly build. Thanks!

Absolutely. I'll test as soon as the build comes out.

Comment 23

[Benny M]

FF Nightly 43.0a1 (2015-09-02) fails. Will try again tomorrow. Let me know if you need anything.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9620db5a619d

Comment 25

[Benny M]

Fix works as of Nightly 43.0a1 (2015-09-03)

Do you know when this will go to production?

Thanks!

Comment 26

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Benny M from comment #25)
> Fix works as of Nightly 43.0a1 (2015-09-03)
> 
> Do you know when this will go to production?
> 
> Thanks!

Many Thanks Benny! This should in 41.0b8 and 41 when it goes live.

Comment 27

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8655007 [details] [diff] [review]
dont enforce h1 framing on non 2xx

Given that the patch was verified, I feel much more confident uplifting to Aurora42 and Beta41.

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/422d0883290e

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/6ca4bc2bbbff

Comment 30

[Benny M]

Can confirm that this issue is resolved in 41.0!

Thanks a ton for all your work and your attention to this issue!

Comment 31

[Jean-Yves Perrier [:teoli]]

Usually we don't document bug fixes (unless very old). In this case, I think the site-compat info will be enough.

Comment 32

[Kohei Yoshino]

Added the site compatibility doc: https://www.fxsitecompat.com/en-US/docs/2015/303-and-307-redirects-after-302-are-not-followed/