Commit 10ac298c62b40a205e771f50f9171c900c0e50dd

Patrick Steinhardt 2019-06-07T11:12:42

attr_file: fix unescaping of escapes required for fnmatch When parsing attribute patterns, we will eventually unescape the parsed pattern. This is required because we require custom escapes for whitespace characters, as normally they are used to terminate the current pattern. Thing is, we don't only unescape those whitespace characters, but in fact all escaped sequences. So for example if the pattern was "\*", we unescape that to "*". As this is directly passed to fnmatch(3) later, fnmatch would treat it as a simple glob matching all files where it should instead only match a file with name "*". Fix the issue by unescaping spaces, only. Add a bunch of tests to exercise escape parsing.

diff --git a/src/attr_file.c b/src/attr_file.c
index 5b1007e..510ed10 100644
--- a/src/attr_file.c
+++ b/src/attr_file.c
@@ -574,6 +574,34 @@ static size_t trailing_space_length(const char *p, size_t len)
 	return len - n;
 }
 
+static size_t unescape_spaces(char *str)
+{
+	char *scan, *pos = str;
+	bool escaped = false;
+
+	if (!str)
+		return 0;
+
+	for (scan = str; *scan; scan++) {
+		if (!escaped && *scan == '\\') {
+			escaped = true;
+			continue;
+		}
+
+		/* Only insert the escape character for escaped non-spaces */
+		if (escaped && !git__isspace(*scan))
+			*pos++ = '\\';
+
+		*pos++ = *scan;
+		escaped = false;
+	}
+
+	if (pos != scan)
+		*pos = '\0';
+
+	return (pos - str);
+}
+
 /*
  * This will return 0 if the spec was filled out,
  * GIT_ENOTFOUND if the fnmatch does not require matching, or
@@ -701,8 +729,8 @@ int git_attr_fnmatch__parse(
 		*base = git__next_line(pattern);
 		return -1;
 	} else {
-		/* strip '\' that might have be used for internal whitespace */
-		spec->length = git__unescape(spec->pattern);
+		/* strip '\' that might have been used for internal whitespace */
+		spec->length = unescape_spaces(spec->pattern);
 		/* TODO: convert remaining '\' into '/' for POSIX ??? */
 	}
 
diff --git a/tests/ignore/path.c b/tests/ignore/path.c
index 425a49c..0c22582 100644
--- a/tests/ignore/path.c
+++ b/tests/ignore/path.c
@@ -459,3 +459,65 @@ void test_ignore_path__negative_directory_rules_only_match_directories(void)
 	assert_is_ignored(false, "src/A.keep");
 	assert_is_ignored(false, ".gitignore");
 }
+
+void test_ignore_path__escaped_character(void)
+{
+	cl_git_rewritefile("attr/.gitignore", "\\c\n");
+	assert_is_ignored(true, "c");
+	assert_is_ignored(false, "\\c");
+}
+
+void test_ignore_path__escaped_newline(void)
+{
+	cl_git_rewritefile(
+		"attr/.gitignore",
+		"\\\nnewline\n"
+	);
+
+	assert_is_ignored(true, "\nnewline");
+}
+
+void test_ignore_path__escaped_glob(void)
+{
+	cl_git_rewritefile("attr/.gitignore", "\\*\n");
+	assert_is_ignored(true, "*");
+	assert_is_ignored(false, "foo");
+}
+
+void test_ignore_path__escaped_comments(void)
+{
+	cl_git_rewritefile(
+		"attr/.gitignore",
+		"#foo\n"
+		"\\#bar\n"
+		"\\##baz\n"
+		"\\#\\\\#qux\n"
+	);
+
+	assert_is_ignored(false, "#foo");
+	assert_is_ignored(true, "#bar");
+	assert_is_ignored(false, "\\#bar");
+	assert_is_ignored(true, "##baz");
+	assert_is_ignored(false, "\\##baz");
+	assert_is_ignored(true, "#\\#qux");
+	assert_is_ignored(false, "##qux");
+	assert_is_ignored(false, "\\##qux");
+}
+
+void test_ignore_path__escaped_slash(void)
+{
+	cl_git_rewritefile(
+		"attr/.gitignore",
+		"\\\\\n"
+		"\\\\preceding\n"
+		"inter\\\\mittent\n"
+		"trailing\\\\\n"
+	);
+
+#ifndef GIT_WIN32
+	assert_is_ignored(true, "\\");
+	assert_is_ignored(true, "\\preceding");
+#endif
+	assert_is_ignored(true, "inter\\mittent");
+	assert_is_ignored(true, "trailing\\");
+}