feat(fileupload): add path encoder and content transcoder options - #666
Open
florinMiro wants to merge 2 commits into
Open
feat(fileupload): add path encoder and content transcoder options#666florinMiro wants to merge 2 commits into
florinMiro wants to merge 2 commits into
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
florinMiro
force-pushed
the
feat/fileupload-path-content-transform-options
branch
2 times, most recently
from
July 24, 2026 13:07
e9c34a4 to
49ab5ac
Compare
florinMiro
marked this pull request as ready for review
July 24, 2026 13:59
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 49ab5ac. Configure here.
This comment has been minimized.
This comment has been minimized.
florinMiro
marked this pull request as draft
July 24, 2026 14:02
Add WithPathEncoder and WithContentTranscoder to the high level file upload client. They let callers URI-encode upload paths and transcode file content, e.g. to UTF-8, without exposing the internal upload revision client. Both default to no-op, so existing callers are unaffected. Both are applied before the client side filters and the batch size accounting run, so those see what is transmitted rather than what is on disk: the path length filter measures the encoded path, and the file size filter and payload size accounting use the size the transcoded file reports from Stat. This is the same file the low level client stats when it validates a batch. The transcoded file is paired with the file it was opened from so that closing it releases the file descriptor even when the transcoder's wrapper does not delegate Close, and a file the transcoder fails on is skipped like any other unreadable file instead of failing the whole revision while its content is streamed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
florinMiro
force-pushed
the
feat/fileupload-path-content-transform-options
branch
from
July 27, 2026 10:08
f1964a6 to
359f665
Compare
florinMiro
marked this pull request as ready for review
July 27, 2026 10:08
This comment has been minimized.
This comment has been minimized.
PR Reviewer Guide 🔍
|
| osFile, err := os.Open(path) | ||
| if err != nil { | ||
| logger.Debug().Msgf("failed to open file: %s", path) | ||
| f.Close() |
Contributor
There was a problem hiding this comment.
Question: Why was this removed here? It is a bind mind bending to understand, maybe we can make this easier?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add
WithPathEncoderandWithContentTranscoderfunctional options to the high-level file upload client (pkg/apiclients/fileupload). They let callers URI-encode upload paths and transcode file content (e.g. to UTF-8) without exposing the internal upload revision client. Both default to no-op, so existing callers are unaffected.Both hooks are applied before the client-side filters and the batch size accounting run, so those see exactly what will be transmitted:
WithPathEncoderis applied right after the relative path is computed, so the path length filter, the paths reported for skipped files, and the transmitted path all see the encoded value. The filesystem path the file is read from is unchanged.WithContentTranscoderwraps each opened file before it is stat'ed, so the file size filter and the payload size accounting use the transcoded size. The transcoded file must report its own size and mode fromStat, which is what the low level client already relies on when validating a batch.Checklist
make test)make generate)make lint)🤖 Generated with Claude Code
Note
Medium Risk
Changes the upload batching pipeline and file descriptor lifecycle; incorrect transcoder
Stator path encoding could cause wrong limits or skipped files, though defaults preserve existing behavior and tests cover the new paths.Overview
Adds
WithPathEncoderandWithContentTranscoderon the high-level file upload client so callers can transform upload paths (e.g. URI encoding) and file streams (e.g. UTF-8) without touching the internal revision client. Both default to no-op.Encoding runs on the relative upload path before open; limits, skipped-file reporting, and transmitted paths use the encoded value while reads still use the real filesystem path. Transcoding wraps each opened file before
Statand filtering, so per-file size, batch payload accounting, and uploads reflect transcoded content. AtranscodedFilewrapper always closes the underlying OS file when the transcoder does not delegateClose. Size-limit skip reasons now reference the upload path instead of the stat name.Reviewed by Cursor Bugbot for commit 16db50c. Bugbot is set up for automated code reviews on this repo. Configure here.