fix: resolve 404/500 error when downloading files containing hash and percent character in filename#15823
Conversation
|
In newer path-to-regexp version the default delimiter is actually changed from |
There was a problem hiding this comment.
Pull request overview
Fixes REST route matching so download endpoints can serve uploaded files whose filenames include # (and similar reserved characters) when routed through Next.js catch-all params / path-to-regexp.
Changes:
- Adjust
path-to-regexpmatching inhandleEndpointsto usedelimiter: '/'and conditional decoding based on whether an overridepathwas provided. - Add an integration test that uploads a file with
#in the filename and asserts it can be fetched via the REST file endpoint.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/payload/src/utilities/handleEndpoints.ts |
Updates endpoint matching options to allow # / ? within :param matches and avoid double-decoding for pre-decoded paths. |
test/uploads/int.spec.ts |
Adds an integration test covering file serving when filenames include a hash character. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
File names with |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…with-hash-alternative
…with-hash-alternative
…with-hash-alternative
…with-hash-alternative
|
Alternative solution in #15799 |
|
Closing in favour of #15799 |
What?
Fixes file downloads returning 404/500 for filenames containing
#and%when served through the REST API.Why?
A request for file named
document%20#123.pdfwill be sent as/api/media/file/document%2520%23123.pdfNext.js catch-all
[...slug]params decodes URL-encoded segments before passing them to route handlers. So the slug will have segments['api', 'media', 'file', 'document%20#123.pdf']. In payload next handler these decoded segments are prepended with / and joined with /, meaning path is sent as/api/media/file/document%20#123.pdftohandleEndpointsfor further processing.path-to-regexpis used insidehandleEndpointsfor endpoint matching.There are two issues here:
path-to-regexpused in payload uses/#?as default delimiterpath-to-regexpdecodes the elements in the path with decodURIComponentBecause of issue 1
/file/:filenamewill not match/file/document%20#123.pdfbecause it has 3 segments['file', 'document%20', '123.pdf']instead of 2 since hash is also considered a separator.When issue 1 is fixed the matching will be be there, but because of issue 2 the parsed
:filenameparam will be set todocument #123.pdf. That is double decoded, resulting in an error that the file is not found on disk.How?
For issue 1 use only / as delimiter in path-to-regexp match call, so that paths are only separated by / as expected. There are no # and ? used as fragment or query separator to think about here, as any of those will be stripped away already, we are only matching pathname part of url.
For issue 2 add a new optional parameter pathEncoding to be able to inform that the path provided is already decoded (e.g. Next.js decoded catch-all slugs) while keeping the handleEndpoints method backwards compatible since this is an exported method and other callers might pass uriencoded paths and expect it to work.
Adds a test that creates a file with % and # in the filename and verifies it can be served via REST.
Fixes #15798