[8.8.0] Prevent NPE crash when updating `MODULE.bazel.lock` with `null` `Repo… (#30895)
…RuleId`.
When a lockfile contains an entry from before commit 3d60f1c or from a
merge conflict resolution that lacks `repoRuleId`, GSON deserializes
`RepoSpec` with `repoRuleId = null`. Later, when Bazel updates and
writes the lockfile in `BazelLockFileModule.updateLockfile`, serializing
`RepoSpec` invokes `repoRuleId.toString()`, which throws an NPE
resulting in a Bazel crash.
Now we have proper `null`-safety checks in GSON TypeAdapters so that
malformed lockfile entries with `null` repoRuleId are cleanly reported.
Fixes https://github.com/bazelbuild/bazel/issues/24716
PiperOrigin-RevId: 971891689
Change-Id: I405c61d5c6f4d20b9908083099a1e64d7f571350
<!--
Thank you for contributing to Bazel!
Please read the contribution guidelines: https://bazel.build/contribute
-->
### Description
<!--
Please provide a brief summary of the changes in this PR.
-->
### Motivation
<!--
Why is this change important? Does it fix a specific bug or add a new
feature?
If this PR fixes an existing issue, please link it here (e.g. "Fixes
#1234").
-->
### Build API Changes
<!--
Does this PR affect the Build API? (e.g. Starlark API, providers,
command-line flags, native rules)
If yes, please answer the following:
1. Has this been discussed in a design doc or issue? (Please link it)
2. Is the change backward compatible?
3. If it's a breaking change, what is the migration plan?
-->
No
### Checklist
- [ ] I have added tests for the new use cases (if any).
- [ ] I have updated the documentation (if applicable).
### Release Notes
<!--
If this is a new feature, please add 'RELNOTES[NEW]: <description>'
here.
If this is a breaking change, please add 'RELNOTES[INC]: <reason>' here.
If this change should be mentioned in release notes, please add
'RELNOTES: <reason>' here.
-->
RELNOTES: None
Commit
https://github.com/bazelbuild/bazel/commit/a671c5e78f4a78196341cf3a487f62ff4101be4f
---------
Co-authored-by: twerth CA <twerth@google.com>
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java
index 3bbe7b0..7b64a37 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java
@@ -120,7 +120,21 @@
Matcher matcher = LOCKFILE_VERSION_PATTERN.matcher(json);
int version = matcher.find() ? Integer.parseInt(matcher.group(1)) : -1;
if (version == BazelLockFileValue.LOCK_FILE_VERSION) {
- return GsonTypeAdapterUtil.LOCKFILE_GSON.fromJson(json, BazelLockFileValue.class);
+ BazelLockFileValue lockFileValue =
+ GsonTypeAdapterUtil.LOCKFILE_GSON.fromJson(json, BazelLockFileValue.class);
+ if (!isValidLockfile(lockFileValue)) {
+ if (lockfileMode == LockfileMode.ERROR) {
+ throw new BazelLockfileFunctionException(
+ ExternalDepsException.withMessage(
+ Code.BAD_LOCKFILE,
+ "The version of MODULE.bazel.lock is not supported by this version of Bazel."
+ + " Please run `bazel mod deps --lockfile_mode=update` to update your"
+ + " lockfile."),
+ Transience.PERSISTENT);
+ }
+ return BazelLockFileValue.EMPTY_LOCKFILE;
+ }
+ return lockFileValue;
} else {
// This is an old version, its information can't be used.
if (lockfileMode == LockfileMode.ERROR) {
@@ -139,6 +153,28 @@
}
}
+ private static boolean isValidLockfile(@Nullable BazelLockFileValue lockFileValue) {
+ if (lockFileValue == null || lockFileValue.getModuleExtensions() == null) {
+ return false;
+ }
+ for (var extensionMap : lockFileValue.getModuleExtensions().values()) {
+ if (extensionMap == null) {
+ return false;
+ }
+ for (LockFileModuleExtension extension : extensionMap.values()) {
+ if (extension == null || extension.getGeneratedRepoSpecs() == null) {
+ return false;
+ }
+ for (RepoSpec repoSpec : extension.getGeneratedRepoSpecs().values()) {
+ if (repoSpec == null || repoSpec.repoRuleId() == null) {
+ return false;
+ }
+ }
+ }
+ }
+ return true;
+ }
+
static final class BazelLockfileFunctionException extends SkyFunctionException {
BazelLockfileFunctionException(ExternalDepsException cause, Transience transience) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java
index c9bcf76..75d16d1 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java
@@ -307,7 +307,8 @@
* @param lockfileRoot Root under which the lockfile is located
* @param updatedLockfile The updated lockfile data to save
*/
- private static void updateLockfile(Path lockfileRoot, BazelLockFileValue updatedLockfile) {
+ @VisibleForTesting
+ static void updateLockfile(Path lockfileRoot, BazelLockFileValue updatedLockfile) {
RootedPath lockfilePath =
RootedPath.toRootedPath(Root.fromPath(lockfileRoot), LabelConstants.MODULE_LOCKFILE_NAME);
try {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java
index 433124f..dc47842 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java
@@ -80,6 +80,10 @@
new TypeAdapter<>() {
@Override
public void write(JsonWriter jsonWriter, ModuleKey moduleKey) throws IOException {
+ if (moduleKey == null) {
+ jsonWriter.nullValue();
+ return;
+ }
jsonWriter.value(moduleKey.toString());
}
@@ -100,6 +104,10 @@
new TypeAdapter<>() {
@Override
public void write(JsonWriter jsonWriter, Label label) throws IOException {
+ if (label == null) {
+ jsonWriter.nullValue();
+ return;
+ }
jsonWriter.value(label.getUnambiguousCanonicalForm());
}
@@ -113,6 +121,10 @@
new TypeAdapter<>() {
@Override
public void write(JsonWriter jsonWriter, RepoRuleId repoRuleId) throws IOException {
+ if (repoRuleId == null) {
+ jsonWriter.nullValue();
+ return;
+ }
jsonWriter.value(repoRuleId.toString());
}
@@ -132,6 +144,10 @@
new TypeAdapter<>() {
@Override
public void write(JsonWriter jsonWriter, RepositoryName repoName) throws IOException {
+ if (repoName == null) {
+ jsonWriter.nullValue();
+ return;
+ }
jsonWriter.value(repoName.getName());
}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModuleTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModuleTest.java
index 1b3de52..d57ea7f 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModuleTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModuleTest.java
@@ -19,6 +19,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.testutil.Scratch;
+import com.google.devtools.build.lib.vfs.Path;
import java.util.Optional;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.Starlark;
@@ -111,4 +113,26 @@
.isEqualTo(
ImmutableMap.of(extensionId, ImmutableMap.of(evalFactors, nonReproducibleResult)));
}
+
+ @Test
+ public void updateLockfileWithNullRepoRuleIdDoesNotThrow() throws Exception {
+ Path workspaceRoot = new Scratch().dir("/workspace");
+ RepoSpec repoSpecWithNullRuleId = new RepoSpec(null, AttributeValues.create(Dict.empty()));
+ LockFileModuleExtension extensionWithNullRuleId =
+ LockFileModuleExtension.builder()
+ .setBzlTransitiveDigest(new byte[] {1, 2, 3})
+ .setUsagesDigest(new byte[] {4, 5, 6})
+ .setRecordedInputs(ImmutableList.of())
+ .setGeneratedRepoSpecs(ImmutableMap.of("repo", repoSpecWithNullRuleId))
+ .build();
+ BazelLockFileValue lockfile =
+ BazelLockFileValue.builder()
+ .setModuleExtensions(
+ ImmutableMap.of(extensionId, ImmutableMap.of(evalFactors, extensionWithNullRuleId)))
+ .build();
+
+ BazelLockFileModule.updateLockfile(workspaceRoot, lockfile);
+
+ assertThat(workspaceRoot.getRelative("MODULE.bazel.lock").exists()).isTrue();
+ }
}
