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 <marc.strapetz@syntevo.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

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 be1d8326c86a2ef225eceeef8b4f37cf87d7d4b2..1293bef9a83e405e810aa93cb751b0febf0131b2 100644 (file)
--- 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<ObjectId> 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<Callable<ObjectId>> blobInsertersForTheSameFanOutDir(
                        final ObjectDirectory dir) {
                Callable<ObjectId> callable = new Callable<ObjectId>() {

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 b9b74bfc21d0aa141ceb8004cbb0a992cedd4e09..eb6f3a0aae3d974a8754368544772de2f4ef4719 100644 (file)
--- 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 ada5bf71163c3c3514588b01ed8ca77f43cf31a8..244356c13b71d630b770f91e295641a35be31e22 100644 (file)
--- 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 ea80528518f5e94a7b4d0d88bfc5a45105a86d9d..c4db19a7ea23afdbdf48f2c49be8997a68acfafc 100644 (file)
--- 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();
                        }