Fix encoding for non-ASCII filenames in Content-Disposition header#58259
Fix encoding for non-ASCII filenames in Content-Disposition header#58259mohithbunny wants to merge 1 commit into
Conversation
Resolved incorrect encoding handling in Content-Disposition header
|
Review requested:
|
nodejs-github-bot
added
http
| return unique; | ||
| } | ||
| function maybeEncodeFilenameHeader(value) { | ||
| const filenameMatch = value.match(/filename="(.+?)"/); |
There was a problem hiding this comment.
If I'm reading this correctly it assumes that the value for the filename is always quoted. It's been a while since I've looked but I'm not sure if that's actually required by the relevant specs. RFC2183 where this is originally defined generally is quite careful about indicating when the value must be quoted and I don't recall anything there saying that filename must be a quoted string value.
There was a problem hiding this comment.
Thanks for pointing that out, @jasnell — you're absolutely right. The current implementation incorrectly assumes that the filename value is always quoted, which isn't guaranteed per RFC 2183. According to the spec, the filename parameter can be either a quoted-string or a token, so it's valid for it to appear without quotes.
I've updated the logic to handle both quoted and unquoted filename values using a more flexible regex:
-> const filenameMatch = value.match(/filename=(?:"([^"]+)"|([^;\s]+))/);
-> const filename = filenameMatch ? (filenameMatch[1] || filenameMatch[2]) : null;
Let me know if you think we should handle any additional edge cases. Thanks again!
There was a problem hiding this comment.
-> const filenameMatch = value.match(/filename=(?:"([^"]+)"|([^;\s]+))/);
-> const filename = filenameMatch ? (filenameMatch[1] || filenameMatch[2]) : null;
|
This pull request has been marked as stale due to 210 days of inactivity. |
|
Hello, I'm doing some backlog hygiene on all PRs related to networking and streaming modules. There hasn't been any activity here for quite a while so I'm going to mark it Thank you! |
|
This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open. |
4 participants
Resolved incorrect encoding handling in Content-Disposition header