fix(mcp): headerParser truncates header values containing colons#39469
fix(mcp): headerParser truncates header values containing colons#394692150997113 wants to merge 1 commit intomicrosoft:mainfrom
Conversation
|
@microsoft-github-policy-service agree |
|
FYI, there is already an open PR for the same issue. |
This comment has been minimized.
This comment has been minimized.
The headerParser function used split(':') which splits on ALL colons,
causing header values like 'http://example.com' to be truncated to 'http'.
This affects --cdp-header CLI flag and PLAYWRIGHT_MCP_CDP_HEADERS env var.
Fix by splitting only on the first colon, as per HTTP header spec (RFC 7230).
Fixes microsoft#1417
2150997113
force-pushed
the
fix/header-parser-colon-bug
branch
from
ac747df to
437c7cb
Compare
| }); | ||
|
|
||
| test('cdp server with headers containing colons', async ({ startClient, server }) => { | ||
| // Regression test for https://github.com/microsoft/playwright-mcp/issues/1417 |
There was a problem hiding this comment.
nit: we usually use
test('cdp server with headers containing colons', {
annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright-mcp/issues/1417' },
},async ({ startClient, server }) => {
Test results for "MCP"6 failed 5218 passed, 171 skipped Merge workflow run. |
| // Regression test for https://github.com/microsoft/playwright-mcp/issues/1417 | ||
| let customHeader = ''; | ||
| server.setRoute('/json/version/', (req, res) => { | ||
| customHeader = req.headers['x-custom']; |
There was a problem hiding this comment.
Let's fix the lint error and we can merge this.
|
ping? |
|
It was fixed in #39401 |
3 participants
Summary
Fixes microsoft/playwright-mcp#1417
The
headerParserfunction inpackages/playwright-core/src/mcp/browser/config.tsusedsplit(':')which splits on all colons in the string. JavaScript destructuringconst [name, value]only captures the first two array elements, silently discarding any content after the second colon.Impact
This bug affects:
--cdp-headerCLI flagPLAYWRIGHT_MCP_CDP_HEADERSenvironment variableExamples of truncated headers:
X-Custom: http://example.comhttp://example.comhttp❌X-Auth: token:secret:datatoken:secret:datatoken❌X-Host: localhost:8080localhost:8080localhost❌Fix
Split only on the first colon using
indexOf(':')andsubstring(), which matches the HTTP header spec (RFC 7230 Section 3.2): header field values can contain colons, only the first colon separates name from value.Test Plan
Added comprehensive test cases in
tests/mcp/config.spec.ts:http://example.com)All tests pass: