From f974c9c790dc383b425e9f0ef047251e79354bb7 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Tue, 20 Nov 2018 12:24:38 +0100 Subject: [PATCH] Undo treating blobs with NULs as a single line This partially reverts commit a551b646: revert the changes in RawParseUtils.lineMap(). Forcing all blobs containing a NUL byte as a single line causes blame to produce useless results as soon as it hits any version containing a NUL byte. Doing binary detection at this level also has the problem that the user cannot control it. Not by setting the text attribute nor in any other way. This came up in bug 541036, where a Java source inadvertently contained NUL bytes in strings. Even fixing this by using escapes "\000" will not fix JGit's blame for this file because the past versions will still contain the NUL byte. Native git can blame that file from bug 541036 fine. Added new tests verifying that blaming a text file containing a NUL byte produces sensible results. Bug: 541036 Change-Id: I8991bec88e9827cc096868c6026ea1890b6d0d32 Signed-off-by: Thomas Wolf --- .../eclipse/jgit/api/BlameCommandTest.java | 70 +++++++++++++++++++ .../org/eclipse/jgit/diff/RawTextTest.java | 9 ++- .../jgit/util/RawParseUtils_LineMapTest.java | 5 +- .../org/eclipse/jgit/util/RawParseUtils.java | 17 ++--- 4 files changed, 84 insertions(+), 17 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BlameCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BlameCommandTest.java index 7a1d222ca0..7e73084e8e 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BlameCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BlameCommandTest.java @@ -44,6 +44,7 @@ package org.eclipse.jgit.api; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import java.io.File; @@ -489,4 +490,73 @@ public class BlameCommandTest extends RepositoryTestCase { assertEquals(side, lines.getSourceCommit(2)); } } + + @Test + public void testBlameWithNulByteInHistory() throws Exception { + try (Git git = new Git(db)) { + String[] content1 = { "First line", "Another line" }; + writeTrashFile("file.txt", join(content1)); + git.add().addFilepattern("file.txt").call(); + RevCommit c1 = git.commit().setMessage("create file").call(); + + String[] content2 = { "First line", "Second line with NUL >\000<", + "Another line" }; + assertTrue("Content should contain a NUL byte", + content2[1].indexOf(0) > 0); + writeTrashFile("file.txt", join(content2)); + git.add().addFilepattern("file.txt").call(); + git.commit().setMessage("add line with NUL").call(); + + String[] content3 = { "First line", "Second line with NUL >\000<", + "Third line" }; + writeTrashFile("file.txt", join(content3)); + git.add().addFilepattern("file.txt").call(); + RevCommit c3 = git.commit().setMessage("change third line").call(); + + String[] content4 = { "First line", "Second line with NUL >\\000<", + "Third line" }; + assertTrue("Content should not contain a NUL byte", + content4[1].indexOf(0) < 0); + writeTrashFile("file.txt", join(content4)); + git.add().addFilepattern("file.txt").call(); + RevCommit c4 = git.commit().setMessage("fix NUL line").call(); + + BlameResult lines = git.blame().setFilePath("file.txt").call(); + assertEquals(3, lines.getResultContents().size()); + assertEquals(c1, lines.getSourceCommit(0)); + assertEquals(c4, lines.getSourceCommit(1)); + assertEquals(c3, lines.getSourceCommit(2)); + } + } + + @Test + public void testBlameWithNulByteInTopRevision() throws Exception { + try (Git git = new Git(db)) { + String[] content1 = { "First line", "Another line" }; + writeTrashFile("file.txt", join(content1)); + git.add().addFilepattern("file.txt").call(); + RevCommit c1 = git.commit().setMessage("create file").call(); + + String[] content2 = { "First line", "Second line with NUL >\000<", + "Another line" }; + assertTrue("Content should contain a NUL byte", + content2[1].indexOf(0) > 0); + writeTrashFile("file.txt", join(content2)); + git.add().addFilepattern("file.txt").call(); + RevCommit c2 = git.commit().setMessage("add line with NUL").call(); + + String[] content3 = { "First line", "Second line with NUL >\000<", + "Third line" }; + writeTrashFile("file.txt", join(content3)); + git.add().addFilepattern("file.txt").call(); + RevCommit c3 = git.commit().setMessage("change third line").call(); + + BlameResult lines = git.blame().setFilePath("file.txt").call(); + assertEquals(3, lines.getResultContents().size()); + assertEquals(c1, lines.getSourceCommit(0)); + assertEquals(c2, lines.getSourceCommit(1)); + assertEquals(c3, lines.getSourceCommit(2)); + } + } + } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RawTextTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RawTextTest.java index 5885d9b7e6..178d62072d 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RawTextTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RawTextTest.java @@ -66,13 +66,16 @@ public class RawTextTest { } @Test - public void testBinary() { + public void testNul() { String input = "foo-a\nf\0o-b\n"; byte[] data = Constants.encodeASCII(input); final RawText a = new RawText(data); assertArrayEquals(a.content, data); - assertEquals(a.size(), 1); - assertEquals(a.getString(0, 1, false), input); + assertEquals(2, a.size()); + assertEquals("foo-a\n", a.getString(0, 1, false)); + assertEquals("f\0o-b\n", a.getString(1, 2, false)); + assertEquals("foo-a", a.getString(0, 1, true)); + assertEquals("f\0o-b", a.getString(1, 2, true)); } @Test diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RawParseUtils_LineMapTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RawParseUtils_LineMapTest.java index 7630c11185..e7bfa000ac 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RawParseUtils_LineMapTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RawParseUtils_LineMapTest.java @@ -85,10 +85,11 @@ public class RawParseUtils_LineMapTest { } @Test - public void testBinary() { + public void testNulByte() { final byte[] buf = "xxxfoo\nb\0ar".getBytes(ISO_8859_1); final IntList map = RawParseUtils.lineMap(buf, 3, buf.length); - assertArrayEquals(new int[]{Integer.MIN_VALUE, 3, buf.length}, asInts(map)); + assertArrayEquals(new int[] { Integer.MIN_VALUE, 3, 7, buf.length }, + asInts(map)); } @Test diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java index bbb1645c59..a440cb275c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java @@ -677,10 +677,6 @@ public final class RawParseUtils { *

* The last element (index map.size()-1) always contains * end. - *

- * If the data contains a '\0' anywhere, the whole region is considered - * binary and a LineMap corresponding to a single line is returned. - *

* * @param buf * buffer to scan. @@ -689,18 +685,15 @@ public final class RawParseUtils { * line 1. * @param end * 1 past the end of the content within buf. - * @return a line map indicating the starting position of each line, or a - * map representing the entire buffer as a single line if - * buf contains a NUL byte. + * @return a line map indicating the starting position of each line. */ public static final IntList lineMap(byte[] buf, int ptr, int end) { - IntList map = lineMapOrNull(buf, ptr, end); - if (map == null) { - map = new IntList(3); - map.add(Integer.MIN_VALUE); + IntList map = new IntList((end - ptr) / 36); + map.fillTo(1, Integer.MIN_VALUE); + for (; ptr < end; ptr = nextLF(buf, ptr)) { map.add(ptr); - map.add(end); } + map.add(end); return map; } -- 2.39.5