Closed
Bug 1503696
Opened 5 years ago
Closed 5 years ago
Firefox 63 is dropping sessions with Spring application, 62 worked as expected.
Categories
(Core :: Networking: Cookies, defect)
Tracking
()
RESOLVED
FIXED
mozilla65
People
(Reporter: kaiwpost, Assigned: mossop, NeedInfo)
References
Details
(Keywords: regression)
Attachments
(2 files)
2.22 MB,
image/png
|
Details | |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
pascalc
:
approval-mozilla-release-
|
Details | Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:63.0) Gecko/20100101 Firefox/63.0 Steps to reproduce: A spring (JBoss) application for the State of Maryland that worked with Firefox 62 no longer works for 63. Everytime a link is clicked or a task is performed the session is dropped and the user is re-routed to the login screen. We tested changing some of the security settings but none of them resolved the problem. This could be an issue for many Spring applications out there, not just this one. Actual results: A spring (JBoss) application for the State of Maryland that worked with Firefox 62 no longer works for 63. Everytime a link is clicked or a task is performed the session is dropped and the user is re-routed to the login screen. We tested changing some of the security settings but none of them resolved the problem. This could be an issue for many Spring applications out there, not just this one. Expected results: Attaching screen shot for 63, including the view of the JSessionID cookies. The top portion of the image is before, the bottom after an action is performed. The session is lost as apparent from the JSessonID cookie. When performed using Firefox 62 this works as expected. Are there any settings we can change to get around this issue? The gorup has been testing and recommending the use of Firefox and were hoping there would be a solution for this.
Comment 1•5 years ago
|
||
This is difficult to debug without access to the site but there is one thing that you could do to narrow down the problem. We have a regression finder tool that downloads Firefox builds from different dates to find the change in Firefox that caused the problem. Can you please try our tool to find the change in Firefox that caused this ? -> https://mozilla.github.io/mozregression/
Flags: needinfo?(kaiwpost)
Updated•5 years ago
|
Keywords: regression
Updated•5 years ago
|
Component: Untriaged → Networking: Cookies
Product: Firefox → Core
Updated•5 years ago
|
Status: UNCONFIRMED → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Comment 3•5 years ago
|
||
No, sorry, I confused this with other bugs.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
Comment 4•5 years ago
|
||
A few questions for the original poster to determine if this is related to tracking protection... 1. What tracking protection settings were enabled in the browser? (See about:preferences#privacy) 2. What domains were loaded on the page (see Network in developer tools) that are related to the service which is broken? 3. What cookies are used or expected by the service, and what domains try to set those cookies? Thanks!
Comment 5•5 years ago
|
||
I can easily reproduce on https://mdedrs.health.maryland.gov. The session cookie changes when you go from the login page to the forgot password page.
Comment 6•5 years ago
|
||
Err, sorry, on https://mdedrs.health.maryland.gov/prod/secureLogin
Comment 7•5 years ago
|
||
This is a regression from bug 1453751. This is such a sad bug... What happens is that the main page is served with a Set-Cookie header and the favicon is set with another Set-Cookie header, and the Set-Cookie header from the favicon seems to override the one received from the main page. Mossop any chance you can have a look please?
Blocks: 1453751
status-firefox63:
--- → affected
status-firefox64:
--- → affected
status-firefox65:
--- → affected
tracking-firefox63:
--- → ?
tracking-firefox64:
--- → ?
tracking-firefox65:
--- → ?
Flags: needinfo?(dtownsend)
Updated•5 years ago
|
Flags: needinfo?(kaiwpost)
Assignee | ||
Comment 8•5 years ago
|
||
(In reply to :Ehsan Akhgari from comment #5) > I can easily reproduce on https://mdedrs.health.maryland.gov. The session > cookie changes when you go from the login page to the forgot password page. You're right that this didn't happen before bug 1453751 landed so Firefox has definitely changed in behaviour here. I can see why too and the fix is straightforward. But I'm not convinced that this is exactly the bug because the same session cookie change happens in Chrome where apparently the webapp works. Regardless I'll get up the fix to take us back to Firefox 62 behaviour and we can see where that takes us.
Comment 9•5 years ago
|
||
(In reply to Dave Townsend [:mossop] (he/him) from comment #8) > (In reply to :Ehsan Akhgari from comment #5) > > I can easily reproduce on https://mdedrs.health.maryland.gov. The session > > cookie changes when you go from the login page to the forgot password page. > > You're right that this didn't happen before bug 1453751 landed so Firefox > has definitely changed in behaviour here. I can see why too and the fix is > straightforward. But I'm not convinced that this is exactly the bug because > the same session cookie change happens in Chrome where apparently the webapp > works. Note that there is a big difference between how Firefox and Chrome set cookies from their content/renderer processes. In Firefox we set it asynchronously and in Chrome (the last time I checked) they set it synchronously. But indeed the behavior in comment 5 happens in Chrome too...
Updated•5 years ago
|
status-firefox-esr60:
--- → unaffected
Assignee | ||
Comment 10•5 years ago
|
||
Ok so here is what is happening with the steps in comment 5: We load https://mdedrs.health.maryland.gov/prod/secureLogin which sets the session cookie for /prod to "foo". We load the icon from https://mdedrs.health.maryland.gov/favicon.ico, no cookie is sent because no cookie is set for the root. Its response sets the session cookie for /prod to "bar". We load https://mdedrs.health.maryland.gov/prod/forgot sending the session cookie "bar". We again load the icon from https://mdedrs.health.maryland.gov/favicon.ico, no cookie is sent because no cookie is set for the root. Its response sets the session cookie for /prod to "baz". So basically every page load will set the session cookie to something new. This didn't used to happen because pre bug 1453751 we don't even contact the webserver if the icon is in the cache. So once the icon sets the cookie to "bar" above, it sticks going forwards. Even that seems like buggy behaviour, ideally the webapp would just set the session cookie to "foo" and have it stick. But I think that that is a bug on the server side. /favicon.ico is setting a session cookie for /prod, presumably because it doesn't receive a session cookie in the request. Short of deciding to ignore cookies from icon loads (and who knows what other sites that might break) I think Firefox is doing the right thing there.
Flags: needinfo?(dtownsend)
Assignee | ||
Comment 11•5 years ago
|
||
Previous to bug 1453751 favicons were loaded from the network by a <xul:image> tag with validate="never". This caused us to always use any cached version if possible. Bug 1453751 used a normal load type causing us to revalidate with the server for each request. This switches the loader to using the cache if possible.
Assignee | ||
Updated•5 years ago
|
Assignee: nobody → dtownsend
Status: REOPENED → ASSIGNED
Comment 12•5 years ago
|
||
(In reply to Dave Townsend [:mossop] (he/him) from comment #10) > Ok so here is what is happening with the steps in comment 5: > > We load https://mdedrs.health.maryland.gov/prod/secureLogin which sets the > session cookie for /prod to "foo". > > We load the icon from https://mdedrs.health.maryland.gov/favicon.ico, no > cookie is sent because no cookie is set for the root. Its response sets the > session cookie for /prod to "bar". > > We load https://mdedrs.health.maryland.gov/prod/forgot sending the session > cookie "bar". > > We again load the icon from https://mdedrs.health.maryland.gov/favicon.ico, > no cookie is sent because no cookie is set for the root. Its response sets > the session cookie for /prod to "baz". > > So basically every page load will set the session cookie to something new. > This didn't used to happen because pre bug 1453751 we don't even contact the > webserver if the icon is in the cache. So once the icon sets the cookie to > "bar" above, it sticks going forwards. > > Even that seems like buggy behaviour, ideally the webapp would just set the > session cookie to "foo" and have it stick. But I think that that is a bug on > the server side. /favicon.ico is setting a session cookie for /prod, > presumably because it doesn't receive a session cookie in the request. Short > of deciding to ignore cookies from icon loads (and who knows what other > sites that might break) I think Firefox is doing the right thing there. Aha, now I see what's happening. The server is returning a 302 redirect respond for /favicon.ico, pointing you to https://mdedrs.health.maryland.gov/prod/secureLogin;jsessionid=gre3cLnjs-TUZDc3vYBPox6x.worker03 which is basically the login page. What is happening here is that the server has been configured to redirect any URL that it doesn't understand to the login page, and alongside that request it also serves you a cookie to ensure that the visit to the login page includes the same value in the Cookie header as in the URL. This interacts poorly with browsers' heuristics to try to guesswork their way and try to download /favicon.ico. So we hit a non-existing URL, and get redirected to the login page. That response isn't cacheable since it's a 302, so we end up requesting it over and over again when trying to validate. So basically the bug is our heuristic hits a case that the server admin hasn't thought about. Who's to blame, who knows. But I bet this is why we used to have VALIDATE_NEVER before. We just didn't have a test for it. :-(
Assignee | ||
Comment 13•5 years ago
|
||
(In reply to :Ehsan Akhgari from comment #12) > This interacts poorly with browsers' heuristics to try to guesswork their > way and try to download /favicon.ico. So we hit a non-existing URL, and get > redirected to the login page. That response isn't cacheable since it's a > 302, so we end up requesting it over and over again when trying to validate. FWIW, I discovered that loading /favicon.ico is actually defined in a spec now. It doesn't specify what cache mode to use so the expectation would be to validate as normal. https://html.spec.whatwg.org/multipage/links.html#rel-icon
Comment 14•5 years ago
|
||
Pushed by dtownsend@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/655b8b4a0e67 Cache cookies in the same way we did before Firefox 63. r=mak
Comment 15•5 years ago
|
||
Backed out for failing eslint at gecko/browser/base/content/test/favicons/browser_favicon_cache.js Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=211235384&revision=655b8b4a0e67c347be75b454ce52a046ccc25a40 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=211235384&repo=autoland&lineNumber=273 Backout: https://hg.mozilla.org/integration/autoland/rev/c78efa8bed086c825bb471494cd10d06ca3423d5
Flags: needinfo?(dtownsend)
Comment 16•5 years ago
|
||
(In reply to Dave Townsend [:mossop] (he/him) from comment #13) > (In reply to :Ehsan Akhgari from comment #12) > > This interacts poorly with browsers' heuristics to try to guesswork their > > way and try to download /favicon.ico. So we hit a non-existing URL, and get > > redirected to the login page. That response isn't cacheable since it's a > > 302, so we end up requesting it over and over again when trying to validate. > > FWIW, I discovered that loading /favicon.ico is actually defined in a spec > now. It doesn't specify what cache mode to use so the expectation would be > to validate as normal. > > https://html.spec.whatwg.org/multipage/links.html#rel-icon Might be worth filing a spec issue about it to get the spec changed based on the findings in this bug (CCing Anne).
Comment 17•5 years ago
|
||
I filed https://github.com/whatwg/html/issues/4176 to at least keep track of this.
Assignee | ||
Updated•5 years ago
|
Flags: needinfo?(dtownsend)
Comment 18•5 years ago
|
||
Pushed by dtownsend@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fddfe98e6db5 Cache cookies in the same way we did before Firefox 63. r=mak
Comment 19•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fddfe98e6db5
Status: ASSIGNED → RESOLVED
Closed: 5 years ago → 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Assignee | ||
Comment 20•5 years ago
|
||
This fix is now available in the latest Firefox Nightly. Any chance you could test it to see if it completely solves the problem for you? Regardless I'm going to try to get the fix uplifted but it would be useful to know if there is more to do here.
Flags: needinfo?(kaiwpost)
Assignee | ||
Comment 21•5 years ago
|
||
Comment on attachment 9023808 [details] Bug 1503696: Cache cookies in the same way we did before Firefox 63. r=mak [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1453751 User impact if declined: User may be logged out of certain web applications with each page load. Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: Yes Needs manual test from QE?: No If yes, steps to reproduce: List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): Simple change to how we cache favicon requests. String changes made/needed:
Attachment #9023808 -
Flags: approval-mozilla-release?
Attachment #9023808 -
Flags: approval-mozilla-beta?
Comment 22•5 years ago
|
||
Comment on attachment 9023808 [details] Bug 1503696: Cache cookies in the same way we did before Firefox 63. r=mak load favicons from cache, fixing regression in 63; approved for 64.0b12.
Attachment #9023808 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 23•5 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/5021562e6baf
Updated•5 years ago
|
Flags: qe-verify-
Comment 24•5 years ago
|
||
Comment on attachment 9023808 [details] Bug 1503696: Cache cookies in the same way we did before Firefox 63. r=mak We don't have plans for another dot release and 64 is 2 weeks away, so wontfix for 63.
Attachment #9023808 -
Flags: approval-mozilla-release? → approval-mozilla-release-
Updated•5 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•