Rewrite push certificate parsing

- Consistently return structured data, such as actual ReceiveCommands,
  which is more useful for callers that are doing things other than
  verifying the signature, e.g. recording the set of commands.
- Store the certificate version field, as this is required to be part
  of the signed payload.
- Add a toText() method to recreate the actual payload for signature
  verification. This requires keeping track of the un-chomped command
  strings from the original protocol stream.
- Separate the parser from the certificate itself, so the actual
  PushCertificate object can be immutable. Make a fair attempt at deep
  immutability, but this is not possible with the current mutable
  ReceiveCommand structure.
- Use more detailed error messages that don't involve NON-NLS strings.
- Document null return values more thoroughly. Instead of having the
  undocumented behavior of throwing NPE from certain methods if they
  are not first guarded by enabled(), eliminate enabled() and return
  null from those methods.
- Add tests for parsing a push cert from a section of pkt-line stream
  using a real live stream captured with Wireshark (which, it should
  be noted, uncovered several simply incorrect statements in C git's
  Documentation/technical/pack-protocol.txt).

This is a slightly breaking API change to classes that were
technically public and technically released in 4.0. However, it is
highly unlikely that people were actually depending on public
behavior, since there were no public methods to create
PushCertificates with anything other than null field values, or a
PushCertificateParser that did anything other than infinite loop or
throw exceptions when reading.

Change-Id: I5382193347a8eb1811032d9b32af9651871372d0

1 files changed, 125 insertions, 25 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java
index 8ee4c17bf2..2eda2b7138 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java
@@ -43,21 +43,26 @@
 
 package org.eclipse.jgit.transport;
 
+import static org.eclipse.jgit.transport.PushCertificateParser.NONCE;
+import static org.eclipse.jgit.transport.PushCertificateParser.PUSHEE;
+import static org.eclipse.jgit.transport.PushCertificateParser.PUSHER;
+import static org.eclipse.jgit.transport.PushCertificateParser.VERSION;
+
+import java.text.MessageFormat;
+import java.util.List;
+
+import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.lib.PersonIdent;
+
 /**
  * The required information to verify the push.
+ * <p>
+ * A valid certificate will not return null from any getter methods; callers may
+ * assume that any null value indicates a missing or invalid certificate.
  *
  * @since 4.0
  */
 public class PushCertificate {
-	/** The tuple "name &lt;email&gt;" as presented in the push certificate. */
-	String pusher;
-
-	/** The remote URL the signed push goes to. */
-	String pushee;
-
-	/** What we think about the returned signed nonce. */
-	NonceStatus nonceStatus;
-
 	/** Verification result of the nonce returned during push. */
 	public enum NonceStatus {
 		/** Nonce was not expected, yet client sent one anyway. */
@@ -72,41 +77,136 @@ public class PushCertificate {
 		SLOP
 	}
 
-	String commandList;
-	String signature;
+	private final String version;
+	private final PersonIdent pusher;
+	private final String pushee;
+	private final String nonce;
+	private final NonceStatus nonceStatus;
+	private final List<ReceiveCommand> commands;
+	private final String rawCommands;
+	private final String signature;
+
+	PushCertificate(String version, PersonIdent pusher, String pushee,
+			String nonce, NonceStatus nonceStatus, List<ReceiveCommand> commands,
+			String rawCommands, String signature) {
+		if (version == null || version.isEmpty()) {
+			throw new IllegalArgumentException(MessageFormat.format(
+					JGitText.get().pushCertificateInvalidField, VERSION));
+		}
+		if (pusher == null) {
+			throw new IllegalArgumentException(MessageFormat.format(
+					JGitText.get().pushCertificateInvalidField, PUSHER));
+		}
+		if (pushee == null || pushee.isEmpty()) {
+			throw new IllegalArgumentException(MessageFormat.format(
+					JGitText.get().pushCertificateInvalidField, PUSHEE));
+		}
+		if (nonce == null || nonce.isEmpty()) {
+			throw new IllegalArgumentException(MessageFormat.format(
+					JGitText.get().pushCertificateInvalidField, NONCE));
+		}
+		if (nonceStatus == null) {
+			throw new IllegalArgumentException(MessageFormat.format(
+					JGitText.get().pushCertificateInvalidField,
+					"nonce status")); //$NON-NLS-1$
+		}
+		if (commands == null || commands.isEmpty()
+				|| rawCommands == null || rawCommands.isEmpty()) {
+			throw new IllegalArgumentException(MessageFormat.format(
+					JGitText.get().pushCertificateInvalidField,
+					"command")); //$NON-NLS-1$
+		}
+		if (signature == null || signature.isEmpty()) {
+			throw new IllegalArgumentException(
+					JGitText.get().pushCertificateInvalidSignature);
+		}
+		this.version = version;
+		this.pusher = pusher;
+		this.pushee = pushee;
+		this.nonce = nonce;
+		this.nonceStatus = nonceStatus;
+		this.commands = commands;
+		this.rawCommands = rawCommands;
+		this.signature = signature;
+	}
 
 	/**
-	 * @return the signature, consisting of the lines received between the lines
-	 *         '----BEGIN GPG SIGNATURE-----\n' and the '----END GPG
-	 *         SIGNATURE-----\n'
+	 * @return the certificate version string.
+	 * @since 4.1
 	 */
-	public String getSignature() {
-		return signature;
+	public String getVersion() {
+		return version;
 	}
 
 	/**
-	 * @return the list of commands as one string to be feed into the signature
-	 *         verifier.
+	 * @return the identity of the pusher who signed the cert, as a string.
+	 * @since 4.0
 	 */
-	public String getCommandList() {
-		return commandList;
+	public String getPusher() {
+		return pusher.toExternalString();
 	}
 
 	/**
-	 * @return the tuple "name &lt;email&gt;" as presented by the client in the
-	 *         push certificate.
+	 * @return identity of the pusher who signed the cert.
+	 * @since 4.1
 	 */
-	public String getPusher() {
+	public PersonIdent getPusherIdent() {
 		return pusher;
 	}
 
-	/** @return URL of the repository the push was originally sent to. */
+	/**
+	 * @return URL of the repository the push was originally sent to.
+	 * @since 4.0
+	 */
 	public String getPushee() {
 		return pushee;
 	}
 
-	/** @return verification status of the nonce embedded in the certificate. */
+	/**
+	 * @return the raw nonce value that was presented by the pusher.
+	 * @since 4.0
+	 */
+	public String getNonce() {
+		return nonce;
+	}
+
+	/**
+	 * @return verification status of the nonce embedded in the certificate.
+	 * @since 4.0
+	 */
 	public NonceStatus getNonceStatus() {
 		return nonceStatus;
 	}
+
+	/**
+	 * @return the list of commands as one string to be feed into the signature
+	 *         verifier.
+	 * @since 4.1
+	 */
+	public List<ReceiveCommand> getCommands() {
+		return commands;
+	}
+
+	/**
+	 * @return the raw signature, consisting of the lines received between the
+	 *     lines {@code "----BEGIN GPG SIGNATURE-----\n"} and
+	 *     {@code "----END GPG SIGNATURE-----\n}", exclusive
+	 * @since 4.0
+	 */
+	public String getSignature() {
+		return signature;
+	}
+
+	/** @return text payload of the certificate for the signature verifier. */
+	public String toText() {
+		return new StringBuilder()
+				.append(VERSION).append(' ').append(version).append('\n')
+				.append(PUSHER).append(' ').append(pusher.toExternalString())
+				.append('\n')
+				.append(PUSHEE).append(' ').append(pushee).append('\n')
+				.append(NONCE).append(' ').append(nonce).append('\n')
+				.append('\n')
+				.append(rawCommands)
+				.toString();
+	}
 }