Skip to content

Fix stateful NON_LATIN1_REGEXP in createparams#128

Open
deepview-autofix wants to merge 1 commit into
jshttp:masterfrom
deepview-autofix:deepview/901018d171
Open

Fix stateful NON_LATIN1_REGEXP in createparams#128
deepview-autofix wants to merge 1 commit into
jshttp:masterfrom
deepview-autofix:deepview/901018d171

Conversation

@deepview-autofix

Copy link
Copy Markdown

The module-level NON_LATIN1_REGEXP has the /g flag, so calling .test(fallback) leaves lastIndex at the match position. A subsequent create() call with a non-ISO-8859-1 fallback would resume matching from that stale index, could miss earlier non-latin1 characters, and silently bypass the "fallback must be ISO-8859-1 string" validation - producing a Content-Disposition with non-latin1 bytes in filename=.

Reset lastIndex to 0 before the .test call to keep the validation deterministic across invocations.

The module-level NON_LATIN1_REGEXP has the /g flag, so calling
`.test(fallback)` leaves `lastIndex` at the match position. A
subsequent `create()` call with a non-ISO-8859-1 fallback would
resume matching from that stale index, could miss earlier
non-latin1 characters, and silently bypass the
"fallback must be ISO-8859-1 string" validation - producing a
Content-Disposition with non-latin1 bytes in filename=.

Reset `lastIndex` to 0 before the `.test` call to keep the
validation deterministic across invocations.

Co-Authored-By: Claude <[email protected]>
Co-Authored-By: DeepView Autofix <[email protected]>
Co-Authored-By: Nikita Skovoroda <[email protected]>
Signed-off-by: Nikita Skovoroda <[email protected]>

@ChALkeR ChALkeR left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is mine

Prior to the fix:

> require('content-disposition')('1.pdf', { fallback: '😭' })
Uncaught TypeError: fallback must be ISO-8859-1 string
> require('content-disposition')('1.pdf', { fallback: '😭' })
Uncaught TypeError: fallback must be ISO-8859-1 string
> require('content-disposition')('1.pdf', { fallback: '😭' })
`attachment; filename="😭"; filename*=UTF-8''1.pdf`

@blakeembrey

Copy link
Copy Markdown
Member

It looks like this is an issue in the current released code, want to make a PR against 1.x branch instead? In the current main, that code will be going out in 2.x and I think it should just remove the /g flag, but it depends on a bunch of other PRs being merged first (including the refactor to not use regexes).

@ChALkeR

ChALkeR commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

@blakeembrey will do

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.

3 participants