threads / patch / 19846

patchDon't NPE in server code if a symbolic reference is incomplete

Subject: [JGIT PATCH] Don't NPE in server code if a symbolic reference is incomplete

## tl;dr

One message between Jun 18, 2009 and Jun 18, 2009. Diffs are folded; open one to read it.

replies: 0people: 1as markdown or json

Shawn O. Pearce· Jun 18, 2009, 02:12 UTC · lore

If a symbolic reference's target doesn't exist, then we still get the Ref instance back from the database but its ObjectId is null. Instead of trying to advertise null as a SHA-1 to the client, skip it and say nothing about that ref.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../org/spearce/jgit/transport/ReceivePack.java    |   45 +++++++++++---------
 .../src/org/spearce/jgit/transport/UploadPack.java |    2 +
 2 files changed, 27 insertions(+), 20 deletions(-)
Show changes to 2 files +27 −20

org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java, org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
index abaf876..c92a903 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
@@ -461,34 +461,39 @@ private void sendAdvertisedRefs() throws IOException {
 		final StringBuilder m = new StringBuilder(100);
 		final char[] idtmp = new char[2 * Constants.OBJECT_ID_LENGTH];
 		final Iterator<Ref> i = RefComparator.sort(refs.values()).iterator();
-		{
-			if (i.hasNext()) {
-				final Ref r = i.next();
-				format(m, idtmp, r.getObjectId(), r.getOrigName());
-			} else {
-				format(m, idtmp, ObjectId.zeroId(), "capabilities^{}");
-			}
-			m.append('\0');
-			m.append(' ');
-			m.append(CAPABILITY_DELETE_REFS);
-			m.append(' ');
-			m.append(CAPABILITY_REPORT_STATUS);
-			if (allowOfsDelta) {
-				m.append(' ');
-				m.append(CAPABILITY_OFS_DELTA);
-			}
-			m.append(' ');
-			writeAdvertisedRef(m);
-		}
-
+		boolean first = true;
 		while (i.hasNext()) {
 			final Ref r = i.next();
+			if (r.getObjectId() == null)
+				continue;
 			format(m, idtmp, r.getObjectId(), r.getOrigName());
+			if (first) {
+				first = false;
+				advertiseCapabilities(m);
+			}
+			writeAdvertisedRef(m);
+		}
+		if (first) {
+			format(m, idtmp, ObjectId.zeroId(), "capabilities^{}");
+			advertiseCapabilities(m);
 			writeAdvertisedRef(m);
 		}
 		pckOut.end();
 	}
 
+	private void advertiseCapabilities(final StringBuilder m) {
+		m.append('\0');
+		m.append(' ');
+		m.append(CAPABILITY_DELETE_REFS);
+		m.append(' ');
+		m.append(CAPABILITY_REPORT_STATUS);
+		if (allowOfsDelta) {
+			m.append(' ');
+			m.append(CAPABILITY_OFS_DELTA);
+		}
+		m.append(' ');
+	}
+
 	private void format(final StringBuilder m, final char[] idtmp,
 			final ObjectId id, final String name) {
 		m.setLength(0);
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
index 159bd10..7d17b2d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
@@ -235,6 +235,8 @@ private void sendAdvertisedRefs() throws IOException {
 		}
 		while (i.hasNext()) {
 			final Ref r = i.next();
+			if (r.getObjectId() == null)
+				continue;
 			final RevObject o = safeParseAny(r.getObjectId());
 			if (o != null) {
 				advertise(m, idtmp, o, r.getOrigName());
-- 
1.6.3.2.406.gd6a466

← back to recent threads