SubmoduleValidator: Always throw SubmoduleValidationException

The fsck test needs more detail about the error than an IOException
with an explanatory message.

Add an error identifier to the SubmoduleValidatorException and make
it the only throwable exception when parsing a file.

Change-Id: Ic3f0955b497e1681b25e681e1282e876cdf3d2c5
Signed-off-by: Ivan Frade <ifrade@google.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
index c959f6c49771f0ceb8d75870ce46c7607a3161c7..dfa50b6bb64bdc85d76d7bdbcc9f8462b9f42d46 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
@@ -492,9 +492,8 @@ public class ReceivePackAdvertiseRefsHookTest extends LocalDiskRepositoryTestCas
                assertSame(PacketLineIn.END, r.readString());
 
                String errorLine = r.readString();
-               System.out.println(errorLine);
-               assertTrue(errorLine.startsWith(
-                               "unpack error Invalid submodule URL '-"));
+               assertTrue(errorLine.startsWith("unpack error"));
+               assertTrue(errorLine.contains("Invalid submodule URL '-"));
                assertEquals("ng refs/heads/s n/a (unpacker error)", r.readString());
                assertSame(PacketLineIn.END, r.readString());
        }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/submodule/SubmoduleValidator.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/submodule/SubmoduleValidator.java
index 365163157359b29caf7d04839330102ebafa8c94..7b872b18602a0fc915b26c97c8a81eb9348c46fc 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/submodule/SubmoduleValidator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/submodule/SubmoduleValidator.java
@@ -45,13 +45,17 @@ package org.eclipse.jgit.internal.submodule;
 import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PATH;
 import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_URL;
 import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_SUBMODULE_SECTION;
+import static org.eclipse.jgit.lib.ObjectChecker.ErrorType.GITMODULES_NAME;
+import static org.eclipse.jgit.lib.ObjectChecker.ErrorType.GITMODULES_PARSE;
+import static org.eclipse.jgit.lib.ObjectChecker.ErrorType.GITMODULES_PATH;
+import static org.eclipse.jgit.lib.ObjectChecker.ErrorType.GITMODULES_URL;
 
-import java.io.IOException;
 import java.text.MessageFormat;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectChecker;
 
 /**
  * Validations for the git submodule fields (name, path, uri).
@@ -66,15 +70,30 @@ public class SubmoduleValidator {
         */
        public static class SubmoduleValidationException extends Exception {
 
+               private static final long serialVersionUID = 1L;
+
+               private final ObjectChecker.ErrorType fsckMessageId;
+
                /**
                 * @param message
                 *            Description of the problem
+                * @param fsckMessageId
+                *            Error identifier, following the git fsck fsck.<msg-id>
+                *            format
                 */
-               public SubmoduleValidationException(String message) {
+               SubmoduleValidationException(String message,
+                               ObjectChecker.ErrorType fsckMessageId) {
                        super(message);
+                       this.fsckMessageId = fsckMessageId;
                }
 
-               private static final long serialVersionUID = 1L;
+
+               /**
+                * @return the error identifier
+                */
+               public ObjectChecker.ErrorType getFsckMessageId() {
+                       return fsckMessageId;
+               }
        }
 
        /**
@@ -100,13 +119,15 @@ public class SubmoduleValidator {
                        // Since Path class is platform dependent, we manually check '/' and
                        // '\\' patterns here.
                        throw new SubmoduleValidationException(MessageFormat
-                                       .format(JGitText.get().invalidNameContainsDotDot, name));
+                                       .format(JGitText.get().invalidNameContainsDotDot, name),
+                                       GITMODULES_NAME);
                }
 
                if (name.startsWith("-")) { //$NON-NLS-1$
                        throw new SubmoduleValidationException(
                                        MessageFormat.format(
-                                                       JGitText.get().submoduleNameInvalid, name));
+                                                       JGitText.get().submoduleNameInvalid, name),
+                                       GITMODULES_NAME);
                }
        }
 
@@ -123,7 +144,8 @@ public class SubmoduleValidator {
                if (uri.startsWith("-")) { //$NON-NLS-1$
                        throw new SubmoduleValidationException(
                                        MessageFormat.format(
-                                                       JGitText.get().submoduleUrlInvalid, uri));
+                                                       JGitText.get().submoduleUrlInvalid, uri),
+                                       GITMODULES_URL);
                }
        }
 
@@ -140,19 +162,22 @@ public class SubmoduleValidator {
                if (path.startsWith("-")) { //$NON-NLS-1$
                        throw new SubmoduleValidationException(
                                        MessageFormat.format(
-                                                       JGitText.get().submodulePathInvalid, path));
+                                                       JGitText.get().submodulePathInvalid, path),
+                                       GITMODULES_PATH);
                }
        }
 
        /**
+        * Validate a .gitmodules file
+        *
         * @param gitModulesContents
         *            Contents of a .gitmodule file. They will be parsed internally.
-        * @throws IOException
-        *             If the contents
+        * @throws SubmoduleValidationException
+        *             if the contents don't look like a configuration file or field
+        *             values are not valid
         */
        public static void assertValidGitModulesFile(String gitModulesContents)
-                       throws IOException {
-               // Validate .gitmodules file
+                       throws SubmoduleValidationException {
                Config c = new Config();
                try {
                        c.fromText(gitModulesContents);
@@ -173,12 +198,9 @@ public class SubmoduleValidator {
                                }
                        }
                } catch (ConfigInvalidException e) {
-                       throw new IOException(
-                                       MessageFormat.format(
-                                                       JGitText.get().invalidGitModules,
-                                                       e));
-               } catch (SubmoduleValidationException e) {
-                       throw new IOException(e.getMessage(), e);
+                       throw new SubmoduleValidationException(
+                                       JGitText.get().invalidGitModules,
+                                       GITMODULES_PARSE);
                }
        }
 }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java
index d37fb21c93271beae17189fd74f4bdff869d0a14..825f81e69155d228469f0e15722f10e996638d00 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java
@@ -173,6 +173,13 @@ public class ObjectChecker {
                /***/ BAD_TIMEZONE,
                /***/ MISSING_EMAIL,
                /***/ MISSING_SPACE_BEFORE_DATE,
+               /***/ GITMODULES_BLOB,
+               /***/ GITMODULES_LARGE,
+               /***/ GITMODULES_NAME,
+               /***/ GITMODULES_PARSE,
+               /***/ GITMODULES_PATH,
+               /***/ GITMODULES_SYMLINK,
+               /***/ GITMODULES_URL,
                /***/ UNKNOWN_TYPE,
 
                // These are unique to JGit.

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
index d3419bc2014d746696f63bb2be83ee06ac17cae5..03763368a8e7e2767157c9ebec66edfe6e5e3537 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
@@ -72,12 +72,14 @@ import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jgit.annotations.Nullable;
 import org.eclipse.jgit.errors.InvalidObjectIdException;
+import org.eclipse.jgit.errors.LargeObjectException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.errors.PackProtocolException;
 import org.eclipse.jgit.errors.TooLargePackException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.file.PackLock;
 import org.eclipse.jgit.internal.submodule.SubmoduleValidator;
+import org.eclipse.jgit.internal.submodule.SubmoduleValidator.SubmoduleValidationException;
 import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.BatchRefUpdate;
 import org.eclipse.jgit.lib.Config;
@@ -1528,8 +1530,12 @@ public abstract class BaseReceivePack {
                        AnyObjectId blobId = entry.getBlobId();
                        ObjectLoader blob = odb.open(blobId, Constants.OBJ_BLOB);
 
-                       SubmoduleValidator.assertValidGitModulesFile(
-                                       new String(blob.getBytes(), UTF_8));
+                       try {
+                               SubmoduleValidator.assertValidGitModulesFile(
+                                               new String(blob.getBytes(), UTF_8));
+                       } catch (LargeObjectException | SubmoduleValidationException e) {
+                               throw new IOException(e);
+                       }
                }
        }