fix(thumb blob path): separators be wrongly modified (#3062)#3116
fix(thumb blob path): separators be wrongly modified (#3062)#3116HFO4 merged 2 commits intocloudreve:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where path separators in thumbnail blob paths were being incorrectly modified by filepath.Dir(). The problem occurred because filepath.Dir() normalizes path separators based on the operating system (converting "/" to "" on Windows), which is undesirable when working with storage systems that require specific separator formats regardless of OS.
Key Changes:
- Replaced
filepath.Dir()with manual string manipulation to preserve original path separators - Uses
strings.LastIndexAny()to find the last "/" or "" and extracts the directory portion manually - Maintains backward compatibility while preventing unwanted separator normalization
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg/util/common.go
Outdated
| lastSlash := strings.LastIndexAny(completeBlobPath, "/\\") | ||
| if lastSlash >= 0 { | ||
| return completeBlobPath[:lastSlash] + fsSeparator | ||
| } | ||
| return fsSeparator |
There was a problem hiding this comment.
The ReplaceMagicVar function, including the new {blob_path} logic, lacks test coverage. Given that this function handles multiple magic variables and the complexity of path handling across different operating systems, comprehensive tests should be added to verify:
- The
{blob_path}behavior with various path formats (e.g., "a/b/c.txt", "/c.txt", "c.txt", "a\b\c.txt") - That the fix correctly preserves original path separators
- Edge cases like paths with mixed separators
Other similar functions in this file (RandStringRunes, ContainsUint, etc.) have test coverage in common_test.go.
|
Thanks! |
No description provided.