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>

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());
 	}

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);
 		}
 	}
 }

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.

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);
+			}
 		}
 	}