@matheusbsbs / matheus-bagliano / commits / d391c0ff94
Fork 0

diff: don't use pathname-based diff drivers for symlinks

When we're diffing symlinks, we consider the contents to be the pathname that the symlink points to. When a user sets up a userdiff driver like "*.pdf diff=pdf", their "diff.pdf.*" config generally tells us what to do with the content of pdf files. With the current code, we will actually process a symlink like "link.pdf" using a configured pdf driver, meaning we are using contents which consist of a pathname with configuration that is expecting contents that consist of an actual pdf file. The most noticeable example of this would have been textconv; however, it was already protected in its own textconv-specific code path. We can still see the breakage with something like "diff.*.binary", though. You could also see it with diff.*.funcname, though it is a bit harder to trigger accidentally there. This patch adds a check for S_ISREG lower in the callstack than the textconv-specific check, which should block use of any userdiff config for non-regular files. We can drop the check in the textconv code, which is now redundant. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 21, 2010 at 17:01 UTC d391c0ff94e1b314b0664db0e8eb5bd92934f9cb
2 files changed +34 -3
diff.c
+8 -3
@@ -1764,8 +1764,14 @@ static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two, char *pre
1764
1765 static void diff_filespec_load_driver(struct diff_filespec *one)
1766 {
1767 - if (!one->driver)
1767 + /* Use already-loaded driver */
1768 + if (one->driver)
1769 + return;
1770 +
1771 + if (S_ISREG(one->mode))
1772 one->driver = userdiff_find_by_path(one->path);
1773 +
1774 + /* Fallback to default settings */
1775 if (!one->driver)
1776 one->driver = userdiff_find_by_name("default");
1777 }
@@ -1813,8 +1819,7 @@ struct userdiff_driver *get_textconv(struct diff_filespec *one)
1819 {
1820 if (!DIFF_FILE_VALID(one))
1821 return NULL;
1816 - if (!S_ISREG(one->mode))
1817 - return NULL;
1822 +
1823 diff_filespec_load_driver(one);
1824 if (!one->driver->textconv)
1825 return NULL;
t/t4011-diff-symlink.sh
+26
@@ -94,4 +94,30 @@ test_expect_success \
94 test_must_fail git diff --no-index pinky brain > output 2> output.err &&
95 grep narf output &&
96 ! grep error output.err'
97 +
98 +test_expect_success SYMLINKS 'setup symlinks with attributes' '
99 + echo "*.bin diff=bin" >>.gitattributes &&
100 + echo content >file.bin &&
101 + ln -s file.bin link.bin &&
102 + git add -N file.bin link.bin
103 +'
104 +
105 +cat >expect <<'EOF'
106 +diff --git a/file.bin b/file.bin
107 +index e69de29..d95f3ad 100644
108 +Binary files a/file.bin and b/file.bin differ
109 +diff --git a/link.bin b/link.bin
110 +index e69de29..dce41ec 120000
111 +--- a/link.bin
112 ++++ b/link.bin
113 +@@ -0,0 +1 @@
114 ++file.bin
115 +\ No newline at end of file
116 +EOF
117 +test_expect_success SYMLINKS 'symlinks do not respect userdiff config by path' '
118 + git config diff.bin.binary true &&
119 + git diff file.bin link.bin >actual &&
120 + test_cmp expect actual
121 +'
122 +
123 test_done