fix: prevent unhandled exception crashes for invalid header values #9638
Conversation
🦋 Changeset detectedLatest commit: e692574 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| : value | ||
| ); | ||
| } catch (error) { | ||
| res.getHeaderNames().forEach((name) => res.removeHeader(name)); |
There was a problem hiding this comment.
Not sure on this one - should we just remove all headers, or just the ones up until the error, or try to parse the other headers and add all but the invalid ones? Are there any security implications to this?
There was a problem hiding this comment.
My thinking was that it's an entirely different response because of the error and it shouldn't carry over any of the original headers.
should we just remove all headers, or just the ones up until the error
In its current state at least, those two would mean the same as all the headers haven't yet been copied over to the res object yet, in essence we're undoing earlier iterations of this loop.
There was a problem hiding this comment.
I would agree with @gtm-nayan's implementation and reasoning
| @@ -485,19 +494,20 @@ test.describe('setHeaders', () => { | |||
| test('allows multiple set-cookie headers with different values', async ({ page }) => { | |||
| const response = await page.goto('/headers/set-cookie/sub'); | |||
| const cookies = (await response?.allHeaders())['set-cookie']; | |||
| expect(cookies.includes('cookie1=value1') && cookies.includes('cookie2=value2')).toBe(true); | |||
|
|
|||
| expect(cookies).toMatch('cookie1=value1'); | |||
There was a problem hiding this comment.
should we also check that there are no unexpected values?
fixes #9628
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.