From 6164d189d07b6da2819433b04cfe307171535dd9 Mon Sep 17 00:00:00 2001
From: Fredrik Medley <fredrik@meroton.com>
Date: Tue, 17 Sep 2024 15:19:11 +0200
Subject: [PATCH] Make repo marker file parsing robust to version changes

Bazel 7.1.0 and 7.2.0 contains a bug where + characters in labels in the
repository marker files cannot be parsed. This was fixed in
commit d62e0a0f32188e1875bb8e62ef4377ea4dc1aab2. To reduce the rusk of
future bugs in the same area, this change skips parsing the file if the
first line shows that the content will not be used anyway, which should
be reasonably safe if introducing new formats.

Improves #23336 that fixed #23322.
---
 .../RepositoryDelegatorFunction.java          | 37 +++++++++----------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
index ee295701b79111..cc966936933808 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
@@ -685,39 +685,38 @@ byte[] areRepositoryAndMarkerFileConsistent(
       String content;
       try {
         content = FileSystemUtils.readContent(markerPath, UTF_8);
-        String markerRuleKey = readMarkerFile(content, recordedInputValues);
-        boolean verified = false;
-        if (Preconditions.checkNotNull(ruleKey).equals(markerRuleKey)) {
-          verified = handler.verifyRecordedInputs(rule, directories, recordedInputValues, env);
-          if (env.valuesMissing()) {
-            return null;
-          }
+        if (!readMarkerFile(content, Preconditions.checkNotNull(ruleKey), recordedInputValues)) {
+          return null;
         }
-
-        if (verified) {
-          return new Fingerprint().addString(content).digestAndReset();
-        } else {
+        if (!handler.verifyRecordedInputs(rule, directories, recordedInputValues, env)) {
           return null;
         }
+        if (env.valuesMissing()) {
+          return null;
+        }
+        return new Fingerprint().addString(content).digestAndReset();
       } catch (IOException e) {
         throw new RepositoryFunctionException(e, Transience.TRANSIENT);
       }
     }
 
     @Nullable
-    private static String readMarkerFile(
-        String content, Map<RepoRecordedInput, String> recordedInputValues) {
-      String markerRuleKey = null;
+    private static boolean readMarkerFile(
+        String content, String expectedRuleKey, Map<RepoRecordedInput, String> recordedInputValues) {
       Iterable<String> lines = Splitter.on('\n').split(content);
 
-      boolean firstLine = true;
+      boolean firstLineVerified = false;
       for (String line : lines) {
         if (line.isEmpty()) {
           continue;
         }
-        if (firstLine) {
-          markerRuleKey = line;
-          firstLine = false;
+        if (!firstLineVerified) {
+          if (!line.equals(expectedRuleKey)) {
+            // Break early, need to reload anyway. This also detects marker file version changes
+            // so that unknown formats are not parsed.
+            return false;
+          }
+          firstLineVerified = true;
         } else {
           int sChar = line.indexOf(' ');
           if (sChar > 0) {
@@ -733,7 +732,7 @@ private static String readMarkerFile(
           break;
         }
       }
-      return markerRuleKey;
+      return firstLineVerified;
     }
 
     private String computeRuleKey(Rule rule, StarlarkSemantics starlarkSemantics) {