Reencode non-ASCII strings in BEP command line and metadata events

src/main/java/com/google/devtools/build/lib/analysis/BUILD

@@ -473,6 +473,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/buildeventstream",
         "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
         "//src/main/java/com/google/devtools/build/lib/events",
+        "//src/main/java/com/google/devtools/build/lib/util:string_encoding",
         "//third_party:guava",
     ],
 )

src/main/java/com/google/devtools/build/lib/analysis/BuildInfoEvent.java

@@ -23,6 +23,7 @@
 import com.google.devtools.build.lib.buildeventstream.BuildEventWithOrderConstraint;
 import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.util.StringEncoding;
 import java.util.Collection;
 import java.util.Map;
 
@@ -67,8 +68,8 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert
     for (Map.Entry<String, String> entry : getBuildInfoMap().entrySet()) {
       status.addItem(
           BuildEventStreamProtos.WorkspaceStatus.Item.newBuilder()
-              .setKey(entry.getKey())
-              .setValue(entry.getValue())
+              .setKey(StringEncoding.internalToUnicode(entry.getKey()))
+              .setValue(StringEncoding.internalToUnicode(entry.getValue()))
               .build());
     }
     return GenericBuildEvent.protoChaining(this).setWorkspaceStatus(status.build()).build();

src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java

@@ -14,7 +14,7 @@
 package com.google.devtools.build.lib.bazel;
 
 import static com.google.common.base.StandardSystemProperty.USER_NAME;
-import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.nio.charset.StandardCharsets.ISO_8859_1;
 import static java.util.stream.Collectors.joining;
 
 import com.google.common.collect.ImmutableList;
@@ -58,7 +58,6 @@
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
-import java.nio.charset.StandardCharsets;
 import java.time.Instant;
 import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
@@ -117,7 +116,9 @@ private String getAdditionalWorkspaceStatus(
           } catch (IOException e) {
             throw createExecutionException(e, Code.STDERR_IO_EXCEPTION);
           }
-          return stdoutStream.toString(UTF_8);
+          // Keep the output in Bazel's internal string encoding (see StringEncoding), like the
+          // other entries in the status maps.
+          return stdoutStream.toString(ISO_8859_1);
         }
       } catch (BadExitStatusException e) {
         throw createExecutionException(e, Code.NON_ZERO_EXIT);
@@ -154,7 +155,9 @@ private static byte[] printStatusMap(Map<String, String> map) {
               .map(entry -> entry.getKey() + " " + entry.getValue())
               .collect(joining("\n"));
       s += "\n";
-      return s.getBytes(StandardCharsets.UTF_8);
+      // The values are in Bazel's internal string encoding (see StringEncoding), write them out as
+      // is.
+      return s.getBytes(ISO_8859_1);
     }
 
     @Override

src/main/java/com/google/devtools/build/lib/runtime/BUILD

@@ -106,6 +106,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/buildeventstream",
         "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
         "//src/main/java/com/google/devtools/build/lib/util:pair",
+        "//src/main/java/com/google/devtools/build/lib/util:string_encoding",
         "//src/main/java/com/google/devtools/common/options",
         "//src/main/protobuf:command_line_java_proto",
         "//src/main/protobuf:option_filters_java_proto",
@@ -326,6 +327,7 @@ java_library(
     deps = [
         "//src/main/java/com/google/devtools/build/lib/buildeventstream",
         "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
+        "//src/main/java/com/google/devtools/build/lib/util:string_encoding",
         "//third_party:guava",
     ],
 )

src/main/java/com/google/devtools/build/lib/runtime/BuildMetadataEvent.java

@@ -21,6 +21,7 @@
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId;
 import com.google.devtools.build.lib.buildeventstream.BuildEventWithOrderConstraint;
 import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent;
+import com.google.devtools.build.lib.util.StringEncoding;
 import java.util.Collection;
 import java.util.Map;
 
@@ -56,7 +57,9 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert
     BuildEventStreamProtos.BuildMetadata.Builder metadataBuilder =
         BuildEventStreamProtos.BuildMetadata.newBuilder();
     for (Map.Entry<String, String> entry : buildMetadata.entrySet()) {
-      metadataBuilder.putMetadata(entry.getKey(), entry.getValue());
+      metadataBuilder.putMetadata(
+          StringEncoding.internalToUnicode(entry.getKey()),
+          StringEncoding.internalToUnicode(entry.getValue()));
     }
     return GenericBuildEvent.protoChaining(this).setBuildMetadata(metadataBuilder.build()).build();
   }

src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java

@@ -16,6 +16,7 @@
 import com.google.common.base.Joiner;
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
 import com.google.common.io.BaseEncoding;
 import com.google.devtools.build.lib.buildeventstream.BuildEventContext;
 import com.google.devtools.build.lib.buildeventstream.BuildEventIdUtil;
@@ -31,6 +32,7 @@
 import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.Option;
 import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.OptionList;
 import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.lib.util.StringEncoding;
 import com.google.devtools.common.options.OptionDefinition;
 import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionMetadataTag;
@@ -165,15 +167,15 @@ private Option createOption(
         String combinedForm,
         @Nullable String value) {
       Option.Builder option = Option.newBuilder();
-      option.setCombinedForm(combinedForm);
+      option.setCombinedForm(StringEncoding.internalToUnicode(combinedForm));
       option.setOptionName(optionDefinition.getOptionName());
       if (value != null) {
-        option.setOptionValue(value);
+        option.setOptionValue(StringEncoding.internalToUnicode(value));
       }
       option.addAllEffectTags(getProtoEffectTags(optionDefinition.getOptionEffectTags()));
       option.addAllMetadataTags(getProtoMetadataTags(optionDefinition.getOptionMetadataTags()));
       if (source != null) {
-        option.setSource(source);
+        option.setSource(StringEncoding.internalToUnicode(source));
       }
       return option.build();
     }
@@ -205,10 +207,10 @@ Option createSingleStarlarkOption(String starlarkFlag, @Nullable Object value) {
         }
       }
       Option.Builder option = Option.newBuilder();
-      option.setCombinedForm(sb.toString());
+      option.setCombinedForm(StringEncoding.internalToUnicode(sb.toString()));
       option.setOptionName(starlarkFlag);
       if (value != null) {
-        option.setOptionValue(String.valueOf(value));
+        option.setOptionValue(StringEncoding.internalToUnicode(String.valueOf(value)));
       }
       return option.build();
     }
@@ -244,13 +246,16 @@ CommandLineSection getResidual() {
           CommandLineSection.newBuilder().setSectionLabel("residual");
       if (commandName.equals("run") && !includeResidueInRunBepEvent && !residue.isEmpty()) {
         String target = residue.get(0);
-        ChunkList.Builder residual = ChunkList.newBuilder().addChunk(target);
+        ChunkList.Builder residual =
+            ChunkList.newBuilder().addChunk(StringEncoding.internalToUnicode(target));
         if (residue.size() > 1) {
           residual.addChunk("REDACTED");
         }
         builder.setChunkList(residual);
       } else {
-        builder.setChunkList(ChunkList.newBuilder().addAllChunk(residue));
+        builder.setChunkList(
+            ChunkList.newBuilder()
+                .addAllChunk(Iterables.transform(residue, StringEncoding::internalToUnicode)));
       }
       return builder.build();
     }

src/test/java/com/google/devtools/build/lib/runtime/BUILD

@@ -150,6 +150,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
         "//src/main/java/com/google/devtools/build/lib/util:exit_code",
         "//src/main/java/com/google/devtools/build/lib/util:os",
+        "//src/main/java/com/google/devtools/build/lib/util:string_encoding",
         "//src/main/java/com/google/devtools/build/lib/util/io:io-proto",
         "//src/main/java/com/google/devtools/build/lib/util/io:out-err",
         "//src/main/java/com/google/devtools/build/lib/vfs",

src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java

@@ -31,6 +31,7 @@
 import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.CommandLineSection.SectionTypeCase;
 import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.OptionList;
 import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.lib.util.StringEncoding;
 import com.google.devtools.common.options.OptionPriority.PriorityCategory;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
@@ -342,6 +343,65 @@ public void testOptionsAtVariousPriorities_canonicalCommandLine() throws Options
     assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0);
   }
 
+  @Test
+  public void testNonAsciiOptionValue_canonicalCommandLine() throws OptionsParsingException {
+    OptionsParser fakeStartupOptions =
+        OptionsParser.builder().optionsClasses(BlazeServerStartupOptions.class).build();
+    OptionsParser fakeCommandOptions =
+        OptionsParser.builder().optionsClasses(TestOptions.class).build();
+    // Option values reach the server in Bazel's internal string encoding (see StringEncoding).
+    fakeCommandOptions.parse(
+        PriorityCategory.COMMAND_LINE,
+        "command line",
+        ImmutableList.of("--test_string=" + StringEncoding.unicodeToInternal("¡Buenos días!")));
+
+    CommandLine line =
+        new CanonicalCommandLineEvent(
+                "testblaze",
+                fakeStartupOptions,
+                "someCommandName",
+                fakeCommandOptions.getResidue(),
+                false,
+                ImmutableSortedMap.of(),
+                ImmutableSortedMap.of(),
+                ImmutableSet.of(),
+                fakeCommandOptions.asListOfCanonicalOptions(),
+                /* replaceable= */ false)
+            .asStreamProto(null)
+            .getStructuredCommandLine();
+
+    // The proto fields are Unicode strings, so the value has to be reencoded.
+    assertThat(line.getSections(3).getOptionList().getOption(0).getOptionValue())
+        .isEqualTo("¡Buenos días!");
+    assertThat(line.getSections(3).getOptionList().getOption(0).getCombinedForm())
+        .isEqualTo("--test_string=¡Buenos días!");
+  }
+
+  @Test
+  public void testNonAsciiResidue_canonicalCommandLine() throws OptionsParsingException {
+    OptionsParser fakeStartupOptions =
+        OptionsParser.builder().optionsClasses(BlazeServerStartupOptions.class).build();
+    OptionsParser fakeCommandOptions =
+        OptionsParser.builder().optionsClasses(TestOptions.class).build();
+
+    CommandLine line =
+        new CanonicalCommandLineEvent(
+                "testblaze",
+                fakeStartupOptions,
+                "someCommandName",
+                ImmutableList.of(StringEncoding.unicodeToInternal("//foo:bär")),
+                false,
+                ImmutableSortedMap.of(),
+                ImmutableSortedMap.of(),
+                ImmutableSet.of(),
+                fakeCommandOptions.asListOfCanonicalOptions(),
+                /* replaceable= */ false)
+            .asStreamProto(null)
+            .getStructuredCommandLine();
+
+    assertThat(line.getSections(4).getChunkList().getChunk(0)).isEqualTo("//foo:bär");
+  }
+
   @Test
   public void testExpansionOption_originalCommandLine() throws OptionsParsingException {
     OptionsParser fakeStartupOptions =

src/test/shell/integration/workspace_status_test.sh

@@ -123,4 +123,26 @@ function test_embed_label_must_be_single_line() {
   expect_log "Value must not contain multiple lines"
 }
 
+function test_non_ascii_status_value() {
+  local script="$TEST_TMPDIR/non_ascii.sh"
+  cat > "$script" <<'EOF'
+#!/usr/bin/env bash
+echo "STABLE_GREETING ¡Buenos días!"
+EOF
+  chmod +x "$script"
+
+  bazel build --stamp --workspace_status_command="$script" \
+      --build_event_json_file="$TEST_TMPDIR/bep.json" >& "$TEST_log" \
+      || fail "Build failed"
+
+  # The status file holds the raw bytes emitted by the script.
+  local status_file="$(bazel info output_path)/stable-status.txt"
+  grep -q "STABLE_GREETING ¡Buenos días!" "$status_file" \
+      || fail "Expected UTF-8 bytes in stable-status.txt, got: $(cat "$status_file")"
+
+  # The BEP is a proto with Unicode string fields, so the value must be reencoded.
+  grep -q '¡Buenos días!' "$TEST_TMPDIR/bep.json" \
+      || fail "Expected correctly encoded value in BEP"
+}
+
 run_suite "${PRODUCT_NAME} workspace status command tests"