Fixes #1835 #1840
Fixes #1835 #1840
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
I have read the Contributor License Agreement and (how could I?) sign it. |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Pull Request Overview
Fixes issue #1835 where attachments with % characters in filenames fail to download with n8n HTTP requests. The PR replaces the content-disposition library usage with a manual RFC 6266 compliant header construction using UTF-8 encoding.
- Removes dependency on
content-dispositionlibrary in DocWorker.ts - Replaces library calls with manual header construction using
filename*=UTF-8''encodeURIComponent(filename)format in both DocWorker.ts and DocApi.ts
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/server/lib/DocWorker.ts | Removes content-disposition import and replaces library usage with manual header construction |
| app/server/lib/DocApi.ts | Replaces content-disposition library call with manual UTF-8 encoded header format |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Construct a content-disposition header of the form 'attachment; filename="NAME"' | ||
| .set('Content-Disposition', contentDisposition(fileName, {type: 'attachment'})) | ||
| .set('Content-Disposition', `attachment; filename*=UTF-8''${encodeURIComponent(fileName)}`) |
There was a problem hiding this comment.
The Content-Disposition header construction is now duplicated between DocApi.ts and DocWorker.ts. Consider extracting this logic into a shared utility function to avoid code duplication and ensure consistency.
Copilot uses AI. Check for mistakes.
|
Thanks for this @tristanrobert. @jordigh plans to review. Btw what do you think of this part of the content disposition spec:
|
these two librairies have not been updated since 3 years ago. A simple replace with a regex could do the job:
|
Context
Attachment with % in filename fails to download with n8n http request on API.
Proposed solution
On DocAPI.ts, it replaces
contentdisposition(filename, {type: 'attachment'})function with afilename*=UTF-8''encodeURIComponent(filename)I am not sure that DocWorker is needed too.
Related issues
Has this been tested?
Yes with a n8n instance and a grist instance on docker compose.
Screenshots / Screencasts