Browse Source

Merge pull request #5148 from Kelimion/fix-clean-path

Fix os2.clean_path on Windows
Jeroen van Rijn 3 months ago
parent
commit
f6a39ca675

+ 1 - 0
.gitignore

@@ -294,4 +294,5 @@ build.sh
 # RAD debugger project file
 *.raddbg
 *.rdi
+tests/issues/build/*
 misc/featuregen/featuregen

+ 5 - 1
core/os/os2/path_windows.odin

@@ -187,7 +187,6 @@ init_long_path_support :: proc() {
 	if value == 1 {
 		can_use_long_paths = true
 	}
-
 }
 
 @(require_results)
@@ -271,6 +270,11 @@ _clean_path_handle_start :: proc(path: string, buffer: []u8) -> (rooted: bool, s
 			start += 1
 		}
 		copy(buffer, path[:start])
+		for n in 0..<start {
+			if _is_path_separator(buffer[n]) {
+				buffer[n] = _Path_Separator
+			}
+		}
 	}
 	return
 }

+ 53 - 27
core/os/os2/stat_windows.odin

@@ -329,42 +329,68 @@ _is_reserved_name :: proc(path: string) -> bool {
 	return false
 }
 
-_is_UNC :: proc(path: string) -> bool {
-	return _volume_name_len(path) > 2
-}
-
-_volume_name_len :: proc(path: string) -> int {
+_volume_name_len :: proc(path: string) -> (length: int) {
 	if len(path) < 2 {
 		return 0
 	}
-	c := path[0]
+
 	if path[1] == ':' {
-		switch c {
+		switch path[0] {
 		case 'a'..='z', 'A'..='Z':
 			return 2
 		}
 	}
 
-	// URL: https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
-	if l := len(path); l >= 5 && _is_path_separator(path[0]) && _is_path_separator(path[1]) &&
-		!_is_path_separator(path[2]) && path[2] != '.' {
-		for n := 3; n < l-1; n += 1 {
-			if _is_path_separator(path[n]) {
-				n += 1
-				if !_is_path_separator(path[n]) {
-					if path[n] == '.' {
-						break
-					}
-				}
-				for ; n < l; n += 1 {
-					if _is_path_separator(path[n]) {
-						break
-					}
-				}
-				return n
+	/*
+		See: URL: https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
+		Further allowed paths can be of the form of:
+		- \\server\share or \\server\share\more\path
+		- \\?\C:\...
+		- \\.\PhysicalDriveX
+	*/
+	// Any remaining kind of path has to start with two slashes.
+	if !_is_path_separator(path[0]) || !_is_path_separator(path[1]) {
+		return 0
+	}
+
+	// Device path. The volume name is the whole string
+	if len(path) >= 5 && path[2] == '.' && _is_path_separator(path[3]) {
+		return len(path)
+	}
+
+	// We're a UNC share `\\host\share`, file namespace `\\?\C:` or UNC in file namespace `\\?\\host\share`
+	prefix := 2
+
+	// File namespace.
+	if len(path) >= 5 && path[2] == '?' && _is_path_separator(path[3]) {
+		if _is_path_separator(path[4]) {
+			// `\\?\\` UNC path in file namespace
+			prefix = 5
+		}
+
+		if len(path) >= 6 && path[5] == ':' {
+			switch path[4] {
+			case 'a'..='z', 'A'..='Z':
+				return 6
+			case:
+				return 0
 			}
-			break
 		}
 	}
-	return 0
-}
+
+	// UNC path, minimum version of the volume is `\\h\s` for host, share.
+	// Can also contain an IP address in the host position.
+	slash_count := 0
+	for i in prefix..<len(path) {
+		// Host needs to be at least 1 character
+		if _is_path_separator(path[i]) && i > 0 {
+			slash_count += 1
+
+			if slash_count == 2 {
+				return i
+			}
+		}
+	}
+
+	return len(path)
+}

+ 48 - 37
tests/core/os/os2/path.odin

@@ -36,47 +36,58 @@ posix_to_dos_path :: proc(path: string) -> string {
 @(test)
 test_clean_path :: proc(t: ^testing.T) {
 	Test_Case :: struct{
-		path: string,
+		path:     string,
 		expected: string,
 	}
 
-	test_cases := [?]Test_Case {
-		{`../../foo/../../`,      `../../..`},
-		{`../../foo/..`,          `../..`},
-		{`../../foo`,             `../../foo`},
-		{`../..`,                 `../..`},
-		{`.././foo`,              `../foo`},
-		{`..`,                    `..`},
-		{`.`,                     `.`},
-		{`.foo`,                  `.foo`},
-		{`/../../foo/../../`,     `/`},
-		{`/../`,                  `/`},
-		{`/..`,                   `/`},
-		{`/`,                     `/`},
-		{`//home/foo/bar/../../`, `/home`},
-		{`/a/../..`,              `/`},
-		{`/a/../`,                `/`},
-		{`/a/あ`,                 `/a/あ`},
-		{`/a/あ/..`,              `/a`},
-		{`/あ/a/..`,              `/あ`},
-		{`/あ/a/../あ`,           `/あ/あ`},
-		{`/home/../`,             `/`},
-		{`/home/..`,              `/`},
-		{`/home/foo/../../usr`,   `/usr`},
-		{`/home/foo/../..`,       `/`},
-		{`/home/foo/../`,         `/home`},
-		{``,                      `.`},
-		{`a/..`,                  `.`},
-		{`a`,                     `a`},
-		{`abc//.//../foo`,        `foo`},
-		{`foo`,                   `foo`},
-		{`home/foo/bar/../../`,   `home`},
-	}
-
 	when ODIN_OS == .Windows {
-		for &tc in test_cases {
-			tc.path = posix_to_dos_path(tc.path)
-			tc.expected = posix_to_dos_path(tc.expected)
+		test_cases := [?]Test_Case {
+			{`W:/odin\examples\demo/demo.odin`,    `W:\odin\examples\demo\demo.odin`},
+			{`\\server\share\path\file.ext`,       `\\server\share\path\file.ext`},
+			{`//server\share/path\file.ext`,       `\\server\share\path\file.ext`},
+			{`/\192.168.0.10\share/path\file.ext`, `\\192.168.0.10\share\path\file.ext`},
+			{`\\?\C:/Users/Foo/path\file.ext`,     `\\?\C:\Users\Foo\path\file.ext`},
+			{`\\?\\localhost\share\file.ext`,      `\\?\\localhost\share\file.ext`},
+			{`//?\/192.168.0.10\share\file.ext`,   `\\?\\192.168.0.10\share\file.ext`},
+			{`\\.\PhysicalDrive3`,                 `\\.\PhysicalDrive3`},
+			{`/\./PhysicalDrive3`,                 `\\.\PhysicalDrive3`},
+			{`C:\a\..\..`,                         `C:\`},
+			{`C:\a\..`,                            `C:\`},
+			{`C:\あ/a/..`,                          `C:\あ`},
+			{`C:\あ/a/../あ`,                       `C:\あ\あ`},
+		}
+	} else {
+		test_cases := [?]Test_Case {
+			{`../../foo/../../`,      `../../..`},
+			{`../../foo/..`,          `../..`},
+			{`../../foo`,             `../../foo`},
+			{`../..`,                 `../..`},
+			{`.././foo`,              `../foo`},
+			{`..`,                    `..`},
+			{`.`,                     `.`},
+			{`.foo`,                  `.foo`},
+			{`/../../foo/../../`,     `/`},
+			{`/../`,                  `/`},
+			{`/..`,                   `/`},
+			{`/`,                     `/`},
+			{`//home/foo/bar/../../`, `/home`},
+			{`/a/../..`,              `/`},
+			{`/a/../`,                `/`},
+			{`/a/あ`,                 `/a/あ`},
+			{`/a/あ/..`,              `/a`},
+			{`/あ/a/..`,              `/あ`},
+			{`/あ/a/../あ`,           `/あ/あ`},
+			{`/home/../`,             `/`},
+			{`/home/..`,              `/`},
+			{`/home/foo/../../usr`,   `/usr`},
+			{`/home/foo/../..`,       `/`},
+			{`/home/foo/../`,         `/home`},
+			{``,                      `.`},
+			{`a/..`,                  `.`},
+			{`a`,                     `a`},
+			{`abc//.//../foo`,        `foo`},
+			{`foo`,                   `foo`},
+			{`home/foo/bar/../../`,   `home`},
 		}
 	}
 

+ 1 - 0
tests/core/os/os2/process.odin

@@ -1,3 +1,4 @@
+#+build !windows
 package tests_core_os_os2
 
 import os "core:os/os2"