Handle absolute paths#306
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for nested and absolute path handling across filesystem operations by introducing a handle_path helper and updating tests to cover these scenarios.
- Introduced
handle_pathto split and resolve multi-segment paths before core operations. - Updated
create,remove_file,remove_dir, andrenameto usehandle_path. - Expanded
encryptedfstests to cover nested and absolute path cases; adjustedbenchsignature and added clippy suppressions.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test_common.rs | Removed unnecessary Sync bound from bench signature |
| src/encryptedfs/test.rs | Added comprehensive nested/absolute path tests for create, remove, rename |
| src/encryptedfs.rs | Implemented handle_path and wired it into core fs methods |
| src/crypto/write.rs | Added #[allow(clippy::manual_is_multiple_of)] to seek impl |
| src/crypto/read.rs | Added #[allow(clippy::manual_is_multiple_of)] to seek impl |
| return Err(FsError::InodeNotFound); | ||
| } | ||
| if self.exists_by_name(parent, name)? { | ||
| let (parent, name) = self.handle_path(parent, name.clone()).await?; |
There was a problem hiding this comment.
After you extract the final name via handle_path, run self.validate_filename(&name)? to enforce filename constraints and catch invalid characters.
| let (parent, name) = self.handle_path(parent, name.clone()).await?; | |
| let (parent, name) = self.handle_path(parent, name.clone()).await?; | |
| self.validate_filename(&name)?; |
| let input = name.expose_secret().to_string(); | ||
| let path = input.strip_prefix(".").unwrap_or(&input).to_owned(); | ||
| let mut path_segments = vec![]; | ||
| for segment in path.split("/") { |
There was a problem hiding this comment.
Consider explicitly rejecting ".." segments (e.g. return an error) to avoid directory traversal and prevent unintended parent‐dir resolution.
| self.data_dir.join(CONTENTS_DIR).join(ino.to_string()) | ||
| } | ||
|
|
||
| // TO-DO-handle-path-maybe |
There was a problem hiding this comment.
[nitpick] This TODO comment appears stale; either implement the missing logic here or remove it to keep the code clean.
Handle absolute paths
Related to -#210
The function handle_paths was created and added on:
EncryptedFs::create
EncryptedFs::remove_file
EncryptedFs::remove_dir
EncryptedFs::rename
Tests added for each function too.
-> New feature (non-breaking change which adds functionality)