Use constants for git packet protocol line identifiers

Introduce named constants for packet line headers and use them instead
of direct string literals everywhere. This not only makes the code more
readable because we don't need NON-NLS markers, it also makes it more
robust since we can use the length of these constants instead of magic
numbers.

Change-Id: Ie4b7239e0b479a68a2dc23e6e05f25061d481a31
Signed-off-by: Thomas Wolf <twolf@apache.org>

6 files changed, 234 insertions, 79 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
index 2aecf63ada..be36d2b834 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
@@ -12,6 +12,18 @@
 
 package org.eclipse.jgit.transport;
 
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DELIM;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DEEPEN;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DEEPEN_NOT;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DEEPEN_SINCE;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DONE;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_END;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_ERR;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_HAVE;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_SHALLOW;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_UNSHALLOW;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_WANT;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -483,7 +495,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 		clearState();
 		String line = pckIn.readString();
 		// If we sent a done, we may have an error reply here.
-		if (sentDone && line.startsWith("ERR ")) { //$NON-NLS-1$
+		if (sentDone && line.startsWith(PACKET_ERR)) {
 			throw new RemoteRepositoryException(uri, line.substring(4));
 		}
 
@@ -491,7 +503,8 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 			line = handleShallowUnshallow(shallowCommits, pckIn);
 			if (!PacketLineIn.isDelimiter(line)) {
 				throw new PackProtocolException(MessageFormat
-						.format(JGitText.get().expectedGot, "0001", line)); //$NON-NLS-1$
+						.format(JGitText.get().expectedGot, PACKET_DELIM,
+								line));
 			}
 			line = pckIn.readString();
 		}
@@ -532,7 +545,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 			if (c == null) {
 				break;
 			}
-			output.writeString("have " + c.getId().name() + '\n'); //$NON-NLS-1$
+			output.writeString(PACKET_HAVE + c.getId().name() + '\n');
 			n++;
 			if (n % 10 == 0 && monitor.isCancelled()) {
 				throw new CancelledException();
@@ -543,7 +556,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 				|| (fetchState.hadAcks
 						&& fetchState.havesWithoutAck > MAX_HAVES)
 				|| fetchState.havesTotal > maxHaves) {
-			output.writeString("done\n"); //$NON-NLS-1$
+			output.writeString(PACKET_DONE + '\n');
 			output.end();
 			return true;
 		}
@@ -610,11 +623,12 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 		if (gotReady) {
 			if (!PacketLineIn.isDelimiter(line)) {
 				throw new PackProtocolException(MessageFormat
-						.format(JGitText.get().expectedGot, "0001", line)); //$NON-NLS-1$
+						.format(JGitText.get().expectedGot, PACKET_DELIM,
+								line));
 			}
 		} else if (!PacketLineIn.isEnd(line)) {
 			throw new PackProtocolException(MessageFormat
-					.format(JGitText.get().expectedGot, "0000", line)); //$NON-NLS-1$
+					.format(JGitText.get().expectedGot, PACKET_END, line));
 		}
 		return gotReady;
 	}
@@ -726,8 +740,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 			}
 
 			final StringBuilder line = new StringBuilder(46);
-			line.append("want "); //$NON-NLS-1$
-			line.append(objectId.name());
+			line.append(PACKET_WANT).append(objectId.name());
 			if (first && TransferConfig.ProtocolVersion.V0
 					.equals(getProtocolVersion())) {
 				line.append(enableCapabilities());
@@ -836,7 +849,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 			}
 
 			ObjectId o = c.getId();
-			pckOut.writeString("have " + o.name() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
+			pckOut.writeString(PACKET_HAVE + o.name() + '\n');
 			havesSent++;
 			havesSinceLastContinue++;
 
@@ -939,7 +952,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 			// loop above while in the middle of a request. This allows us
 			// to just write done immediately.
 			//
-			pckOut.writeString("done\n"); //$NON-NLS-1$
+			pckOut.writeString(PACKET_DONE + '\n');
 			pckOut.flush();
 		}
 
@@ -956,7 +969,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 			String line = handleShallowUnshallow(shallowCommits, pckIn);
 			if (!PacketLineIn.isEnd(line)) {
 				throw new PackProtocolException(MessageFormat
-						.format(JGitText.get().expectedGot, "0000", line)); //$NON-NLS-1$
+						.format(JGitText.get().expectedGot, PACKET_END, line));
 			}
 		}
 
@@ -1041,7 +1054,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 	private void markCommon(RevObject obj, AckNackResult anr, boolean useState)
 			throws IOException {
 		if (useState && anr == AckNackResult.ACK_COMMON && !obj.has(STATE)) {
-			pckState.writeString("have " + obj.name() + '\n'); //$NON-NLS-1$
+			pckState.writeString(PACKET_HAVE + obj.name() + '\n');
 			obj.add(STATE);
 		}
 		obj.add(COMMON);
@@ -1074,41 +1087,46 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 		}
 	}
 
-	private void sendShallow(Set<ObjectId> shallowCommits, PacketLineOut output) throws IOException {
+	private void sendShallow(Set<ObjectId> shallowCommits, PacketLineOut output)
+			throws IOException {
 		for (ObjectId shallowCommit : shallowCommits) {
-			output.writeString("shallow " + shallowCommit.name()); //$NON-NLS-1$
+			output.writeString(PACKET_SHALLOW + shallowCommit.name());
 		}
 
 		if (depth != null) {
-			output.writeString("deepen " + depth); //$NON-NLS-1$
+			output.writeString(PACKET_DEEPEN + depth);
 		}
 
 		if (deepenSince != null) {
-			output.writeString("deepen-since " + deepenSince.getEpochSecond()); //$NON-NLS-1$
+			output.writeString(
+					PACKET_DEEPEN_SINCE + deepenSince.getEpochSecond());
 		}
 
 		if (deepenNots != null) {
 			for (String deepenNotRef : deepenNots) {
-				output.writeString("deepen-not " + deepenNotRef); //$NON-NLS-1$
+				output.writeString(PACKET_DEEPEN_NOT + deepenNotRef);
 			}
 		}
 	}
 
-	private String handleShallowUnshallow(Set<ObjectId> advertisedShallowCommits, PacketLineIn input)
+	private String handleShallowUnshallow(
+			Set<ObjectId> advertisedShallowCommits, PacketLineIn input)
 			throws IOException {
 		String line = input.readString();
 		ObjectDatabase objectDatabase = local.getObjectDatabase();
-		HashSet<ObjectId> newShallowCommits = new HashSet<>(advertisedShallowCommits);
+		HashSet<ObjectId> newShallowCommits = new HashSet<>(
+				advertisedShallowCommits);
 		while (!PacketLineIn.isDelimiter(line) && !PacketLineIn.isEnd(line)) {
-			if (line.startsWith("shallow ")) { //$NON-NLS-1$
+			if (line.startsWith(PACKET_SHALLOW)) {
 				newShallowCommits.add(ObjectId
-						.fromString(line.substring("shallow ".length()))); //$NON-NLS-1$
-			} else if (line.startsWith("unshallow ")) { //$NON-NLS-1$
+						.fromString(line.substring(PACKET_SHALLOW.length())));
+			} else if (line.startsWith(PACKET_UNSHALLOW)) {
 				ObjectId unshallow = ObjectId
-						.fromString(line.substring("unshallow ".length())); //$NON-NLS-1$
+						.fromString(line.substring(PACKET_UNSHALLOW.length()));
 				if (!advertisedShallowCommits.contains(unshallow)) {
-					throw new PackProtocolException(MessageFormat.format(JGitText.get()
-							.notShallowedUnshallow, unshallow.name()));
+					throw new PackProtocolException(MessageFormat.format(
+							JGitText.get().notShallowedUnshallow,
+							unshallow.name()));
 				}
 				newShallowCommits.remove(unshallow);
 			}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java
index 24ea552ba6..be14e92d07 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java
@@ -343,6 +343,106 @@ public final class GitProtocolConstants {
 	 */
 	public static final String VERSION_2_REQUEST = "version=2"; //$NON-NLS-1$
 
+	/**
+	 * The flush packet.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_FLUSH = "0000"; //$NON-NLS-1$
+
+	/**
+	 * An alias for {@link #PACKET_FLUSH}. "Flush" is the name used in the C git
+	 * documentation; the Java implementation calls this "end" in several
+	 * places.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_END = PACKET_FLUSH;
+
+	/**
+	 * The delimiter packet in protocol V2.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_DELIM = "0001"; //$NON-NLS-1$
+
+	/**
+	 * A "deepen" packet beginning.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_DEEPEN = "deepen "; //$NON-NLS-1$
+
+	/**
+	 * A "deepen-not" packet beginning.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_DEEPEN_NOT = "deepen-not "; //$NON-NLS-1$
+
+	/**
+	 * A "deepen-since" packet beginning.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_DEEPEN_SINCE = "deepen-since "; //$NON-NLS-1$
+
+	/**
+	 * An "ACK" packet beginning.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_ACK = "ACK "; //$NON-NLS-1$
+
+	/**
+	 * A "done" packet beginning.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_DONE = "done"; //$NON-NLS-1$
+
+	/**
+	 * A "ERR" packet beginning.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_ERR = "ERR "; //$NON-NLS-1$
+
+	/**
+	 * A "have" packet beginning.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_HAVE = "have "; //$NON-NLS-1$
+
+	/**
+	 * A "shallow" packet beginning.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_SHALLOW = OPTION_SHALLOW + ' ';
+
+	/**
+	 * A "shallow" packet beginning.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_UNSHALLOW = "unshallow "; //$NON-NLS-1$
+
+	/**
+	 * A "want" packet beginning.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_WANT = "want "; //$NON-NLS-1$
+
+	/**
+	 * A "want-ref" packet beginning.
+	 *
+	 * @since 6.3
+	 */
+	public static final String PACKET_WANT_REF = OPTION_WANT_REF + ' ';
+
 	enum MultiAck {
 		OFF, CONTINUE, DETAILED;
 	}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java
index 4ddcb99419..21a492577f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java
@@ -10,6 +10,11 @@
 package org.eclipse.jgit.transport;
 
 import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_FILTER;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DEEPEN;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DEEPEN_NOT;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DEEPEN_SINCE;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_SHALLOW;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_WANT;
 
 import java.io.EOFException;
 import java.io.IOException;
@@ -70,8 +75,9 @@ final class ProtocolV0Parser {
 				break;
 			}
 
-			if (line.startsWith("deepen ")) { //$NON-NLS-1$
-				int depth = Integer.parseInt(line.substring(7));
+			if (line.startsWith(PACKET_DEEPEN)) {
+				int depth = Integer
+						.parseInt(line.substring(PACKET_DEEPEN.length()));
 				if (depth <= 0) {
 					throw new PackProtocolException(
 							MessageFormat.format(JGitText.get().invalidDepth,
@@ -89,8 +95,9 @@ final class ProtocolV0Parser {
 				continue;
 			}
 
-			if (line.startsWith("deepen-not ")) { //$NON-NLS-1$
-				reqBuilder.addDeepenNot(line.substring(11));
+			if (line.startsWith(PACKET_DEEPEN_NOT)) {
+				reqBuilder.addDeepenNot(
+						line.substring(PACKET_DEEPEN_NOT.length()));
 				if (reqBuilder.getDepth() != 0) {
 					throw new PackProtocolException(
 							JGitText.get().deepenNotWithDeepen);
@@ -98,9 +105,10 @@ final class ProtocolV0Parser {
 				continue;
 			}
 
-			if (line.startsWith("deepen-since ")) { //$NON-NLS-1$
+			if (line.startsWith(PACKET_DEEPEN_SINCE)) {
 				// TODO: timestamps should be long
-				int ts = Integer.parseInt(line.substring(13));
+				int ts = Integer
+						.parseInt(line.substring(PACKET_DEEPEN_SINCE.length()));
 				if (ts <= 0) {
 					throw new PackProtocolException(MessageFormat
 							.format(JGitText.get().invalidTimestamp, line));
@@ -113,9 +121,10 @@ final class ProtocolV0Parser {
 				continue;
 			}
 
-			if (line.startsWith("shallow ")) { //$NON-NLS-1$
+			if (line.startsWith(PACKET_SHALLOW)) {
 				reqBuilder.addClientShallowCommit(
-						ObjectId.fromString(line.substring(8)));
+						ObjectId.fromString(
+								line.substring(PACKET_SHALLOW.length())));
 				continue;
 			}
 
@@ -133,7 +142,7 @@ final class ProtocolV0Parser {
 				continue;
 			}
 
-			if (!line.startsWith("want ") || line.length() < 45) { //$NON-NLS-1$
+			if (!line.startsWith(PACKET_WANT) || line.length() < 45) {
 				throw new PackProtocolException(MessageFormat
 						.format(JGitText.get().expectedGot, "want", line)); //$NON-NLS-1$
 			}
@@ -147,7 +156,8 @@ final class ProtocolV0Parser {
 				}
 			}
 
-			reqBuilder.addWantId(ObjectId.fromString(line.substring(5)));
+			reqBuilder.addWantId(
+					ObjectId.fromString(line.substring(PACKET_WANT.length())));
 			isFirst = false;
 		}
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java
index e502831a2b..b38deb69c0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java
@@ -20,7 +20,14 @@ import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDEBAND_AL
 import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND_64K;
 import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_THIN_PACK;
 import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_WAIT_FOR_DONE;
-import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_WANT_REF;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DEEPEN;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DEEPEN_NOT;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DEEPEN_SINCE;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DONE;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_HAVE;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_SHALLOW;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_WANT;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_WANT_REF;
 
 import java.io.IOException;
 import java.text.MessageFormat;
@@ -115,15 +122,17 @@ final class ProtocolV2Parser {
 
 		boolean filterReceived = false;
 		for (String line2 : pckIn.readStrings()) {
-			if (line2.startsWith("want ")) { //$NON-NLS-1$
-				reqBuilder.addWantId(ObjectId.fromString(line2.substring(5)));
+			if (line2.startsWith(PACKET_WANT)) {
+				reqBuilder.addWantId(ObjectId
+						.fromString(line2.substring(PACKET_WANT.length())));
 			} else if (transferConfig.isAllowRefInWant()
-					&& line2.startsWith(OPTION_WANT_REF + " ")) { //$NON-NLS-1$
+					&& line2.startsWith(PACKET_WANT_REF)) {
 				reqBuilder.addWantedRef(
-						line2.substring(OPTION_WANT_REF.length() + 1));
-			} else if (line2.startsWith("have ")) { //$NON-NLS-1$
-				reqBuilder.addPeerHas(ObjectId.fromString(line2.substring(5)));
-			} else if (line2.equals("done")) { //$NON-NLS-1$
+						line2.substring(PACKET_WANT_REF.length()));
+			} else if (line2.startsWith(PACKET_HAVE)) {
+				reqBuilder.addPeerHas(ObjectId
+						.fromString(line2.substring(PACKET_HAVE.length())));
+			} else if (line2.equals(PACKET_DONE)) {
 				reqBuilder.setDoneReceived();
 			} else if (line2.equals(OPTION_WAIT_FOR_DONE)) {
 				reqBuilder.setWaitForDone();
@@ -135,11 +144,13 @@ final class ProtocolV2Parser {
 				reqBuilder.addClientCapability(OPTION_INCLUDE_TAG);
 			} else if (line2.equals(OPTION_OFS_DELTA)) {
 				reqBuilder.addClientCapability(OPTION_OFS_DELTA);
-			} else if (line2.startsWith("shallow ")) { //$NON-NLS-1$
+			} else if (line2.startsWith(PACKET_SHALLOW)) {
 				reqBuilder.addClientShallowCommit(
-						ObjectId.fromString(line2.substring(8)));
-			} else if (line2.startsWith("deepen ")) { //$NON-NLS-1$
-				int parsedDepth = Integer.parseInt(line2.substring(7));
+						ObjectId.fromString(
+								line2.substring(PACKET_SHALLOW.length())));
+			} else if (line2.startsWith(PACKET_DEEPEN)) {
+				int parsedDepth = Integer
+						.parseInt(line2.substring(PACKET_DEEPEN.length()));
 				if (parsedDepth <= 0) {
 					throw new PackProtocolException(
 							MessageFormat.format(JGitText.get().invalidDepth,
@@ -154,16 +165,18 @@ final class ProtocolV2Parser {
 							JGitText.get().deepenNotWithDeepen);
 				}
 				reqBuilder.setDepth(parsedDepth);
-			} else if (line2.startsWith("deepen-not ")) { //$NON-NLS-1$
-				reqBuilder.addDeepenNot(line2.substring(11));
+			} else if (line2.startsWith(PACKET_DEEPEN_NOT)) {
+				reqBuilder.addDeepenNot(
+						line2.substring(PACKET_DEEPEN_NOT.length()));
 				if (reqBuilder.getDepth() != 0) {
 					throw new PackProtocolException(
 							JGitText.get().deepenNotWithDeepen);
 				}
 			} else if (line2.equals(OPTION_DEEPEN_RELATIVE)) {
 				reqBuilder.addClientCapability(OPTION_DEEPEN_RELATIVE);
-			} else if (line2.startsWith("deepen-since ")) { //$NON-NLS-1$
-				int ts = Integer.parseInt(line2.substring(13));
+			} else if (line2.startsWith(PACKET_DEEPEN_SINCE)) {
+				int ts = Integer.parseInt(
+						line2.substring(PACKET_DEEPEN_SINCE.length()));
 				if (ts <= 0) {
 					throw new PackProtocolException(MessageFormat
 							.format(JGitText.get().invalidTimestamp, line2));
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
index 2542105c07..b70eedca63 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
@@ -20,6 +20,8 @@ import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_QUIET;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REPORT_STATUS;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_SIDE_BAND_64K;
 import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_AGENT;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_ERR;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_SHALLOW;
 import static org.eclipse.jgit.transport.SideBandOutputStream.CH_DATA;
 import static org.eclipse.jgit.transport.SideBandOutputStream.CH_ERROR;
 import static org.eclipse.jgit.transport.SideBandOutputStream.CH_PROGRESS;
@@ -1247,7 +1249,7 @@ public class ReceivePack {
 	public void sendAdvertisedRefs(RefAdvertiser adv)
 			throws IOException, ServiceMayNotContinueException {
 		if (advertiseError != null) {
-			adv.writeOne("ERR " + advertiseError); //$NON-NLS-1$
+			adv.writeOne(PACKET_ERR + advertiseError);
 			return;
 		}
 
@@ -1255,7 +1257,7 @@ public class ReceivePack {
 			advertiseRefsHook.advertiseRefs(this);
 		} catch (ServiceMayNotContinueException fail) {
 			if (fail.getMessage() != null) {
-				adv.writeOne("ERR " + fail.getMessage()); //$NON-NLS-1$
+				adv.writeOne(PACKET_ERR + fail.getMessage());
 				fail.setOutput();
 			}
 			throw fail;
@@ -1339,8 +1341,9 @@ public class ReceivePack {
 					break;
 				}
 
-				if (line.length() >= 48 && line.startsWith("shallow ")) { //$NON-NLS-1$
-					parseShallow(line.substring(8, 48));
+				int len = PACKET_SHALLOW.length() + 40;
+				if (line.length() >= len && line.startsWith(PACKET_SHALLOW)) {
+					parseShallow(line.substring(PACKET_SHALLOW.length(), len));
 					continue;
 				}
 
@@ -1795,9 +1798,9 @@ public class ReceivePack {
 			@Override
 			void sendString(String s) throws IOException {
 				if (reportStatus) {
-					pckOut.writeString(s + "\n"); //$NON-NLS-1$
+					pckOut.writeString(s + '\n');
 				} else if (msgOut != null) {
-					msgOut.write(Constants.encode(s + "\n")); //$NON-NLS-1$
+					msgOut.write(Constants.encode(s + '\n'));
 				}
 			}
 		};
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index 409161d58b..65dbf12b2f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -36,6 +36,12 @@ import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND;
 import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND_64K;
 import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_THIN_PACK;
 import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_WAIT_FOR_DONE;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_ACK;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_DONE;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_ERR;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_HAVE;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_SHALLOW;
+import static org.eclipse.jgit.transport.GitProtocolConstants.PACKET_UNSHALLOW;
 import static org.eclipse.jgit.transport.GitProtocolConstants.VERSION_2_REQUEST;
 import static org.eclipse.jgit.util.RefMap.toRefMap;
 
@@ -1076,9 +1082,10 @@ public class UploadPack implements Closeable {
 			deepenNots = parseDeepenNots(req.getDeepenNots());
 			if (req.getDepth() != 0 || req.getDeepenSince() != 0 || !req.getDeepenNots().isEmpty()) {
 				computeShallowsAndUnshallows(req, shallow -> {
-					pckOut.writeString("shallow " + shallow.name() + '\n'); //$NON-NLS-1$
+					pckOut.writeString(PACKET_SHALLOW + shallow.name() + '\n');
 				}, unshallow -> {
-					pckOut.writeString("unshallow " + unshallow.name() + '\n'); //$NON-NLS-1$
+					pckOut.writeString(
+							PACKET_UNSHALLOW + unshallow.name() + '\n');
 					unshallowCommits.add(unshallow);
 				}, deepenNots);
 				pckOut.end();
@@ -1227,7 +1234,7 @@ public class UploadPack implements Closeable {
 					GitProtocolConstants.SECTION_ACKNOWLEDGMENTS + '\n');
 			for (ObjectId id : req.getPeerHas()) {
 				if (walk.getObjectReader().has(id)) {
-					pckOut.writeString("ACK " + id.getName() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
+					pckOut.writeString(PACKET_ACK + id.getName() + '\n');
 				}
 			}
 			processHaveLines(req.getPeerHas(), ObjectId.zeroId(),
@@ -1245,12 +1252,13 @@ public class UploadPack implements Closeable {
 			if (mayHaveShallow) {
 				if (sectionSent)
 					pckOut.writeDelim();
-				pckOut.writeString("shallow-info\n"); //$NON-NLS-1$
+				pckOut.writeString(
+						GitProtocolConstants.SECTION_SHALLOW_INFO + '\n');
 				for (ObjectId o : shallowCommits) {
-					pckOut.writeString("shallow " + o.getName() + '\n'); //$NON-NLS-1$
+					pckOut.writeString(PACKET_SHALLOW + o.getName() + '\n');
 				}
 				for (ObjectId o : unshallowCommits) {
-					pckOut.writeString("unshallow " + o.getName() + '\n'); //$NON-NLS-1$
+					pckOut.writeString(PACKET_UNSHALLOW + o.getName() + '\n');
 				}
 				sectionSent = true;
 			}
@@ -1314,7 +1322,7 @@ public class UploadPack implements Closeable {
 						.format(JGitText.get().missingObject, oid.name()), e);
 			}
 
-			pckOut.writeString(oid.getName() + " " + size); //$NON-NLS-1$
+			pckOut.writeString(oid.getName() + ' ' + size);
 		}
 
 		pckOut.end();
@@ -1386,7 +1394,7 @@ public class UploadPack implements Closeable {
 			protocolV2Hook
 					.onCapabilities(CapabilitiesV2Request.builder().build());
 			for (String s : getV2CapabilityAdvertisement()) {
-				pckOut.writeString(s + "\n"); //$NON-NLS-1$
+				pckOut.writeString(s + '\n');
 			}
 			pckOut.end();
 
@@ -1613,7 +1621,7 @@ public class UploadPack implements Closeable {
 	 */
 	public void sendMessage(String what) {
 		try {
-			msgOut.write(Constants.encode(what + "\n")); //$NON-NLS-1$
+			msgOut.write(Constants.encode(what + '\n'));
 		} catch (IOException e) {
 			// Ignore write failures.
 		}
@@ -1720,24 +1728,26 @@ public class UploadPack implements Closeable {
 				if (commonBase.isEmpty() || multiAck != MultiAck.OFF)
 					pckOut.writeString("NAK\n"); //$NON-NLS-1$
 				if (noDone && sentReady) {
-					pckOut.writeString("ACK " + last.name() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
+					pckOut.writeString(PACKET_ACK + last.name() + '\n');
 					return true;
 				}
 				if (!biDirectionalPipe)
 					return false;
 				pckOut.flush();
 
-			} else if (line.startsWith("have ") && line.length() == 45) { //$NON-NLS-1$
-				peerHas.add(ObjectId.fromString(line.substring(5)));
+			} else if (line.startsWith(PACKET_HAVE)
+					&& line.length() == PACKET_HAVE.length() + 40) {
+				peerHas.add(ObjectId
+						.fromString(line.substring(PACKET_HAVE.length())));
 				accumulator.haves++;
-			} else if (line.equals("done")) { //$NON-NLS-1$
+			} else if (line.equals(PACKET_DONE)) {
 				last = processHaveLines(peerHas, last, pckOut, accumulator, Option.NONE);
 
 				if (commonBase.isEmpty())
 					pckOut.writeString("NAK\n"); //$NON-NLS-1$
 
 				else if (multiAck != MultiAck.OFF)
-					pckOut.writeString("ACK " + last.name() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
+					pckOut.writeString(PACKET_ACK + last.name() + '\n');
 
 				return true;
 
@@ -1798,14 +1808,15 @@ public class UploadPack implements Closeable {
 				//
 				switch (multiAck) {
 				case OFF:
-					if (commonBase.size() == 1)
-						out.writeString("ACK " + obj.name() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
+					if (commonBase.size() == 1) {
+						out.writeString(PACKET_ACK + obj.name() + '\n');
+					}
 					break;
 				case CONTINUE:
-					out.writeString("ACK " + obj.name() + " continue\n"); //$NON-NLS-1$ //$NON-NLS-2$
+					out.writeString(PACKET_ACK + obj.name() + " continue\n"); //$NON-NLS-1$
 					break;
 				case DETAILED:
-					out.writeString("ACK " + obj.name() + " common\n"); //$NON-NLS-1$ //$NON-NLS-2$
+					out.writeString(PACKET_ACK + obj.name() + " common\n"); //$NON-NLS-1$
 					break;
 				}
 			}
@@ -1844,11 +1855,11 @@ public class UploadPack implements Closeable {
 							break;
 						case CONTINUE:
 							out.writeString(
-									"ACK " + id.name() + " continue\n"); //$NON-NLS-1$ //$NON-NLS-2$
+									PACKET_ACK + id.name() + " continue\n"); //$NON-NLS-1$
 							break;
 						case DETAILED:
 							out.writeString(
-									"ACK " + id.name() + " ready\n"); //$NON-NLS-1$ //$NON-NLS-2$
+									PACKET_ACK + id.name() + " ready\n"); //$NON-NLS-1$
 							readySent = true;
 							break;
 						}
@@ -1861,7 +1872,7 @@ public class UploadPack implements Closeable {
 		if (multiAck == MultiAck.DETAILED && !didOkToGiveUp
 				&& okToGiveUp()) {
 			ObjectId id = peerHas.get(peerHas.size() - 1);
-			out.writeString("ACK " + id.name() + " ready\n"); //$NON-NLS-1$ //$NON-NLS-2$
+			out.writeString(PACKET_ACK + id.name() + " ready\n"); //$NON-NLS-1$
 			readySent = true;
 		}
 
@@ -2552,7 +2563,7 @@ public class UploadPack implements Closeable {
 		@Override
 		public void writeError(String message) throws IOException {
 			new PacketLineOut(requireNonNull(rawOut))
-					.writeString("ERR " + message + '\n'); //$NON-NLS-1$
+					.writeString(PACKET_ERR + message + '\n');
 		}
 	}
 }