opendev/gerrit
Code Issues Proposed changes

Merge changes from topic 'coverity'

Browse Source 
* changes:
  BaseInit: Throw the result of die
  PrologCompiler: Fix NPEs when temp dir can't be listed
  UploadArchive: Correct check for tree-ish not found
  ReviewerSuggestionCache: Explicitly check for ID field existence
  MergeOp: Fix null dereference in no-op update log statement
  MergeOp: Fix null dereference when HEAD is missing
  SubmoduleOp: Fix potential null dereference when branch is gone
  SubmoduleOp: Remove unnecessary null check
  InternalChangeQuery: Check that schema is not null
  GerritPublicKeyChecker: Fix possible null dereference
This commit is contained in:
Dave Borowitz 2015-09-04 13:20:17 +00:00 committed by Gerrit Code Review
commit 586121a16e
9 changed files with 57 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
View File

@ -17,6 +17,7 @@ package com.google.gerrit.gpg;
import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString;
import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_GPGKEY;
import com.google.common.base.MoreObjects;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.Ordering;
import com.google.gerrit.common.PageLinks;
@ -35,6 +36,7 @@ import org.eclipse.jgit.transport.PushCertificateIdent;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@ -78,8 +80,7 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
while (userIds.hasNext()) {
String userId = userIds.next();
if (isAllowed(userId, allowedUserIds)) {
@SuppressWarnings("unchecked")
Iterator<PGPSignature> sigs = key.getSignaturesForID(userId);
Iterator<PGPSignature> sigs = getSignaturesForId(key, userId);
while (sigs.hasNext()) {
if (isValidCertification(key, sigs.next(), userId)) {
return;
@ -96,6 +97,14 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
}
}
@SuppressWarnings("unchecked")
private Iterator<PGPSignature> getSignaturesForId(PGPPublicKey key,
String userId) {
return MoreObjects.firstNonNull(
key.getSignaturesForID(userId),
Collections.emptyIterator());
}
private Set<String> getAllowedUserIds() {
IdentifiedUser user = userProvider.get();
Set<String> result = new HashSet<>();

11
gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java
View File

@ -215,12 +215,11 @@ public class BaseInit extends SiteProgram {
if (secureStoreInitData != null && currentSecureStoreClassName != null
&& !currentSecureStoreClassName.equals(secureStoreInitData.className)) {
String err =
String.format(
"Different secure store was previously configured: %s. "
+ "Use SwitchSecureStore program to switch between implementations.",
currentSecureStoreClassName);
die(err, new RuntimeException("secure store mismatch"));
String err = String.format(
"Different secure store was previously configured: %s. "
+ "Use SwitchSecureStore program to switch between implementations.",
currentSecureStoreClassName);
throw die(err);
}
m.add(new InitModule(standalone, initDb));

28
gerrit-pgm/src/main/java/com/google/gerrit/pgm/rules/PrologCompiler.java
View File

@ -262,14 +262,16 @@ public class PrologCompiler implements Callable<PrologCompiler.Status> {
}
}
private List<File> getAllFiles(File dir, String extension) {
private List<File> getAllFiles(File dir, String extension)
throws IOException {
ArrayList<File> fileList = new ArrayList<>();
getAllFiles(dir, extension, fileList);
return fileList;
}
private void getAllFiles(File dir, String extension, List<File> fileList) {
for (File f : dir.listFiles()) {
private void getAllFiles(File dir, String extension, List<File> fileList)
throws IOException {
for (File f : listFiles(dir)) {
if (f.getName().endsWith(extension)) {
fileList.add(f);
}
@ -279,14 +281,16 @@ public class PrologCompiler implements Callable<PrologCompiler.Status> {
}
}
private List<String> getRelativePaths(File dir, String extension) {
private List<String> getRelativePaths(File dir, String extension)
throws IOException {
ArrayList<String> pathList = new ArrayList<>();
getRelativePaths(dir, extension, "", pathList);
return pathList;
}
private void getRelativePaths(File dir, String extension, String path, List<String> pathList) {
for (File f : dir.listFiles()) {
private static void getRelativePaths(File dir, String extension, String path,
List<String> pathList) throws IOException {
for (File f : listFiles(dir)) {
if (f.getName().endsWith(extension)) {
pathList.add(path + f.getName());
}
@ -296,8 +300,8 @@ public class PrologCompiler implements Callable<PrologCompiler.Status> {
}
}
private void deleteAllFiles(File dir) {
for (File f : dir.listFiles()) {
private static void deleteAllFiles(File dir) throws IOException {
for (File f : listFiles(dir)) {
if (f.isDirectory()) {
deleteAllFiles(f);
} else {
@ -306,4 +310,12 @@ public class PrologCompiler implements Callable<PrologCompiler.Status> {
}
dir.delete();
}
private static File[] listFiles(File dir) throws IOException {
File[] files = dir.listFiles();
if (files == null) {
throw new IOException("Failed to list directory: " + dir);
}
return files;
}
}

7
gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestionCache.java
View File

@ -14,6 +14,8 @@
package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.base.Splitter;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
@ -41,6 +43,7 @@ import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.IndexWriterConfig;
import org.apache.lucene.index.IndexWriterConfig.OpenMode;
import org.apache.lucene.index.IndexableField;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanClause.Occur;
import org.apache.lucene.search.BooleanQuery;
@ -124,8 +127,8 @@ public class ReviewerSuggestionCache {
for (ScoreDoc h : hits) {
Document doc = searcher.doc(h.doc);
AccountInfo info = new AccountInfo(
doc.getField(ID).numericValue().intValue());
IndexableField idField = checkNotNull(doc.getField(ID));
AccountInfo info = new AccountInfo(idField.numericValue().intValue());
info.name = doc.get(NAME);
info.email = doc.get(EMAIL);
info.username = doc.get(USERNAME);

10
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@ -505,7 +505,7 @@ public class MergeOp {
if (branchUpdate.getOldObjectId() != null) {
branchTip =
(CodeReviewCommit) rw.parseCommit(branchUpdate.getOldObjectId());
} else if (repo.getFullBranch().equals(destBranch.get())) {
} else if (Objects.equals(repo.getFullBranch(), destBranch.get())) {
branchTip = null;
branchUpdate.setExpectedOldObjectId(ObjectId.zeroId());
} else {
@ -702,8 +702,12 @@ public class MergeOp {
CodeReviewCommit currentTip =
mergeTip != null ? mergeTip.getCurrentTip() : null;
if (Objects.equals(branchTip, currentTip)) {
logDebug("Branch already at merge tip {}, no update to perform",
currentTip.name());
if (currentTip != null) {
logDebug("Branch already at merge tip {}, no update to perform",
currentTip.name());
} else {
logDebug("Both branch and merge tip are nonexistent, no update");
}
return null;
} else if (currentTip == null) {
logDebug("No merge tip, no update to perform");

11
gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
View File

@ -121,6 +121,10 @@ public class SubmoduleOp {
RevWalk rw = new RevWalk(repo)) {
ObjectId id = repo.resolve(destBranch.get());
if (id == null) {
logAndThrowSubmoduleException(
"Cannot resolve submodule destination branch " + destBranch);
}
RevCommit commit = rw.parseCommit(id);
Set<SubmoduleSubscription> oldSubscriptions =
@ -253,7 +257,7 @@ public class SubmoduleOp {
}
DirCacheEntry dce = dc.getEntry(s.getPath());
ObjectId oldId = null;
ObjectId oldId;
if (dce != null) {
if (!dce.getFileMode().equals(FileMode.GITLINK)) {
log.error("Requested to update gitlink " + s.getPath() + " in "
@ -283,10 +287,7 @@ public class SubmoduleOp {
try {
rw.markStart(newCommit);
if (oldId != null) {
rw.markUninteresting(rw.parseCommit(oldId));
}
rw.markUninteresting(rw.parseCommit(oldId));
for (RevCommit c : rw) {
msgbuf.append(c.getFullMessage() + "\n\n");
}

2
gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommitPredicate.java
View File

@ -28,7 +28,7 @@ class CommitPredicate extends IndexPredicate<ChangeData> {
static FieldDef<ChangeData, ?> commitField(Schema<ChangeData> schema,
String id) {
if (id.length() == OBJECT_ID_STRING_LENGTH
&& schema.hasField(EXACT_COMMIT)) {
&& schema != null && schema.hasField(EXACT_COMMIT)) {
return EXACT_COMMIT;
}
return COMMIT;

2
gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
View File

@ -128,7 +128,7 @@ public class InternalChangeQuery {
public Iterable<ChangeData> byCommitsOnBranchNotMerged(Branch.NameKey branch,
List<String> hashes) throws OrmException {
Schema<ChangeData> schema = schema(indexes);
if (schema.hasField(ChangeField.EXACT_COMMIT)) {
if (schema != null && schema.hasField(ChangeField.EXACT_COMMIT)) {
return query(commitsOnBranchNotMerged(branch, commits(schema, hashes)));
} else {
return byCommitsOnBranchNotMerged(

2
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/UploadArchive.java
View File

@ -162,7 +162,7 @@ public class UploadArchive extends AbstractGitCommand {
// Find out the object to get from the specified reference and paths
ObjectId treeId = repo.resolve(options.treeIsh);
if (treeId.equals(ObjectId.zeroId())) {
if (treeId == null) {
throw new Failure(4, "fatal: reference not found");
}