Skip to content

Commit ccbc119

Browse files
ByronEliahKagan
andcommitted
Apply suggestions from code review
The [Naming Files, Paths, and Namespaces](https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file) article does not state that control characters or non-printable characters are in general forbidden in filenames. Instead, it says that it is okay to > Use any character in the current code page for a name, including Unicode characters and characters in the extended character set (128–255), except for the following: and then lists various things that are not allowed, where the one that is relevant to control characters is: > Characters whose integer representations are in the range from 1 through 31, except for alternate data streams where these characters are allowed. *[...]* No mention is made of 127 (0x7F). On Windows 10, I used PowerShell 7 for this experiment, which I believe would also work in PowerShell 6, but not Windows PowerShell, which doesn't support `` `u ``. First, as a baseline, I checked what happened if I tried to create a file whose name contained a low-numbered control character: ```text C:\Users\ek\source\repos\unusual-filenames [main]> echo hello > a`u{8}b Out-File: The filename, directory name, or volume label syntax is incorrect. : 'C:\Users\ek\source\repos\unusual-filenames\b' C:\Users\ek\source\repos\unusual-filenames [main]> echo hello > a`u{08}b Out-File: The filename, directory name, or volume label syntax is incorrect. : 'C:\Users\ek\source\repos\unusual-filenames\b' ``` I created a file whose name contained the `DEL` character, and even a file whose entire name is that character: ```text C:\Users\ek\source\repos\unusual-filenames [main]> echo hello > a`u{7F}b C:\Users\ek\source\repos\unusual-filenames [main +1 ~0 -0 !]> echo goodbye > `u{7F} C:\Users\ek\source\repos\unusual-filenames [main +2 ~0 -0 !]> ls Directory: C:\Users\ek\source\repos\unusual-filenames Mode LastWriteTime Length Name ---- ------------- ------ ---- -a--- 5/20/2024 5:59 PM 9 -a--- 5/20/2024 5:59 PM 7 ab ``` Thus this appears to work fine on Windows, and it seems fine that Git permits it: ```text C:\Users\ek\source\repos\unusual-filenames [main +2 ~0 -0 !]> git status On branch main No commits yet Untracked files: (use "git add <file>..." to include in what will be committed) "a\177b" "\177" nothing added to commit but untracked files present (use "git add" to track) C:\Users\ek\source\repos\unusual-filenames [main +2 ~0 -0 !]> git add . C:\Users\ek\source\repos\unusual-filenames [main +2 ~0 -0 ~]> git commit -m 'Initial commit' [main (root-commit) 543ccd5] Initial commit 2 files changed, 2 insertions(+) create mode 100644 "a\177b" create mode 100644 "\177" ``` Thus, gitoxide should probably permit it too. To be sure, I also tried creating such a file in Python 3.12 on the same system, by calling the `touch` method on a `Path` object. That worked, too. Co-authored-by: Eliah Kagan <[email protected]>
1 parent fcc3b69 commit ccbc119

File tree

2 files changed

+8
-8
lines changed

2 files changed

+8
-8
lines changed

gix-fs/tests/stack/mod.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ fn path_join_handling() {
5858
assert_eq!(
5959
p("c:").join("relative"),
6060
p("c:relative"),
61-
"absolute + relative = strange joined result with missing back-slash, but it's a valid path that works just like `c:\relative`"
61+
"absolute + relative = strange joined result with missing backslash, but it's a valid path that works just like `c:\relative`"
6262
);
6363
assert_eq!(
6464
p("c:\\").join("relative"),
@@ -74,7 +74,7 @@ fn path_join_handling() {
7474
assert_eq!(
7575
p("\\\\.\\base").join(absolute),
7676
p("\\\\.\\base\\absolute"),
77-
"absolute1 + absolute2 = joined result with backslash (device relative)"
77+
"absolute1 + absolute2 = joined result with backslash (device namespace)"
7878
);
7979
assert_eq!(
8080
p("\\\\?\\base").join(bs_absolute),
@@ -84,7 +84,7 @@ fn path_join_handling() {
8484
assert_eq!(
8585
p("\\\\.\\base").join(bs_absolute),
8686
p("\\\\.\\base\\absolute"),
87-
"absolute1 + absolute2 = joined result (device relative)"
87+
"absolute1 + absolute2 = joined result (device namespace)"
8888
);
8989

9090
assert_eq!(p("/").join("C:"), p("C:"), "unix-absolute + win-drive = win-drive");
@@ -101,7 +101,7 @@ fn path_join_handling() {
101101
assert_eq!(
102102
p("c:\\").join("\\\\.\\"),
103103
p("\\\\.\\"),
104-
"d-drive-with-bs + device-relative-unc = device-relative-unc"
104+
"d-drive-with-bs + device-namespace-unc = device-namespace-unc"
105105
);
106106
assert_eq!(
107107
p("/").join("C:/"),
@@ -112,7 +112,7 @@ fn path_join_handling() {
112112
assert_eq!(
113113
p("\\\\.").join("C:"),
114114
p("C:"),
115-
"device-relative-unc + win-drive-relative = win-drive-relative - c: was supposed to be relative, but it's not acting like it."
115+
"device-namespace-unc + win-drive-relative = win-drive-relative - c: was supposed to be relative, but it's not acting like it."
116116
);
117117
assert_eq!(p("relative").join("C:"), p("C:"), "relative + win-drive = win-drive");
118118

@@ -150,7 +150,7 @@ fn path_join_handling() {
150150
"absolute1 + absolute2 = absolute2"
151151
);
152152

153-
assert_eq!(p("/").join("C:"), p("/C:"), "absolute + win-absolute = joined result");
153+
assert_eq!(p("/").join("C:"), p("/C:"), "absolute + win-drive = joined result");
154154
assert_eq!(p("/").join("C:/"), p("/C:/"), "absolute + win-absolute = joined result");
155155
assert_eq!(
156156
p("/").join("C:\\"),
@@ -160,7 +160,7 @@ fn path_join_handling() {
160160
assert_eq!(
161161
p("relative").join("C:"),
162162
p("relative/C:"),
163-
"relative + win-absolute = joined result"
163+
"relative + win-drive = joined result"
164164
);
165165

166166
assert_eq!(

gix-validate/src/path.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ fn check_win_devices_and_illegal_characters(input: &BStr) -> Option<component::E
160160
{
161161
return Some(component::Error::WindowsReservedName);
162162
}
163-
if input.iter().any(|b| b.is_ascii_control() || b":<>\"|?*".contains(b)) {
163+
if input.iter().any(|b| *b < 0x20 || b":<>\"|?*".contains(b)) {
164164
return Some(component::Error::WindowsIllegalCharacter);
165165
}
166166
if input.ends_with(b".") || input.ends_with(b" ") {

0 commit comments

Comments
 (0)