fix(firefox): preserve response header field boundaries by dcrousso · Pull Request #42832 · microsoft/playwright · GitHub
Skip to content

fix(firefox): preserve response header field boundaries - #42832

Merged
Devin Rousso (dcrousso) merged 1 commit into
microsoft:mainfrom
dcrousso:fix-42687
Oct 5, 2026
Merged

Devin Rousso (dcrousso) merged 1 commit into
microsoft:mainfrom
dcrousso:fix-42687

Conversation

@dcrousso

Copy link
Copy Markdown
Contributor

splitting response headers on commas corrupts values and invents field boundaries

use the original fields reported by Juggler without parsing them again

reuse Chromium's Set-Cookie splitter for fulfillment so multiple cookies remain separate fields

work towards #42687

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

🟡 Almost certainly flakes — one I can't prove

Hi, I'm the Playwright bot and I triaged the MCP run.

None of the 4 failures touch response headers: this PR only changes Firefox/Chromium header field handling, and the failing tests assert on browser listing, file upload, and WebMCP frame disambiguation. Three of them have a documented history of failing on other SHAs; the fourth has no history either way.

Details

Pre-existing flake / infra

Uncertain

  • [chromium] › mcp/cli-webmcp.spec.ts:162 › webmcp-call disambiguates duplicate tool names by frame @mcp-macos-latest-chromium — this is its only failure in the results DB (1 of 24 runs), so by the numbers alone I can't call it pre-existing. It does fail the same way as its sibling above — the echo tool resolved in a single frame instead of being reported as ambiguous — and this file is only ~25 runs old, so there is little history to lean on. To be sure it's a flake I'd want a re-run of the MCP job, or one more sighting on an unrelated SHA.

Triaged by the Playwright bot - agent run

splitting response headers on commas corrupts values and invents field boundaries

use the original fields reported by Juggler without parsing them again

reuse Chromium's `Set-Cookie` splitter for fulfillment so multiple cookies remain separate fields
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

4 flaky ⚠️ [chromium-library] › library/browsercontext-page-event.spec.ts:173 › should work with Ctrl-clicking `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-page] › page/workers.spec.ts:191 › should attribute network activity for worker inside iframe to the iframe `@chromium-ubuntu-22.04-node20`
⚠️ [firefox-library] › library/browsercontext-cookies-third-party.spec.ts:257 › third party 'Partitioned;' cookies `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-library] › library/browsercontext-cookies-third-party.spec.ts:470 › top level 'Partitioned;' cookie and same origin iframe `@firefox-ubuntu-22.04-node20`

52544 passed, 1270 skipped


Merge workflow run.

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Test results for "MCP"

8937 passed, 1489 skipped


Merge workflow run.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aren't they already split here? My understanding is that now we pass split cookies between playwright and browser, but at which point can we glue them together so that this split is necessary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

route.fulfill({ response }) uses APIResponse.prototype.headers which joins Set-Cookie values with \n so we still need to split them before fulfillment

@dcrousso
Devin Rousso (dcrousso) merged commit ece43bf into microsoft:main Oct 5, 2026
45 checks passed
@dcrousso
Devin Rousso (dcrousso) deleted the fix-42687 branch October 5, 2026 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants