fix: ensure multi domain passivity #20442
Conversation
|
Thanks for taking the time to open a PR!
|
AtofStryker
changed the title
|
Test summary
Run details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
||||||||||||||||||||||||||||||
AtofStryker
force-pushed
the
issue-19882-ensure-multi-domain-passivity
branch
from
dbb6c1b
to
33c94d5
Compare
SetInjectionLevel tests
AtofStryker
force-pushed
the
issue-19882-ensure-multi-domain-passivity
branch
from
33c94d5
to
2f0a428
Compare
AtofStryker
force-pushed
the
issue-19882-ensure-multi-domain-passivity
branch
from
2f0a428
to
e7e5d03
Compare
| @@ -213,7 +234,7 @@ describe('http/response-middleware', function () { | |||
| ...props.res, | |||
| }, | |||
| req: { | |||
| proxiedUrl: 'http:127.0.0.1:3501/multi-domain.html', | |||
| proxiedUrl: 'http://127.0.0.1:3501/multi-domain.html', | |||
There was a problem hiding this comment.
Turns out this wouldn't be parsable by the cors.urlMatchesOriginPolicyProps helper. It doesn't have an effect in this test since the remote strategy is set to foo, but figured it would be good to update this incase these tests are updated in the future and this helps discover a potential issue
| @@ -40,7 +40,7 @@ const onServer = function (app) { | |||
|
|
|||
| app.get('/cors', (req, res) => { | |||
| res.send(`<script> | |||
| fetch('https://127.0.0.2:44665/cross_origin') | |||
| fetch('https://www.foo.com:44665/cross_origin') | |||
There was a problem hiding this comment.
the snapshot for this test should NOT change
AtofStryker
changed the title
| @@ -13,5 +13,6 @@ | |||
| "retries": { | |||
| "runMode": 2, | |||
| "openMode": 0 | |||
| } | |||
| }, | |||
| "experimentalMultiDomain": true | |||
There was a problem hiding this comment.
Should we also be setting experimentalSessionSupport here?
There was a problem hiding this comment.
@mschile sorry I missed this comment! I don't think we need to because it likely would impact the cookie_spec. We can get away with setting experimentalSessionSupport as an override because the server doesn't need the value, which is nice!
User facing changelog
n/a
Additional details
To prevent current users from being impacted by the multi-domain work,
experimentalMultiDomainconfig checks have been added in the proxy to prevent any injection/delaying issues for users when they upgrade cypress when multi-domain is released, but aren't actively using it. The goal here is to make sure cross origin behavior with cypress remains the same when multi-domain is not in use.Most of the code changes are minimal, but tests needed to be updated/added. I added tests for the
SetInjectionLevelmiddleware to help better understand what injections are needed in what cases, as well as to test current injection behavior with/without the presence of configuration options.While working on multi-domain, we skipped the
captures cross origin failurestest innavigation_specasfullMultiDomainwas being injected regardless of flag. Since theexperimentalMultiDomainconfig option is now set to true for all driver tests, this test was migrated to a system test to test when this option isfalse. There are currently some issues replicating this behavior in firefox under system-test, and the test needs to be updated in the future once a suitable testing method is found.navigation_speccaptures cross origin failuresresults ondevelopnavigation_speccaptures cross origin failuresconverted to system-test running headfully example (slightly different)While doing work on this issue, I noticed that there was a test in the
web_security_specsystem test that was passing but should be failing. I adjusted the code to yield the following and hopefully gives us something to fall back on for multi-domain.How has the user experience changed?
N/A
PR Tasks
cypress-documentation?type definitions?cypress.schema.json?