From 1c4b3f8c45fe2bbcc0ea6aa9ffee107312b37310 Mon Sep 17 00:00:00 2001 From: Marc Strapetz Date: Tue, 10 Jan 2017 16:28:54 +0100 Subject: [PATCH] Fix possible InvalidObjectIdException in ObjectDirectory ObjectDirectory.getShallowCommits should throw an IOException instead of an InvalidArgumentException if invalid SHAs are present in .git/shallow (as this file is usually edited by a human). Change-Id: Ia3a39d38f7aec4282109c7698438f0795fbec905 Signed-off-by: Marc Strapetz Signed-off-by: David Pursehouse --- .../storage/file/ObjectDirectoryTest.java | 44 +++++++++++++++++++ .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../storage/file/ObjectDirectory.java | 10 ++++- 4 files changed, 54 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java index be1d8326c..1293bef9a 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java @@ -47,13 +47,18 @@ import static org.junit.Assert.assertTrue; import java.io.File; import java.io.FilenameFilter; +import java.io.IOException; +import java.io.PrintWriter; +import java.text.MessageFormat; import java.util.Collection; import java.util.Collections; +import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.junit.RepositoryTestCase; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; @@ -61,10 +66,15 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.junit.Assume; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; public class ObjectDirectoryTest extends RepositoryTestCase { + @Rule + public ExpectedException expectedEx = ExpectedException.none(); + @Test public void testConcurrentInsertionOfBlobsToTheSameNewFanOutDirectory() throws Exception { @@ -171,6 +181,40 @@ public class ObjectDirectoryTest extends RepositoryTestCase { } } + @Test + public void testShallowFile() + throws Exception { + FileRepository repository = createBareRepository(); + ObjectDirectory dir = repository.getObjectDatabase(); + + String commit = "d3148f9410b071edd4a4c85d2a43d1fa2574b0d2"; + try (PrintWriter writer = new PrintWriter( + new File(repository.getDirectory(), Constants.SHALLOW))) { + writer.println(commit); + } + Set shallowCommits = dir.getShallowCommits(); + assertTrue(shallowCommits.remove(ObjectId.fromString(commit))); + assertTrue(shallowCommits.isEmpty()); + } + + @Test + public void testShallowFileCorrupt() + throws Exception { + FileRepository repository = createBareRepository(); + ObjectDirectory dir = repository.getObjectDatabase(); + + String commit = "X3148f9410b071edd4a4c85d2a43d1fa2574b0d2"; + try (PrintWriter writer = new PrintWriter( + new File(repository.getDirectory(), Constants.SHALLOW))) { + writer.println(commit); + } + + expectedEx.expect(IOException.class); + expectedEx.expectMessage(MessageFormat + .format(JGitText.get().badShallowLine, commit)); + dir.getShallowCommits(); + } + private Collection> blobInsertersForTheSameFanOutDir( final ObjectDirectory dir) { Callable callable = new Callable() { diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index b9b74bfc2..eb6f3a0aa 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -31,6 +31,7 @@ badGroupHeader=Bad group header badObjectType=Bad object type: {0} badRef=Bad ref: {0}: {1} badSectionEntry=Bad section entry: {0} +badShallowLine=Bad shallow line: {0} bareRepositoryNoWorkdirAndIndex=Bare Repository has neither a working tree, nor an index base64InputNotProperlyPadded=Base64 input not properly padded. baseLengthIncorrect=base length incorrect diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index ada5bf711..244356c13 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -90,6 +90,7 @@ public class JGitText extends TranslationBundle { /***/ public String badObjectType; /***/ public String badRef; /***/ public String badSectionEntry; + /***/ public String badShallowLine; /***/ public String bareRepositoryNoWorkdirAndIndex; /***/ public String base64InputNotProperlyPadded; /***/ public String baseLengthIncorrect; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java index ea8052851..c4db19a7e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java @@ -690,8 +690,14 @@ public class ObjectDirectory extends FileObjectDatabase { final BufferedReader reader = open(shallowFile); try { String line; - while ((line = reader.readLine()) != null) - shallowCommitsIds.add(ObjectId.fromString(line)); + while ((line = reader.readLine()) != null) { + try { + shallowCommitsIds.add(ObjectId.fromString(line)); + } catch (IllegalArgumentException ex) { + throw new IOException(MessageFormat + .format(JGitText.get().badShallowLine, line)); + } + } } finally { reader.close(); }