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>

3 files changed, 54 insertions, 19 deletions

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 3651631573..7b872b1860 100644
--- 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 d37fb21c93..825f81e691 100644
--- 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 d3419bc201..03763368a8 100644
--- 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);
+			}
 		}
 	}