From 95b36b397b528f858ef1559da3c33c6cf3d7117b Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Sat, 28 Nov 2015 00:15:36 +0100 Subject: [PATCH] Null-annotated Ref class and fixed related compiler errors This change fixes all compiler errors in JGit and replaces possible NPE's with either appropriate exceptions, avoiding multiple "Nullable return" method calls or early returning from the method. Change-Id: I24c8a600ec962d61d5f40abf73eac4203e115240 Signed-off-by: Andrey Loskutov --- .../src/org/eclipse/jgit/pgm/Merge.java | 9 ++-- .../src/org/eclipse/jgit/pgm/RevParse.java | 8 ++- .../eclipse/jgit/internal/JGitText.properties | 2 + .../org/eclipse/jgit/api/CheckoutCommand.java | 13 +++-- .../org/eclipse/jgit/api/CloneCommand.java | 13 +++-- .../org/eclipse/jgit/api/RebaseCommand.java | 19 +++++-- .../org/eclipse/jgit/internal/JGitText.java | 2 + .../storage/dfs/InMemoryRepository.java | 13 +++-- .../jgit/internal/storage/file/GC.java | 6 ++- .../internal/storage/file/RefDirectory.java | 49 +++++++++++++------ .../src/org/eclipse/jgit/lib/ObjectIdRef.java | 36 ++++++++++---- .../src/org/eclipse/jgit/lib/Ref.java | 17 +++++-- .../src/org/eclipse/jgit/lib/RefWriter.java | 26 +++++++--- .../src/org/eclipse/jgit/lib/Repository.java | 12 +++-- .../src/org/eclipse/jgit/lib/SymbolicRef.java | 11 ++++- .../jgit/merge/MergeMessageFormatter.java | 23 ++++----- .../transport/BasePackFetchConnection.java | 8 ++- .../transport/BasePackPushConnection.java | 7 ++- .../jgit/transport/BaseReceivePack.java | 34 ++++++++----- .../eclipse/jgit/transport/FetchProcess.java | 15 +++++- .../eclipse/jgit/transport/PushProcess.java | 9 +++- .../jgit/transport/WalkFetchConnection.java | 4 ++ 22 files changed, 244 insertions(+), 92 deletions(-) diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java index cd65af954..e739b58ae 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java @@ -148,9 +148,12 @@ class Merge extends TextBuiltin { break; case FAST_FORWARD: ObjectId oldHeadId = oldHead.getObjectId(); - outw.println(MessageFormat.format(CLIText.get().updating, oldHeadId - .abbreviate(7).name(), result.getNewHead().abbreviate(7) - .name())); + if (oldHeadId != null) { + String oldId = oldHeadId.abbreviate(7).name(); + String newId = result.getNewHead().abbreviate(7).name(); + outw.println(MessageFormat.format(CLIText.get().updating, oldId, + newId)); + } outw.println(result.getMergeStatus().toString()); break; case CHECKOUT_CONFLICT: diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevParse.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevParse.java index 4456fd534..939f5836f 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevParse.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevParse.java @@ -74,7 +74,13 @@ class RevParse extends TextBuiltin { if (all) { Map allRefs = db.getRefDatabase().getRefs(ALL); for (final Ref r : allRefs.values()) { - outw.println(r.getObjectId().name()); + ObjectId objectId = r.getObjectId(); + // getRefs skips dangling symrefs, so objectId should never be + // null. + if (objectId == null) { + throw new NullPointerException(); + } + outw.println(objectId.name()); } } else { if (verify && commits.size() > 1) { diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 0e9b0b59e..840059d34 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -99,6 +99,7 @@ cannotSquashFixupWithoutPreviousCommit=Cannot {0} without previous commit. cannotStoreObjects=cannot store objects cannotResolveUniquelyAbbrevObjectId=Could not resolve uniquely the abbreviated object ID cannotUnloadAModifiedTree=Cannot unload a modified tree. +cannotUpdateUnbornBranch=Cannot update unborn branch cannotWorkWithOtherStagesThanZeroRightNow=Cannot work with other stages than zero right now. Won't write corrupt index. cannotWriteObjectsPath=Cannot write {0}/{1}: {2} canOnlyCherryPickCommitsWithOneParent=Cannot cherry-pick commit ''{0}'' because it has {1} parents, only commits with exactly one parent are supported. @@ -595,6 +596,7 @@ transportExceptionInvalid=Invalid {0} {1}:{2} transportExceptionMissingAssumed=Missing assumed {0} transportExceptionReadRef=read {0} transportNeedsRepository=Transport needs repository +transportProvidedRefWithNoObjectId=Transport provided ref {0} with no object id transportProtoAmazonS3=Amazon S3 transportProtoBundleFile=Git Bundle File transportProtoFTP=FTP diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java index 8743ea9ac..4f918fa35 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java @@ -331,9 +331,16 @@ public class CheckoutCommand extends GitCommand { } private String getShortBranchName(Ref headRef) { - if (headRef.getTarget().getName().equals(headRef.getName())) - return headRef.getTarget().getObjectId().getName(); - return Repository.shortenRefName(headRef.getTarget().getName()); + if (headRef.isSymbolic()) { + return Repository.shortenRefName(headRef.getTarget().getName()); + } + // Detached HEAD. Every non-symbolic ref in the ref database has an + // object id, so this cannot be null. + ObjectId id = headRef.getObjectId(); + if (id == null) { + throw new NullPointerException(); + } + return id.getName(); } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java index b3bc319ae..2ac872950 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java @@ -61,6 +61,7 @@ import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ProgressMonitor; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; @@ -235,7 +236,7 @@ public class CloneCommand extends TransportCommand { } if (head == null || head.getObjectId() == null) - return; // throw exception? + return; // TODO throw exception? if (head.getName().startsWith(Constants.R_HEADS)) { final RefUpdate newHead = clonedRepo.updateRef(Constants.HEAD); @@ -287,20 +288,24 @@ public class CloneCommand extends TransportCommand { private Ref findBranchToCheckout(FetchResult result) { final Ref idHEAD = result.getAdvertisedRef(Constants.HEAD); - if (idHEAD == null) + ObjectId headId = idHEAD != null ? idHEAD.getObjectId() : null; + if (headId == null) { return null; + } Ref master = result.getAdvertisedRef(Constants.R_HEADS + Constants.MASTER); - if (master != null && master.getObjectId().equals(idHEAD.getObjectId())) + ObjectId objectId = master != null ? master.getObjectId() : null; + if (headId.equals(objectId)) { return master; + } Ref foundBranch = null; for (final Ref r : result.getAdvertisedRefs()) { final String n = r.getName(); if (!n.startsWith(Constants.R_HEADS)) continue; - if (r.getObjectId().equals(idHEAD.getObjectId())) { + if (headId.equals(r.getObjectId())) { foundBranch = r; break; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java index 8582bbb0d..e3e76c95f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java @@ -560,6 +560,8 @@ public class RebaseCommand extends GitCommand { lastStepWasForward = newHead != null; if (!lastStepWasForward) { ObjectId headId = getHead().getObjectId(); + // getHead() checks for null + assert headId != null; if (!AnyObjectId.equals(headId, newParents.get(0))) checkoutCommit(headId.getName(), newParents.get(0)); @@ -674,6 +676,8 @@ public class RebaseCommand extends GitCommand { return; ObjectId headId = getHead().getObjectId(); + // getHead() checks for null + assert headId != null; String head = headId.getName(); String currentCommits = rebaseState.readFile(CURRENT_COMMIT); for (String current : currentCommits.split("\n")) //$NON-NLS-1$ @@ -1073,11 +1077,12 @@ public class RebaseCommand extends GitCommand { Ref head = getHead(); - String headName = getHeadName(head); ObjectId headId = head.getObjectId(); - if (headId == null) + if (headId == null) { throw new RefNotFoundException(MessageFormat.format( JGitText.get().refNotResolved, Constants.HEAD)); + } + String headName = getHeadName(head); RevCommit headCommit = walk.lookupCommit(headId); RevCommit upstream = walk.lookupCommit(upstreamCommit.getId()); @@ -1188,10 +1193,14 @@ public class RebaseCommand extends GitCommand { private static String getHeadName(Ref head) { String headName; - if (head.isSymbolic()) + if (head.isSymbolic()) { headName = head.getTarget().getName(); - else - headName = head.getObjectId().getName(); + } else { + ObjectId headId = head.getObjectId(); + // the callers are checking this already + assert headId != null; + headName = headId.getName(); + } return headName; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 796eaaebf..c476f1773 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -158,6 +158,7 @@ public class JGitText extends TranslationBundle { /***/ public String cannotStoreObjects; /***/ public String cannotResolveUniquelyAbbrevObjectId; /***/ public String cannotUnloadAModifiedTree; + /***/ public String cannotUpdateUnbornBranch; /***/ public String cannotWorkWithOtherStagesThanZeroRightNow; /***/ public String cannotWriteObjectsPath; /***/ public String canOnlyCherryPickCommitsWithOneParent; @@ -663,6 +664,7 @@ public class JGitText extends TranslationBundle { /***/ public String transportProtoSFTP; /***/ public String transportProtoSSH; /***/ public String transportProtoTest; + /***/ public String transportProvidedRefWithNoObjectId; /***/ public String transportSSHRetryInterrupt; /***/ public String treeEntryAlreadyExists; /***/ public String treeFilterMarkerTooManyFilters; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java index 1c664b409..cdebb7a8f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java @@ -334,11 +334,14 @@ public class InMemoryRepository extends DfsRepository { reject(cmds); return; } - } else if (r.isSymbolic() || r.getObjectId() == null - || !r.getObjectId().equals(c.getOldId())) { - c.setResult(ReceiveCommand.Result.LOCK_FAILURE); - reject(cmds); - return; + } else { + ObjectId objectId = r.getObjectId(); + if (r.isSymbolic() || objectId == null + || !objectId.equals(c.getOldId())) { + c.setResult(ReceiveCommand.Result.LOCK_FAILURE); + reject(cmds); + return; + } } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index 4c40538b6..c0f56f448 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -67,6 +67,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.Set; import java.util.TreeMap; @@ -483,9 +484,10 @@ public class GC { return false; return r1.getTarget().getName().equals(r2.getTarget().getName()); } else { - if (r2.isSymbolic()) + if (r2.isSymbolic()) { return false; - return r1.getObjectId().equals(r2.getObjectId()); + } + return Objects.equals(r1.getObjectId(), r2.getObjectId()); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java index 69f7e9707..2c8e5f9d1 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java @@ -73,6 +73,7 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.errors.InvalidObjectIdException; import org.eclipse.jgit.errors.LockFailedException; import org.eclipse.jgit.errors.MissingObjectException; @@ -715,16 +716,20 @@ public class RefDirectory extends RefDatabase { */ private Ref peeledPackedRef(Ref f) throws MissingObjectException, IOException { - if (f.getStorage().isPacked() && f.isPeeled()) + if (f.getStorage().isPacked() && f.isPeeled()) { return f; - if (!f.isPeeled()) + } + if (!f.isPeeled()) { f = peel(f); - if (f.getPeeledObjectId() != null) + } + ObjectId peeledObjectId = f.getPeeledObjectId(); + if (peeledObjectId != null) { return new ObjectIdRef.PeeledTag(PACKED, f.getName(), - f.getObjectId(), f.getPeeledObjectId()); - else + f.getObjectId(), peeledObjectId); + } else { return new ObjectIdRef.PeeledNonTag(PACKED, f.getName(), f.getObjectId()); + } } void log(final RefUpdate update, final String msg, final boolean deref) @@ -985,7 +990,7 @@ public class RefDirectory extends RefDatabase { try { id = ObjectId.fromString(buf, 0); if (ref != null && !ref.isSymbolic() - && ref.getTarget().getObjectId().equals(id)) { + && id.equals(ref.getTarget().getObjectId())) { assert(currentSnapshot != null); currentSnapshot.setClean(otherSnapshot); return ref; @@ -1103,8 +1108,8 @@ public class RefDirectory extends RefDatabase { implements LooseRef { private final FileSnapshot snapShot; - LoosePeeledTag(FileSnapshot snapshot, String refName, ObjectId id, - ObjectId p) { + LoosePeeledTag(FileSnapshot snapshot, @NonNull String refName, + @NonNull ObjectId id, @NonNull ObjectId p) { super(LOOSE, refName, id, p); this.snapShot = snapshot; } @@ -1122,7 +1127,8 @@ public class RefDirectory extends RefDatabase { implements LooseRef { private final FileSnapshot snapShot; - LooseNonTag(FileSnapshot snapshot, String refName, ObjectId id) { + LooseNonTag(FileSnapshot snapshot, @NonNull String refName, + @NonNull ObjectId id) { super(LOOSE, refName, id); this.snapShot = snapshot; } @@ -1140,7 +1146,8 @@ public class RefDirectory extends RefDatabase { implements LooseRef { private FileSnapshot snapShot; - LooseUnpeeled(FileSnapshot snapShot, String refName, ObjectId id) { + LooseUnpeeled(FileSnapshot snapShot, @NonNull String refName, + @NonNull ObjectId id) { super(LOOSE, refName, id); this.snapShot = snapShot; } @@ -1149,13 +1156,24 @@ public class RefDirectory extends RefDatabase { return snapShot; } + @NonNull + @Override + public ObjectId getObjectId() { + ObjectId id = super.getObjectId(); + assert id != null; // checked in constructor + return id; + } + public LooseRef peel(ObjectIdRef newLeaf) { - if (newLeaf.getPeeledObjectId() != null) + ObjectId peeledObjectId = newLeaf.getPeeledObjectId(); + ObjectId objectId = getObjectId(); + if (peeledObjectId != null) { return new LoosePeeledTag(snapShot, getName(), - getObjectId(), newLeaf.getPeeledObjectId()); - else + objectId, peeledObjectId); + } else { return new LooseNonTag(snapShot, getName(), - getObjectId()); + objectId); + } } } @@ -1163,7 +1181,8 @@ public class RefDirectory extends RefDatabase { LooseRef { private final FileSnapshot snapShot; - LooseSymbolicRef(FileSnapshot snapshot, String refName, Ref target) { + LooseSymbolicRef(FileSnapshot snapshot, @NonNull String refName, + @NonNull Ref target) { super(refName, target); this.snapShot = snapshot; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdRef.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdRef.java index f481c772d..c286f5e46 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdRef.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdRef.java @@ -44,6 +44,9 @@ package org.eclipse.jgit.lib; +import org.eclipse.jgit.annotations.NonNull; +import org.eclipse.jgit.annotations.Nullable; + /** A {@link Ref} that points directly at an {@link ObjectId}. */ public abstract class ObjectIdRef implements Ref { /** Any reference whose peeled value is not yet known. */ @@ -56,13 +59,15 @@ public abstract class ObjectIdRef implements Ref { * @param name * name of this ref. * @param id - * current value of the ref. May be null to indicate a ref - * that does not exist yet. + * current value of the ref. May be {@code null} to indicate + * a ref that does not exist yet. */ - public Unpeeled(Storage st, String name, ObjectId id) { + public Unpeeled(@NonNull Storage st, @NonNull String name, + @Nullable ObjectId id) { super(st, name, id); } + @Nullable public ObjectId getPeeledObjectId() { return null; } @@ -88,11 +93,13 @@ public abstract class ObjectIdRef implements Ref { * @param p * the first non-tag object that tag {@code id} points to. */ - public PeeledTag(Storage st, String name, ObjectId id, ObjectId p) { + public PeeledTag(@NonNull Storage st, @NonNull String name, + @Nullable ObjectId id, @NonNull ObjectId p) { super(st, name, id); peeledObjectId = p; } + @NonNull public ObjectId getPeeledObjectId() { return peeledObjectId; } @@ -112,13 +119,15 @@ public abstract class ObjectIdRef implements Ref { * @param name * name of this ref. * @param id - * current value of the ref. May be null to indicate a ref - * that does not exist yet. + * current value of the ref. May be {@code null} to indicate + * a ref that does not exist yet. */ - public PeeledNonTag(Storage st, String name, ObjectId id) { + public PeeledNonTag(@NonNull Storage st, @NonNull String name, + @Nullable ObjectId id) { super(st, name, id); } + @Nullable public ObjectId getPeeledObjectId() { return null; } @@ -142,15 +151,17 @@ public abstract class ObjectIdRef implements Ref { * @param name * name of this ref. * @param id - * current value of the ref. May be null to indicate a ref that - * does not exist yet. + * current value of the ref. May be {@code null} to indicate a + * ref that does not exist yet. */ - protected ObjectIdRef(Storage st, String name, ObjectId id) { + protected ObjectIdRef(@NonNull Storage st, @NonNull String name, + @Nullable ObjectId id) { this.name = name; this.storage = st; this.objectId = id; } + @NonNull public String getName() { return name; } @@ -159,22 +170,27 @@ public abstract class ObjectIdRef implements Ref { return false; } + @NonNull public Ref getLeaf() { return this; } + @NonNull public Ref getTarget() { return this; } + @Nullable public ObjectId getObjectId() { return objectId; } + @NonNull public Storage getStorage() { return storage; } + @NonNull @Override public String toString() { StringBuilder r = new StringBuilder(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Ref.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Ref.java index f119c44fe..a78a90fe5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Ref.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Ref.java @@ -43,6 +43,9 @@ package org.eclipse.jgit.lib; +import org.eclipse.jgit.annotations.NonNull; +import org.eclipse.jgit.annotations.Nullable; + /** * Pairing of a name and the {@link ObjectId} it currently has. *

@@ -126,6 +129,7 @@ public interface Ref { * * @return name of this ref. */ + @NonNull public String getName(); /** @@ -156,6 +160,7 @@ public interface Ref { * * @return the reference that actually stores the ObjectId value. */ + @NonNull public abstract Ref getLeaf(); /** @@ -170,22 +175,27 @@ public interface Ref { * * @return the target reference, or {@code this}. */ + @NonNull public abstract Ref getTarget(); /** * Cached value of this ref. * - * @return the value of this ref at the last time we read it. + * @return the value of this ref at the last time we read it. May be + * {@code null} to indicate a ref that does not exist yet or a + * symbolic ref pointing to an unborn branch. */ + @Nullable public abstract ObjectId getObjectId(); /** * Cached value of ref^{} (the ref peeled to commit). * * @return if this ref is an annotated tag the id of the commit (or tree or - * blob) that the annotated tag refers to; null if this ref does not - * refer to an annotated tag. + * blob) that the annotated tag refers to; {@code null} if this ref + * does not refer to an annotated tag. */ + @Nullable public abstract ObjectId getPeeledObjectId(); /** @@ -201,5 +211,6 @@ public interface Ref { * * @return type of ref. */ + @NonNull public abstract Storage getStorage(); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefWriter.java index 747fa62b5..3a02b2281 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefWriter.java @@ -119,13 +119,20 @@ public abstract class RefWriter { continue; } - r.getObjectId().copyTo(tmp, w); + ObjectId objectId = r.getObjectId(); + if (objectId == null) { + // Symrefs to unborn branches aren't advertised in the info/refs + // file. + continue; + } + objectId.copyTo(tmp, w); w.write('\t'); w.write(r.getName()); w.write('\n'); - if (r.getPeeledObjectId() != null) { - r.getPeeledObjectId().copyTo(tmp, w); + ObjectId peeledObjectId = r.getPeeledObjectId(); + if (peeledObjectId != null) { + peeledObjectId.copyTo(tmp, w); w.write('\t'); w.write(r.getName()); w.write("^{}\n"); //$NON-NLS-1$ @@ -167,14 +174,21 @@ public abstract class RefWriter { if (r.getStorage() != Ref.Storage.PACKED) continue; - r.getObjectId().copyTo(tmp, w); + ObjectId objectId = r.getObjectId(); + if (objectId == null) { + // A packed ref cannot be a symref, let alone a symref + // to an unborn branch. + throw new NullPointerException(); + } + objectId.copyTo(tmp, w); w.write(' '); w.write(r.getName()); w.write('\n'); - if (r.getPeeledObjectId() != null) { + ObjectId peeledObjectId = r.getPeeledObjectId(); + if (peeledObjectId != null) { w.write('^'); - r.getPeeledObjectId().copyTo(tmp, w); + peeledObjectId.copyTo(tmp, w); w.write('\n'); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index 49a970d03..f8266133a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -911,12 +911,16 @@ public abstract class Repository implements AutoCloseable { @Nullable public String getFullBranch() throws IOException { Ref head = getRef(Constants.HEAD); - if (head == null) + if (head == null) { return null; - if (head.isSymbolic()) + } + if (head.isSymbolic()) { return head.getTarget().getName(); - if (head.getObjectId() != null) - return head.getObjectId().name(); + } + ObjectId objectId = head.getObjectId(); + if (objectId != null) { + return objectId.name(); + } return null; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/SymbolicRef.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/SymbolicRef.java index 43b1510f9..eeab921a7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/SymbolicRef.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/SymbolicRef.java @@ -43,6 +43,9 @@ package org.eclipse.jgit.lib; +import org.eclipse.jgit.annotations.NonNull; +import org.eclipse.jgit.annotations.Nullable; + /** * A reference that indirectly points at another {@link Ref}. *

@@ -62,11 +65,12 @@ public class SymbolicRef implements Ref { * @param target * the ref we reference and derive our value from. */ - public SymbolicRef(String refName, Ref target) { + public SymbolicRef(@NonNull String refName, @NonNull Ref target) { this.name = refName; this.target = target; } + @NonNull public String getName() { return name; } @@ -75,6 +79,7 @@ public class SymbolicRef implements Ref { return true; } + @NonNull public Ref getLeaf() { Ref dst = getTarget(); while (dst.isSymbolic()) @@ -82,18 +87,22 @@ public class SymbolicRef implements Ref { return dst; } + @NonNull public Ref getTarget() { return target; } + @Nullable public ObjectId getObjectId() { return getLeaf().getObjectId(); } + @NonNull public Storage getStorage() { return Storage.LOOSE; } + @Nullable public ObjectId getPeeledObjectId() { return getLeaf().getPeeledObjectId(); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeMessageFormatter.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeMessageFormatter.java index 191f3d836..82cbf368c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeMessageFormatter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeMessageFormatter.java @@ -46,6 +46,7 @@ import java.util.ArrayList; import java.util.List; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.util.ChangeIdUtil; @@ -76,22 +77,22 @@ public class MergeMessageFormatter { List commits = new ArrayList(); List others = new ArrayList(); for (Ref ref : refsToMerge) { - if (ref.getName().startsWith(Constants.R_HEADS)) + if (ref.getName().startsWith(Constants.R_HEADS)) { branches.add("'" + Repository.shortenRefName(ref.getName()) //$NON-NLS-1$ + "'"); //$NON-NLS-1$ - - else if (ref.getName().startsWith(Constants.R_REMOTES)) + } else if (ref.getName().startsWith(Constants.R_REMOTES)) { remoteBranches.add("'" //$NON-NLS-1$ + Repository.shortenRefName(ref.getName()) + "'"); //$NON-NLS-1$ - - else if (ref.getName().startsWith(Constants.R_TAGS)) + } else if (ref.getName().startsWith(Constants.R_TAGS)) { tags.add("'" + Repository.shortenRefName(ref.getName()) + "'"); //$NON-NLS-1$ //$NON-NLS-2$ - - else if (ref.getName().equals(ref.getObjectId().getName())) - commits.add("'" + ref.getName() + "'"); //$NON-NLS-1$ //$NON-NLS-2$ - - else - others.add(ref.getName()); + } else { + ObjectId objectId = ref.getObjectId(); + if (objectId != null && ref.getName().equals(objectId.getName())) { + commits.add("'" + ref.getName() + "'"); //$NON-NLS-1$ //$NON-NLS-2$ + } else { + others.add(ref.getName()); + } + } } List listings = new ArrayList(); 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 cf13582db..754cf361a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java @@ -464,8 +464,12 @@ public abstract class BasePackFetchConnection extends BasePackConnection final PacketLineOut p = statelessRPC ? pckState : pckOut; boolean first = true; for (final Ref r : want) { + ObjectId objectId = r.getObjectId(); + if (objectId == null) { + continue; + } try { - if (walk.parseAny(r.getObjectId()).has(REACHABLE)) { + if (walk.parseAny(objectId).has(REACHABLE)) { // We already have this object. Asking for it is // not a very good idea. // @@ -478,7 +482,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection final StringBuilder line = new StringBuilder(46); line.append("want "); //$NON-NLS-1$ - line.append(r.getObjectId().name()); + line.append(objectId.name()); if (first) { line.append(enableCapabilities()); first = false; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java index 0834c359a..4499f66d5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java @@ -239,8 +239,11 @@ public abstract class BasePackPushConnection extends BasePackConnection implemen final StringBuilder sb = new StringBuilder(); ObjectId oldId = rru.getExpectedOldObjectId(); if (oldId == null) { - Ref adv = getRef(rru.getRemoteName()); - oldId = adv != null ? adv.getObjectId() : ObjectId.zeroId(); + final Ref advertised = getRef(rru.getRemoteName()); + oldId = advertised != null ? advertised.getObjectId() : null; + if (oldId == null) { + oldId = ObjectId.zeroId(); + } } sb.append(oldId.name()); sb.append(' '); 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 776a9f695..c224d8eeb 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -1372,16 +1372,21 @@ public abstract class BaseReceivePack { } } - if (cmd.getType() == ReceiveCommand.Type.DELETE && ref != null - && !ObjectId.zeroId().equals(cmd.getOldId()) - && !ref.getObjectId().equals(cmd.getOldId())) { - // Delete commands can be sent with the old id matching our - // advertised value, *OR* with the old id being 0{40}. Any - // other requested old id is invalid. - // - cmd.setResult(Result.REJECTED_OTHER_REASON, - JGitText.get().invalidOldIdSent); - continue; + if (cmd.getType() == ReceiveCommand.Type.DELETE && ref != null) { + ObjectId id = ref.getObjectId(); + if (id == null) { + id = ObjectId.zeroId(); + } + if (!ObjectId.zeroId().equals(cmd.getOldId()) + && !id.equals(cmd.getOldId())) { + // Delete commands can be sent with the old id matching our + // advertised value, *OR* with the old id being 0{40}. Any + // other requested old id is invalid. + // + cmd.setResult(Result.REJECTED_OTHER_REASON, + JGitText.get().invalidOldIdSent); + continue; + } } if (cmd.getType() == ReceiveCommand.Type.UPDATE) { @@ -1391,8 +1396,15 @@ public abstract class BaseReceivePack { cmd.setResult(Result.REJECTED_OTHER_REASON, JGitText.get().noSuchRef); continue; } + ObjectId id = ref.getObjectId(); + if (id == null) { + // We cannot update unborn branch + cmd.setResult(Result.REJECTED_OTHER_REASON, + JGitText.get().cannotUpdateUnbornBranch); + continue; + } - if (!ref.getObjectId().equals(cmd.getOldId())) { + if (!id.equals(cmd.getOldId())) { // A properly functioning client will send the same // object id we advertised. // diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java index 9aae1c37a..c4b3f8304 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java @@ -397,11 +397,17 @@ class FetchProcess { private void expandFetchTags() throws TransportException { final Map haveRefs = localRefs(); for (final Ref r : conn.getRefs()) { - if (!isTag(r)) + if (!isTag(r)) { + continue; + } + ObjectId id = r.getObjectId(); + if (id == null) { continue; + } final Ref local = haveRefs.get(r.getName()); - if (local == null || !r.getObjectId().equals(local.getObjectId())) + if (local == null || !id.equals(local.getObjectId())) { wantTag(r); + } } } @@ -413,6 +419,11 @@ class FetchProcess { private void want(final Ref src, final RefSpec spec) throws TransportException { final ObjectId newId = src.getObjectId(); + if (newId == null) { + throw new NullPointerException(MessageFormat.format( + JGitText.get().transportProvidedRefWithNoObjectId, + src.getName())); + } if (spec.getDestination() != null) { final TrackingRefUpdate tru = createUpdate(spec, newId); if (newId.equals(tru.getOldObjectId())) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java index 4fd192dbb..5cea88215 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java @@ -188,8 +188,13 @@ class PushProcess { final Map result = new HashMap(); for (final RemoteRefUpdate rru : toPush.values()) { final Ref advertisedRef = connection.getRef(rru.getRemoteName()); - final ObjectId advertisedOld = (advertisedRef == null ? ObjectId - .zeroId() : advertisedRef.getObjectId()); + ObjectId advertisedOld = null; + if (advertisedRef != null) { + advertisedOld = advertisedRef.getObjectId(); + } + if (advertisedOld == null) { + advertisedOld = ObjectId.zeroId(); + } if (rru.getNewObjectId().equals(advertisedOld)) { if (rru.isDelete()) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java index 1c6b8b736..dfc3ee4c3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java @@ -267,6 +267,10 @@ class WalkFetchConnection extends BaseFetchConnection { final HashSet inWorkQueue = new HashSet(); for (final Ref r : want) { final ObjectId id = r.getObjectId(); + if (id == null) { + throw new NullPointerException(MessageFormat.format( + JGitText.get().transportProvidedRefWithNoObjectId, r.getName())); + } try { final RevObject obj = revWalk.parseAny(id); if (obj.has(COMPLETE))