Skip to content

fix(api): keep non-ASCII characters in uploaded file names - #3042

Open
ryanchou1994 wants to merge 2 commits into
karakeep-app:mainfrom
ryanchou1994:fix/preserve-non-ascii-upload-filenames
Open

fix(api): keep non-ASCII characters in uploaded file names#3042
ryanchou1994 wants to merge 2 commits into
karakeep-app:mainfrom
ryanchou1994:fix/preserve-non-ascii-upload-filenames

Conversation

@ryanchou1994

Copy link
Copy Markdown
Contributor

Description

Uploaded file names lost every non-ASCII character: Prüfung Größe Öl.pdf came back as Pr_fung Gr__e _l.pdf (one _ per character, so not reversible), and since asset bookmarks fall back to fileName for their title, that mangled name is what got displayed and searched.

The replacement was introduced in 39a650f for #1765, where a non-ASCII file name crashed S3 uploads: the name is stored as S3 user-defined metadata (x-amz-meta-file-name), which is sent as an HTTP header and must be ASCII (ERR_INVALID_CHAR). Sanitising at upload time fixed that crash but threw the information away for every backend — including the local file system store and the assets.fileName column the UI reads.

This PR moves the ASCII constraint to the only place it applies:

  • packages/shared/s3MetadataEncoding.ts (new): encodeS3MetadataValue / decodeS3MetadataValue. Values that are already printable ASCII are stored verbatim, so existing objects and the existing S3 e2e expectations are unchanged. Anything else is wrapped as an RFC 2047 encoded-word (=?UTF-8?B?…?=), which is ASCII-only and round-trips losslessly.
  • packages/shared/assetdb.ts: the S3 store encodes fileName when writing metadata and decodes it when reading it back.
  • packages/api/utils/fileName.ts (new) + upload.ts: the upload path now only replaces control characters (C0, DEL, C1) and keeps everything else, including non-ASCII letters. The file name is only ever stored as data (DB column, asset metadata, API response) and is never used as a filesystem path, so nothing else needs escaping there.

Fixes #3041

How Has This Been Tested?

  • New unit tests: packages/api/utils/fileName.test.ts (3) and packages/shared/s3MetadataEncoding.test.ts (4) — pnpm --filter @karakeep/api test and pnpm --filter @karakeep/shared test all green (8 and 111 tests respectively).
  • Red/green check: reverting sanitizeUploadFileName to the old /[^\x20-\x7E]/g makes the "preserves non-ASCII letters" test fail; making encodeS3MetadataValue return its input unchanged makes the "ASCII-only encoded word" test fail. Both pass again with the fix in place.
  • The header constraint itself, checked with Node 24's http.validateHeaderValue: 報告 2026.pdf and 🦞.txt are rejected with ERR_INVALID_CHAR when passed raw, and accepted once wrapped as =?UTF-8?B?…?=. (Latin-1 names such as Prüfung Größe Öl.pdf happen to pass Node's check, but S3 user-defined metadata is still specified as US-ASCII, so they are encoded too.)
  • pnpm --filter @karakeep/api typecheck, pnpm --filter @karakeep/shared typecheck, lint and format all clean.
  • Not run: the S3 e2e suite (packages/e2e_tests/tests/assetdb/s3-store.test.ts, needs MinIO). Its existing expectations use ASCII file names, which this change stores byte-for-byte as before.

Checklist:

  • I have carefully read CONTRIBUTING.md
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable (none needed)
  • I have no unrelated changes in the PR.
  • I have confirmed that any new dependencies are strictly necessary. (no new dependencies)
  • I have written tests for new code (if applicable)

Please describe to which degree, if any, an LLM was used in creating this pull request.

I directed and planned this change myself: choosing the issue, deciding to move the ASCII constraint to the S3 boundary instead of sanitising at upload, and reviewing and running every test. An LLM assistant (Claude Code) was used for roughly a third of the work — mainly drafting code, tests and parts of this description. I take full responsibility for the change.

Copilot AI lite review requested due to automatic review settings August 28, 2026 15:37

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR preserves Unicode upload filenames by limiting sanitization to control characters and by encoding non-ASCII S3 metadata as printable ASCII.

  • Adds filename sanitization and regression tests for Unicode and control characters.
  • Adds S3 metadata encoding/decoding with tests for ASCII compatibility and Unicode round trips.
  • Integrates the encoding boundary into S3 asset writes and reads.

Confidence Score: 4/5

The filename encoding collision should be fixed before merging because a valid literal ASCII filename can be changed when read from S3 metadata.

The encoder leaves all printable ASCII unchanged while the decoder reserves a subset of those values as encoded words, so the new transport representation cannot round-trip every valid filename.

Files Needing Attention: packages/shared/s3MetadataEncoding.ts

Important Files Changed

Filename Overview
packages/api/utils/fileName.ts Replaces only control characters while preserving Unicode filename data; no issue identified.
packages/api/utils/upload.ts Routes uploaded filenames through the new sanitizer before database and asset-store persistence.
packages/shared/assetdb.ts Applies transport encoding at the S3 metadata boundary, but decoded filenames can inherit the encoding collision.
packages/shared/s3MetadataEncoding.ts Unicode values round-trip, but literal ASCII filenames matching the encoded-word syntax are incorrectly decoded.
packages/api/utils/fileName.test.ts Covers ASCII preservation, Unicode preservation, and control-character replacement.
packages/shared/s3MetadataEncoding.test.ts Covers ordinary ASCII and Unicode cases but omits the literal encoded-word collision.
Prompt To Fix All With AI
### Issue 1
packages/shared/s3MetadataEncoding.ts:13
**Encoded-word filenames lose identity**

When a literal ASCII filename matches the encoded-word syntax, `encodeS3MetadataValue` stores it verbatim but `decodeS3MetadataValue` interprets it as encoded data, causing names such as `=?UTF-8?B?SGVsbG8=?=` to become `Hello` when S3 metadata is read and potentially persisted back to the database.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(api): keep non-ASCII characters in u..." | Re-trigger Greptile


export function encodeS3MetadataValue(value: string): string {
if (ASCII_ONLY.test(value)) {
return value;

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.

P1 Encoded-word filenames lose identity

When a literal ASCII filename matches the encoded-word syntax, encodeS3MetadataValue stores it verbatim but decodeS3MetadataValue interprets it as encoded data, causing names such as =?UTF-8?B?SGVsbG8=?= to become Hello when S3 metadata is read and potentially persisted back to the database.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/shared/s3MetadataEncoding.ts
Line: 13

Comment:
**Encoded-word filenames lose identity**

When a literal ASCII filename matches the encoded-word syntax, `encodeS3MetadataValue` stores it verbatim but `decodeS3MetadataValue` interprets it as encoded data, causing names such as `=?UTF-8?B?SGVsbG8=?=` to become `Hello` when S3 metadata is read and potentially persisted back to the database.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@ryanchou1994

Copy link
Copy Markdown
Contributor Author

Good catch on the encoded-word collision — fixed in 70b0085: a literal name that already matches the =?UTF-8?B?…?= syntax is now encoded as well, so decoding is unambiguous. Added a round-trip test for that case.

Uploads replaced every non-ASCII character in the file name with "_", so
"Prüfung Größe Öl.pdf" was stored and shown as "Pr_fung Gr__e _l.pdf" and the
original name was lost (one underscore per character, not reversible).

The replacement was added in 39a650f for karakeep-app#1765: the file name is also written
to S3 as user-defined metadata (x-amz-meta-file-name), which travels as an HTTP
header and must be ASCII, otherwise Node rejects the request with
ERR_INVALID_CHAR. Sanitising the name at upload time fixed the crash but threw
away the information for every backend, including the local file system and
the database column that the UI displays.

Move the ASCII constraint to where it actually applies: the S3 store now wraps
non-ASCII metadata values as an RFC 2047 encoded-word on write and unwraps them
on read, while plain ASCII values are stored verbatim so existing objects are
unaffected. The upload path only replaces control characters (C0, DEL, C1) and
keeps everything else as-is; the file name is stored as data and never used as
a filesystem path.

Fixes karakeep-app#3041
…-word

A literal ASCII file name such as "=?UTF-8?B?SGVsbG8=?=" was stored verbatim
but unwrapped to "Hello" when read back from S3 metadata. Encode such values
too so decodeS3MetadataValue can never misinterpret a literal name.
@ryanchou1994
ryanchou1994 force-pushed the fix/preserve-non-ascii-upload-filenames branch from 70b0085 to 4300501 Compare September 2, 2026 07:57
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.

Uploaded filenames lose all non-ASCII characters and the original is not preserved

2 participants