Firefox not following 303/7 Status Code after 302

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: benny, Assigned: mcmanus)

Tracking

({dev-doc-complete, regression, site-compat})

39 Branch
mozilla43
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox41 fixed, firefox42 fixed, firefox43 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

4 years ago
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
Reporter

Updated

4 years ago
Component: Untriaged → Untriaged
OS: Unspecified → All
Product: Core → Firefox
Hardware: Unspecified → All

Updated

4 years ago
Component: Untriaged → Networking: HTTP
Product: Firefox → Core

Comment 1

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

Comment 2

4 years ago
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
Reporter

Comment 3

4 years ago
(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

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

Comment 5

4 years ago
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"
Assignee

Comment 6

4 years ago
oh - and those logs are text and compress very well..
Reporter

Comment 7

4 years ago
(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
Whiteboard: 1196237
Whiteboard: 1196237
Assignee

Comment 8

4 years ago
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
Flags: needinfo?(daniel)
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?
Flags: needinfo?(daniel)
Assignee

Comment 10

4 years ago
(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?
I was thinking about bug 1186830 in addition to this.
Assignee

Updated

4 years ago
Duplicate of this bug: 1186830
Assignee

Updated

4 years ago
Assignee: nobody → mcmanus
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8655007 [details] [diff] [review]
dont enforce h1 framing on non 2xx

Looking good!
Attachment #8655007 - Flags: review?(daniel) → review+
Assignee

Updated

4 years ago
Blocks: 237623
Assignee

Comment 19

4 years ago
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
Attachment #8655007 - Flags: approval-mozilla-beta?
Attachment #8655007 - Flags: approval-mozilla-aurora?
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!
Flags: needinfo?(benny)
Adding the keyword regression and setting status affected for 41+
Reporter

Comment 22

4 years ago
(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.
Flags: needinfo?(benny)
Reporter

Comment 23

4 years ago
FF Nightly 43.0a1 (2015-09-02) fails. Will try again tomorrow. Let me know if you need anything.
https://hg.mozilla.org/mozilla-central/rev/9620db5a619d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Reporter

Comment 25

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

Do you know when this will go to production?

Thanks!
(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 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.
Attachment #8655007 - Flags: approval-mozilla-beta?
Attachment #8655007 - Flags: approval-mozilla-beta+
Attachment #8655007 - Flags: approval-mozilla-aurora?
Attachment #8655007 - Flags: approval-mozilla-aurora+
Reporter

Comment 30

4 years ago
Can confirm that this issue is resolved in 41.0!

Thanks a ton for all your work and your attention to this issue!
Usually we don't document bug fixes (unless very old). In this case, I think the site-compat info will be enough.
You need to log in before you can comment on or make changes to this bug.