Repository navigation
Conversation
…_file overwrite GET /fs/read_file on a directory returned 200 with a body that failed on the first read. It now returns 400 "path is a directory". PUT /fs/write_file passed mode only to OpenFile, which applies it when creating the file and through the umask. Overwriting an existing file kept its old mode, and mode=777 became 755. An explicit mode is now set on the file with Chmod; without one, an overwrite keeps the current mode.
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
1 participant
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.
Fixes #477.
Summary
GET /fs/read_file: return400 "path is a directory"when the path is a directory. Before, it sent200and the body failed on its first read.PUT /fs/write_file: whenmodeis given, set it withf.Chmodafter opening the file.OpenFileapplies the permission only on create and through the umask, so an overwrite kept the old mode and777became755. Withoutmode, an overwrite still keeps the file's current mode.No API or spec changes:
read_filealready documents a 400 response.Tests
TestReadFileDirectory: a directory path returnsReadFile400JSONResponse.TestWriteFileModeOnOverwritecovers four cases:755applies 755777is not narrowed by the umaskmodekeeps the current modemainand pass with this change.cd server && go test ./cmd/api/api/ -run 'File|Fs|Upload|Download|Dir|Zip|Zstd'passes, andgo vetandgofmtare clean.Note
Low Risk
Localized filesystem handler fixes with targeted tests; no auth or API contract changes beyond aligning behavior with documented 400 responses.
Overview
Fixes filesystem API edge cases on read and write without changing the OpenAPI surface.
ReadFilenow stats the opened path and returns 400 with"path is a directory"when the target is a directory, instead of streaming a 200 response whose body errors on first read.WriteFilecallsChmodwith the requested mode when themodequery param is set, so overwrites honor the explicit permission (including modes that would be masked on create) while overwrites withoutmodestill preserve the existing file mode.Adds
TestReadFileDirectoryandTestWriteFileModeOnOverwriteto lock in default 644, explicit 755/777 on overwrite, and unchanged mode whenmodeis omitted.Reviewed by Cursor Bugbot for commit 36aade9. Bugbot is set up for automated code reviews on this repo. Configure here.