opendev/gerrit
Code Issues Proposed changes

AccountsOnInit#getPath: Fix NPE in call to checkState

Browse Source 
The checkState is asserting that file is not null after being resolved,
but then in the case where it *is* null, attempts to dereference it to
build the error message, which will result in NPE without the expected
message.

Fix it so that it doesn't dereference the null file, but uses the path
that was previously used in the resolve attempt.

At the same time, replace usage of checkState and checkArgument with
requireNonNull. The former methods are not recognized as null checks by
some static checkers, SpotBugs for example, and thus result in false
positive warnings about null dereferences. This also means there is a
slight behavioral change: NullPointerException is raised rather than
IllegalStateException.

Change-Id: If66d32afb1d46ec451a2634c8eac82b46e4d5b8f
This commit is contained in:
David Pursehouse
2019-11-28 08:45:29 +09:00
parent 4fe28a47e8
commit da60637959
1 changed files with 6 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
java/com/google/gerrit/pgm/init/AccountsOnInit.java
View File

@@ -14,9 +14,8 @@
package com.google.gerrit.pgm.init;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Objects.requireNonNull;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.RefNames;
@@ -129,9 +128,10 @@ public class AccountsOnInit {
private File getPath() {
Path basePath = site.resolve(flags.cfg.getString("gerrit", null, "basePath"));
checkArgument(basePath != null, "gerrit.basePath must be configured");
File file = FileKey.resolve(basePath.resolve(allUsers).toFile(), FS.DETECTED);
checkState(file != null, "%s does not exist", file.getAbsolutePath());
return file;
requireNonNull(basePath, "gerrit.basePath must be configured");
File file = basePath.resolve(allUsers).toFile();
File resolvedFile = FileKey.resolve(file, FS.DETECTED);
requireNonNull(resolvedFile, () -> String.format("%s does not exist", file.getAbsolutePath()));
return resolvedFile;
}
}