-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cypress reuses a stale cookie #25841
Comments
Here's the rerun logs using d44b202#comments binary as you asked @AtofStryker: https://gist.github.com/valscion/842d6f893d43e5698d57d64d44f2c74d The error still remains. Let me know if I can help with this case in any way. |
@valscion thank you for moving this over into it's own issue. I'm surprised the binary didn't really have an impact as I would think it would take the newer cookie over the stale value in the jar before sending the request out. You have already provided a lot with a reproduction for the prepended dot issue, but would you also be able to get a small reproduction up and running for this issue? I might be able to debug the issue a bit faster and see if the solution I think might work will help us out here. |
Eesh, I can try — I was quite surprised to see that the reproduction I made with the prepended dot issue didn't show this issue as I tried to make it so 😅 I'll try to create a reproduction as I know fixing this issue without one will be next to impossible. |
@valscion I think it will be tough, but I think I have an idea what is going on here, especially from the logs. What I can try to do is try to get a fix together for what I think might fix the issue and build a binary for it. The problem is the fix might be fairly involved, likely more so than the prepended dot work. I have some other things I need to get through but I am hoping I can start trying something by Friday? |
@AtofStryker I managed to create a reproduction after a few hours of headbanging! 🎉 Here: https://github.com/valscion/cypress-stale-cookie-issue-reproduction
Let me know if there's anything more to this that you'd need |
@valscion awesome this should work! I need to figure out a way to run it since I have some permissions issues updating |
Oh damn. I can take a quick look if I can get this reproduction to run the Rails server inside Docker and update if I can't make it happen. I think you should be OK with not upgrading your system |
Ok there's now a way to boot the Rails test server with Docker:
Let me know if that works for you |
@valscion docker-compose worked great! I was able to reproduce the issue and verify the actual behavior in the browser. Hoping to be able to investigate soon. |
I updated the reproduction repository to v12.7.0 and posted the logs, screenshots and videos of failed test runs in this issue: |
@valscion awesome. I am hoping to take a deeper dive soon to figure out what the problem is. |
I have the same issue as well and can verify my logs match what is shown. I also turned on cookie debug and noticed there was a difference in the sameSite attribute for these cookies when they were being set by Cypress. The old cookie has sameSite:"lax" and the new cookie has sameSite:"unspecified". In the browser I'm not sure if that helps or hurts but that is the only obvious difference between the two that I can see on the front end. In the dev tools for the Chrome browser there isn't a value shown for sameSite |
That appears slightly different to what's happening in here. Maybe it's worth it to open a new issue about your issue @o3-steven and provide all the necessary issue details to triage that case? |
I have the same issue, with pretty much the same setup: I do a login via an AJAX request, which sets new cookies. Afterwards the page is reloaded, but this page reload sends the old cookies, instead of the new ones. This first starts happening with Cypress 10.10.0 and persists up to Cypress 12.11.0, whereas Cypress 10.9.0 worked fine. The changelog for 10.10.0 mentions the following, so it might be related to one of the mentioned issues:
|
Has anyone found a workaround for this issue? My team is actively trying to upgrade to v12 but is blocked by this issue. |
We're still blocked by this at least. I would help with this if I could but it seems the amount of context one needs to have to solve this is quite high. I'd also be happy to hear about workarounds. |
Im not sure if this is related but using latest Cypress and has caused some headache for our automation scripts with cookies, a developer had to help us resolve "Basically the problem why this needs added is Cypress is adding a cookie value to the Request Headers of Platform requests, which is falling over and causing a stack trace issue" :-( |
@DobQA do you know what version you were on and what you upgraded to? |
Latest version / Angular 14 webpage. I had todo a work around like below: cy.intercept('POST', '*', (req) => { |
OK just for historical context that should be |
If it helps, we're seeing this in our codebase and have it isolated. I'd be happy to jump on a video call, but creating a repro from our app/stack would be challenging. Our logic goes:
In my Cypress test, this is what I see (which is surprising and hopefully helpful). // login via API
cy.getCookies().then((cookies) => {
console.log(cookies) // <-- Cookie is present via cy.getCookies()
})
cy.document().then((document) => {
console.log(document.cookie) // <-- Cookie is NOT present on document, which breaks our app
console.log(window.parent.document.cookie) // <-- Cookie is present on Cypress host window
}) |
This also looks like a different issue than the one this issue is about. Could you create a new issue and fill in all the issue template questions there? |
I have updated the reproduction repository to Cypress v12.13.0 and here's the logs: valscion/cypress-stale-cookie-issue-reproduction#3 (comment) |
My apologies, I realized this morning that I was using the new Our webapp is an oauth provider, so we have tests that start (via The fix was: cy.visit('appurl') // make an initial visit to your app to set it as the "top"
cy.origin('partner site url', () => {
cy.visit('partner site url')
cy.get('oauth signup button').click()
})
cy.url().should('contain', 'appurl')
// back on app site, do other stuff with your cookies in tact |
I did want to check in and say we haven't forgotten about this issue. The team is currently occupied on other issues, but I am hoping we can get to a fix in the near future! |
Hey @AtofStryker we just ran into another issue with #27216. We are going to downgrade fastglob so that we can work around this and still use grep but issues like these and not being able to upgrade cypress are going to slowly put us in a bad spot. |
Reproduction repository has been updated to Cypress v12.17.3 and the failure logs are here: valscion/cypress-stale-cookie-issue-reproduction#3 (comment) |
@AtofStryker any updates on the issue? |
@Mert75 no updates yet, but as soon as we are able to work the issue I will post an update, unless anyone wants to work this as an open source contribution. |
Hi, @AtofStryker, are there any updates on this issue, we faced the same problem. |
Nope, no workaround :(. We're still stuck with a very old Cypress version because of this |
In our case, after the button is clicked, there are two consecutive requests. The first request will respond with a new cookie value, set-cookie, and then the new cookie value will be used in the second request. When seeing the networking tab in dev tools, the browser displays the correct cookies to be sent; however, with debug in Cypress, it does send the old cookies from the cookie jar. Here the temporary workaround I am currently using: |
Thanks @ksazhnev! I tested your workaround in here: However, it seems like that workaround would clear the cookies too late for the reproduction to go through unless I make the click the button again after clearing cookies. |
Seems like all that is needed is clearing the cookies before doing the However, if the cookies would need to be there e.g. to have a valid login session then this workaround would not work for all the possible use cases. EDIT: Thank you a ton, @ksazhnev! I was able to get our application tesys upgraded to use Cypress 13.5.0 with this simple workaround! 🎉 |
Sure glad it helped you @valscion, and thanks for opening this issue! I think even with the login it could work depending when the cy.clearCookies() is implemented, in our case there are previous cookies that have to be passed to the request with a button click, so that is why in my case I had to delete them after the cookie was passed to the first request and then delete it before I get the respond with a new cookie. This issue is still not resolved, I hope this will be resolved with the new version without the need for a workaround! |
Please create a new issue for this case as it's not clearly linked to this stale cookie usage. This sounds like a |
tried the binary with Cypress 14 prerelease mentioned here and unfortunately does not seem to fix the issue |
Current behavior
See below
Desired behavior
No response
Test code to reproduce
See below
Cypress Version
38a65a6#comments
Node version
v16.13.0
Operating System
macOS 13.1 (22C65)
Debug Logs
No response
Other
I took the version in 38a65a6#comments for a spin and that doesn't yet seem to resolve my original issue. However I can no longer see duplicated cookies prepended with a dot (as in #25174) so I'll have to dig deeper into figuring out what could cause the error I'm seeing in #25174 (comment)
I'd be more than happy to debug further if there's parts that would help figure out what's going wrong here. Anything that could provide you more insight onto this particular case is appreciated.
The application code under test is mostly doing this:
jQuery.ajax
document.location.reload(true)
With Cypress v11.2.0
The exported HAR file from Google Chrome's Network inspector: test.venuu.fi-cypress-11.2.har.zip
With this PR
The exported HAR file from Google Chrome's Network inspector: test.venuu.fi-cypress-12.x.har.zip
Originally posted by @valscion in #25761 (comment)
@valscion thank you for taking a look at this! I was going to update today since I was out, but you beat me to it. I am going to take a look and see what might be causing your issue here. Possibly a cookie getting overwritten? I should have an update soon!
Originally posted by @AtofStryker in #25761 (comment)
@valscion Just from looking at the screenshots, the cookies being sent look correct? Are you able to verify if cookies are being doubled up in the request with
DEBUG=cypress-verbose:proxy:http
enabled and seeing the cookies attached in the requests with the"cookies being sent with request"
log?Originally posted by @AtofStryker in #25761 (comment)
Here's the full STDERR output (using the build from commit c1d8360) from running with
DEBUG=cypress-verbose:proxy:http
: https://gist.github.com/valscion/8c05b57d24102f42ed5931e51c463439I noticed these parts from the logs that look suspicious to me (formatted the JS objects output for easier reading):
And more specifically this one:
It appears that it's sending
while the earlier line used a much much longer
_venuu_flash
value here:That is, the cookie value should've been:
Originally posted by @valscion in #25761 (comment)
Thank you for sending that over @valscion
I think I can see what is going on here. I looked at the gist you provided and right along this line I noticed something.
From what I can see, this request and a few others, are making
Document
requests to the main server, which is telling the server side cookie jar that cookies should be simulated since we might need to simulate the navigation. The problem is the initial cookie that is set is stored in the cookie jar, but isn't updated because it doesn't fit the simulation criteria. Then, when this request is sent through, the cookie jar overwrites the cookies sent in the request with stale cookie values, which is what we are seeing in these logs.Unfortunately, I think this issue might be out of scope for this PR, but I am trying to think of ways we might be able to solve it.
To help confirm this is the case, are you able to try this binary? The windows binaries did not build for this job, but this commit might be a good start to rule out if this is the issue since we don't have a reprod for this particular thing. If you need a windows binary, let me know!
Also, would you be willing to open a separate issue for this outside of #25174 since I think the cause of your problem might actually be unrelated?
Originally posted by @AtofStryker in #25761 (comment)
The text was updated successfully, but these errors were encountered: