From 7d3040368fa8e00c4dfcbd0b7d2d92768cfc2c7e Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Mon, 19 Feb 2018 23:42:46 +0100 Subject: [PATCH] Fix DiffFormatter for diffs against working tree with autocrlf=true The WorkingTreeSource produced an ObjectLoader that returned inconsistent sizes: the file size in getSize(), but then a correctly filtered smaller stream in openStream(). This resulted either in an IOE "short read of block" or in an EOFException depending on the resulting filtered size. Fix this by ensuring that getSize() does return the size of the filtered stream. Bug: 530106 Change-Id: I7c7c85036047dc10030ed29c1d5a6c7f34f2bdff Signed-off-by: Thomas Wolf --- .../eclipse/jgit/diff/DiffFormatterTest.java | 98 +++++++++++++++++++ .../org/eclipse/jgit/diff/ContentSource.java | 5 +- .../src/org/eclipse/jgit/diff/RawText.java | 2 +- 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java index fabf03440..45832f495 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java @@ -55,12 +55,14 @@ import org.eclipse.jgit.dircache.DirCacheIterator; import org.eclipse.jgit.junit.RepositoryTestCase; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.patch.FileHeader; import org.eclipse.jgit.patch.HunkHeader; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.treewalk.FileTreeIterator; import org.eclipse.jgit.treewalk.filter.PathFilter; import org.eclipse.jgit.util.FileUtils; @@ -485,6 +487,102 @@ public class DiffFormatterTest extends RepositoryTestCase { } } + @Test + public void testDiffAutoCrlfSmallFile() throws Exception { + String content = "01234\r\n01234\r\n01234\r\n"; + String expectedDiff = "diff --git a/test.txt b/test.txt\n" + + "index fe25983..a44a032 100644\n" // + + "--- a/test.txt\n" // + + "+++ b/test.txt\n" // + + "@@ -1,3 +1,4 @@\n" // + + " 01234\n" // + + "+ABCD\n" // + + " 01234\n" // + + " 01234\n"; + doAutoCrLfTest(content, expectedDiff); + } + + @Test + public void testDiffAutoCrlfMediumFile() throws Exception { + String content = mediumCrLfString(); + String expectedDiff = "diff --git a/test.txt b/test.txt\n" + + "index 215c502..c10f08c 100644\n" // + + "--- a/test.txt\n" // + + "+++ b/test.txt\n" // + + "@@ -1,4 +1,5 @@\n" // + + " 01234567\n" // + + "+ABCD\n" // + + " 01234567\n" // + + " 01234567\n" // + + " 01234567\n"; + doAutoCrLfTest(content, expectedDiff); + } + + @Test + public void testDiffAutoCrlfLargeFile() throws Exception { + String content = largeCrLfString(); + String expectedDiff = "diff --git a/test.txt b/test.txt\n" + + "index 7014942..c0487a7 100644\n" // + + "--- a/test.txt\n" // + + "+++ b/test.txt\n" // + + "@@ -1,4 +1,5 @@\n" + + " 012345678901234567890123456789012345678901234567\n" + + "+ABCD\n" + + " 012345678901234567890123456789012345678901234567\n" + + " 012345678901234567890123456789012345678901234567\n" + + " 012345678901234567890123456789012345678901234567\n"; + doAutoCrLfTest(content, expectedDiff); + } + + private void doAutoCrLfTest(String content, String expectedDiff) + throws Exception { + FileBasedConfig config = db.getConfig(); + config.setString(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_AUTOCRLF, "true"); + config.save(); + commitFile("test.txt", content, "master"); + // Insert a line into content + int i = content.indexOf('\n'); + content = content.substring(0, i + 1) + "ABCD\r\n" + + content.substring(i + 1); + writeTrashFile("test.txt", content); + // Create the patch + try (ByteArrayOutputStream os = new ByteArrayOutputStream(); + DiffFormatter dfmt = new DiffFormatter( + new BufferedOutputStream(os))) { + dfmt.setRepository(db); + dfmt.format(new DirCacheIterator(db.readDirCache()), + new FileTreeIterator(db)); + dfmt.flush(); + + String actual = os.toString("UTF-8"); + + assertEquals(expectedDiff, actual); + } + } + + private static String largeCrLfString() { + String line = "012345678901234567890123456789012345678901234567\r\n"; + StringBuilder builder = new StringBuilder( + 2 * RawText.FIRST_FEW_BYTES); + while (builder.length() < 2 * RawText.FIRST_FEW_BYTES) { + builder.append(line); + } + return builder.toString(); + } + + private static String mediumCrLfString() { + // Create a CR-LF string longer than RawText.FIRST_FEW_BYTES whose + // canonical representation is shorter than RawText.FIRST_FEW_BYTES. + String line = "01234567\r\n"; // 10 characters + StringBuilder builder = new StringBuilder( + RawText.FIRST_FEW_BYTES + line.length()); + while (builder.length() <= RawText.FIRST_FEW_BYTES) { + builder.append(line); + } + return builder.toString(); + } + private static String makeDiffHeader(String pathA, String pathB, ObjectId aId, ObjectId bId) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java index 5ede3ea6d..f3e986a5e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java @@ -170,10 +170,11 @@ public abstract class ContentSource { @Override public ObjectLoader open(String path, ObjectId id) throws IOException { seek(path); + long entrySize = ptr.getEntryContentLength(); return new ObjectLoader() { @Override public long getSize() { - return ptr.getEntryLength(); + return entrySize; } @Override @@ -184,7 +185,7 @@ public abstract class ContentSource { @Override public ObjectStream openStream() throws MissingObjectException, IOException { - long contentLength = ptr.getEntryContentLength(); + long contentLength = entrySize; InputStream in = ptr.openEntryStream(); in = new BufferedInputStream(in); return new ObjectStream.Filter(getType(), contentLength, in); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/RawText.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/RawText.java index 1c9807493..ec88ce4ff 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/RawText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/RawText.java @@ -74,7 +74,7 @@ public class RawText extends Sequence { public static final RawText EMPTY_TEXT = new RawText(new byte[0]); /** Number of bytes to check for heuristics in {@link #isBinary(byte[])} */ - private static final int FIRST_FEW_BYTES = 8000; + static final int FIRST_FEW_BYTES = 8000; /** The file content for this sequence. */ protected final byte[] content;