Merge pull request #5435 from libgit2/ethomson/canonical

win32: don't canonicalize relative paths

src/win32/path_w32.c

@@ -25,6 +25,9 @@
 #define path__is_unc(p) \
 	(((p)[0] == '\\' && (p)[1] == '\\') || ((p)[0] == '/' && (p)[1] == '/'))
 
+#define path__startswith_slash(p) \
+	((p)[0] == '\\' || (p)[0] == '/')
+
 GIT_INLINE(int) path__cwd(wchar_t *path, int size)
 {
 	int len;
@@ -221,7 +224,7 @@ int git_win32_path_from_utf8(git_win32_path out, const char *src)
 			goto on_error;
 	}
 	/* Absolute paths omitting the drive letter */
-	else if (src[0] == '\\' || src[0] == '/') {
+	else if (path__startswith_slash(src)) {
 		if (path__cwd(dest, MAX_PATH) < 0)
 			goto on_error;
 
@@ -257,6 +260,30 @@ on_error:
 	return -1;
 }
 
+int git_win32_path_relative_from_utf8(git_win32_path out, const char *src)
+{
+	wchar_t *dest = out, *p;
+	int len;
+
+	/* Handle absolute paths */
+	if (git_path_is_absolute(src) ||
+	    path__is_nt_namespace(src) ||
+	    path__is_unc(src) ||
+	    path__startswith_slash(src)) {
+		return git_win32_path_from_utf8(out, src);
+	}
+
+	if ((len = git__utf8_to_16(dest, MAX_PATH, src)) < 0)
+		return -1;
+
+	for (p = dest; p < (dest + len); p++) {
+		if (*p == L'/')
+			*p = L'\\';
+	}
+
+	return len;
+}
+
 int git_win32_path_to_utf8(git_win32_utf8_path dest, const wchar_t *src)
 {
 	char *out = dest;

src/win32/path_w32.h

@@ -11,7 +11,9 @@
 #include "vector.h"
 
 /**
- * Create a Win32 path (in UCS-2 format) from a UTF-8 string.
+ * Create a Win32 path (in UCS-2 format) from a UTF-8 string.  If the given
+ * path is relative, then it will be turned into an absolute path by having
+ * the current working directory prepended.
  *
  * @param dest The buffer to receive the wide string.
  * @param src The UTF-8 string to convert.
@@ -20,12 +22,25 @@
 extern int git_win32_path_from_utf8(git_win32_path dest, const char *src);
 
 /**
+ * Create a Win32 path (in UCS-2 format) from a UTF-8 string.  If the given
+ * path is relative, then it will not be turned into an absolute path.
+ *
+ * @param dest The buffer to receive the wide string.
+ * @param src The UTF-8 string to convert.
+ * @return The length of the wide string, in characters (not counting the NULL terminator), or < 0 for failure
+ */
+extern int git_win32_path_relative_from_utf8(git_win32_path dest, const char *src);
+
+/**
  * Canonicalize a Win32 UCS-2 path so that it is suitable for delivery to the
  * Win32 APIs: remove multiple directory separators, squashing to a single one,
  * strip trailing directory separators, ensure directory separators are all
  * canonical (always backslashes, never forward slashes) and process any
  * directory entries of '.' or '..'.
  *
+ * Note that this is intended to be used on absolute Windows paths, those
+ * that start with `C:\`, `\\server\share`, `\\?\`, etc.
+ *
  * This processes the buffer in place.
  *
  * @param path The buffer to process

src/win32/posix_w32.c

@@ -447,8 +447,7 @@ int p_symlink(const char *target, const char *path)
 	 * relative symlinks, this is not someting we want.
 	 */
 	if (git_win32_path_from_utf8(path_w, path) < 0 ||
-	    git__utf8_to_16(target_w, MAX_PATH, target) < 0 ||
-	    git_win32_path_canonicalize(target_w) < 0)
+	    git_win32_path_relative_from_utf8(target_w, target) < 0)
 		return -1;
 
 	dwFlags = SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE;

tests/core/posix.c

@@ -190,6 +190,26 @@ void test_core_posix__symlink_resolves_to_correct_type(void)
 	git_buf_dispose(&contents);
 }
 
+void test_core_posix__relative_symlink(void)
+{
+	git_buf contents = GIT_BUF_INIT;
+
+	if (!git_path_supports_symlinks(clar_sandbox_path()))
+		clar__skip();
+
+	cl_must_pass(git_futils_mkdir("dir", 0777, 0));
+	cl_git_mkfile("file", "contents");
+	cl_git_pass(p_symlink("../file", "dir/link"));
+	cl_git_pass(git_futils_readbuffer(&contents, "dir/link"));
+	cl_assert_equal_s(contents.ptr, "contents");
+
+	cl_must_pass(p_unlink("file"));
+	cl_must_pass(p_unlink("dir/link"));
+	cl_must_pass(p_rmdir("dir"));
+
+	git_buf_dispose(&contents);
+}
+
 void test_core_posix__symlink_to_file_across_dirs(void)
 {
 	git_buf contents = GIT_BUF_INIT;

tests/path/win32.c

@@ -21,6 +21,21 @@ void test_utf8_to_utf16(const char *utf8_in, const wchar_t *utf16_expected)
 #endif
 }
 
+void test_utf8_to_utf16_relative(const char* utf8_in, const wchar_t* utf16_expected)
+{
+#ifdef GIT_WIN32
+	git_win32_path path_utf16;
+	int path_utf16len;
+
+	cl_assert((path_utf16len = git_win32_path_relative_from_utf8(path_utf16, utf8_in)) >= 0);
+	cl_assert_equal_wcs(utf16_expected, path_utf16);
+	cl_assert_equal_i(wcslen(utf16_expected), path_utf16len);
+#else
+	GIT_UNUSED(utf8_in);
+	GIT_UNUSED(utf16_expected);
+#endif
+}
+
 void test_path_win32__utf8_to_utf16(void)
 {
 #ifdef GIT_WIN32
@@ -129,6 +144,31 @@ void test_path_win32__absolute_from_relative(void)
 #endif
 }
 
+void test_path_win32__keeps_relative(void)
+{
+#ifdef GIT_WIN32
+	/* Relative paths stay relative */
+	test_utf8_to_utf16_relative("Foo", L"Foo");
+	test_utf8_to_utf16_relative("..\\..\\Foo", L"..\\..\\Foo");
+	test_utf8_to_utf16_relative("Foo\\..", L"Foo\\..");
+	test_utf8_to_utf16_relative("Foo\\..\\..", L"Foo\\..\\..");
+	test_utf8_to_utf16_relative("Foo\\Bar", L"Foo\\Bar");
+	test_utf8_to_utf16_relative("Foo\\..\\Bar", L"Foo\\..\\Bar");
+	test_utf8_to_utf16_relative("../../Foo", L"..\\..\\Foo");
+	test_utf8_to_utf16_relative("Foo/..", L"Foo\\..");
+	test_utf8_to_utf16_relative("Foo/../..", L"Foo\\..\\..");
+	test_utf8_to_utf16_relative("Foo/Bar", L"Foo\\Bar");
+	test_utf8_to_utf16_relative("Foo/../Bar", L"Foo\\..\\Bar");
+	test_utf8_to_utf16_relative("Foo/../Bar/", L"Foo\\..\\Bar\\");
+	test_utf8_to_utf16_relative("", L"");
+
+	/* Absolute paths are canonicalized */
+	test_utf8_to_utf16_relative("\\Foo", L"\\\\?\\C:\\Foo");
+	test_utf8_to_utf16_relative("/Foo/Bar/", L"\\\\?\\C:\\Foo\\Bar");
+	test_utf8_to_utf16_relative("\\\\server\\c$\\unc\\path", L"\\\\?\\UNC\\server\\c$\\unc\\path");
+#endif
+}
+
 #ifdef GIT_WIN32
 static void test_canonicalize(const wchar_t *in, const wchar_t *expected)
 {
@@ -203,16 +243,6 @@ void test_path_win32__canonicalize(void)
 	test_canonicalize(L"C:/Foo/Bar", L"C:\\Foo\\Bar");
 	test_canonicalize(L"C:/", L"C:\\");
 
-	test_canonicalize(L"Foo\\\\Bar\\\\Asdf\\\\", L"Foo\\Bar\\Asdf");
-	test_canonicalize(L"Foo\\\\Bar\\\\..\\\\Asdf\\", L"Foo\\Asdf");
-	test_canonicalize(L"Foo\\\\Bar\\\\.\\\\Asdf\\", L"Foo\\Bar\\Asdf");
-	test_canonicalize(L"Foo\\\\..\\Bar\\\\.\\\\Asdf\\", L"Bar\\Asdf");
-	test_canonicalize(L"\\", L"");
-	test_canonicalize(L"", L"");
-	test_canonicalize(L"Foo\\..\\..\\..\\..", L"");
-	test_canonicalize(L"..\\..\\..\\..", L"");
-	test_canonicalize(L"\\..\\..\\..\\..", L"");
-
 	test_canonicalize(L"\\\\?\\C:\\Foo\\Bar", L"\\\\?\\C:\\Foo\\Bar");
 	test_canonicalize(L"\\\\?\\C:\\Foo\\Bar\\", L"\\\\?\\C:\\Foo\\Bar");
 	test_canonicalize(L"\\\\?\\C:\\\\Foo\\.\\Bar\\\\..\\", L"\\\\?\\C:\\Foo");