diff --git a/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java b/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java index 24c5f89e9b1f3b..83d0685264fa80 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java @@ -38,7 +38,9 @@ import com.google.devtools.build.lib.remote.util.Utils; import com.google.devtools.build.lib.vfs.Path; import com.google.gson.Gson; +import com.google.gson.JsonArray; import com.google.gson.JsonObject; +import com.google.gson.JsonPrimitive; import io.grpc.CallCredentials; import io.grpc.Channel; import io.grpc.StatusRuntimeException; @@ -129,7 +131,13 @@ public void download( RemoteActionExecutionContext.create(metadata); final FetchBlobRequest request = - newFetchBlobRequest(options.remoteInstanceName, urls, authHeaders, checksum, canonicalId); + newFetchBlobRequest( + options.remoteInstanceName, + urls, + authHeaders, + checksum, + canonicalId, + options.remoteDownloaderSendAllHeaders); try { FetchBlobResponse response = retrier.execute( @@ -169,7 +177,8 @@ static FetchBlobRequest newFetchBlobRequest( List urls, Map>> authHeaders, com.google.common.base.Optional checksum, - String canonicalId) { + String canonicalId, + boolean includeAllHeaders) { FetchBlobRequest.Builder requestBuilder = FetchBlobRequest.newBuilder().setInstanceName(instanceName); for (URL url : urls) { @@ -190,7 +199,7 @@ static FetchBlobRequest newFetchBlobRequest( requestBuilder.addQualifiers( Qualifier.newBuilder() .setName(QUALIFIER_AUTH_HEADERS) - .setValue(authHeadersJson(authHeaders)) + .setValue(authHeadersJson(authHeaders, includeAllHeaders)) .build()); } @@ -216,16 +225,23 @@ private OutputStream newOutputStream( return out; } - private static String authHeadersJson(Map>> authHeaders) { + private static String authHeadersJson(Map>> authHeaders, boolean includeAllHeaders) { Map subObjects = new TreeMap<>(); for (Map.Entry>> entry : authHeaders.entrySet()) { JsonObject subObject = new JsonObject(); Map> orderedHeaders = new TreeMap<>(entry.getValue()); for (Map.Entry> subEntry : orderedHeaders.entrySet()) { - // TODO(yannic): Introduce incompatible flag for including all headers, not just the first. - String value = Iterables.getFirst(subEntry.getValue(), null); - if (value != null) { - subObject.addProperty(subEntry.getKey(), value); + if (includeAllHeaders) { + JsonArray values = new JsonArray(subEntry.getValue().size()); + for (String value : subEntry.getValue()) { + values.add(value); + } + subObject.add(subEntry.getKey(), values); + } else { + String value = Iterables.getFirst(subEntry.getValue(), null); + if (value != null) { + subObject.addProperty(subEntry.getKey(), value); + } } } subObjects.put(entry.getKey().toString(), subObject); diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index 101641aaf5c0fa..db1f85f10ba080 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -608,6 +608,15 @@ public RemoteOutputsStrategyConverter() { + "`all` to print always.") public ExecutionMessagePrintMode remotePrintExecutionMessages; + @Option( + name = "incompatible_remote_downloader_send_all_headers", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, + help = "Whether to send all values of a multi-valued header to the remote downloader instead of just the first.") + public boolean remoteDownloaderSendAllHeaders; + // The below options are not configurable by users, only tests. // This is part of the effort to reduce the overall number of flags. diff --git a/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java index df90c90e8abc1c..9f2696aa4c6c2d 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java @@ -354,13 +354,14 @@ public void testFetchBlobRequest() throws Exception { new URI("http://example.com"), ImmutableMap.of( "Some-Header", ImmutableList.of("some header content"), - "Another-Header", ImmutableList.of("another header content")), + "Another-Header", ImmutableList.of("another header content", "even more header content")), new URI("http://example.org"), - ImmutableMap.of("Org-Header", ImmutableList.of("org header content"))), + ImmutableMap.of("Org-Header", ImmutableList.of("org header content", "and a second one", "and a third one"))), com.google.common.base.Optional.of( Checksum.fromSubresourceIntegrity( "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")), - "canonical ID"); + "canonical ID", + /* includeAllHeaders= */ false); final String expectedAuthHeadersJson = "{" @@ -392,4 +393,57 @@ public void testFetchBlobRequest() throws Exception { .setValue(expectedAuthHeadersJson)) .build()); } + + @Test + public void testFetchBlobRequestWithAllHeaders() throws Exception { + FetchBlobRequest request = + GrpcRemoteDownloader.newFetchBlobRequest( + "instance name", + ImmutableList.of( + new URL("http://example.com/a"), + new URL("http://example.com/b"), + new URL("file:/not/limited/to/http")), + ImmutableMap.of( + new URI("http://example.com"), + ImmutableMap.of( + "Some-Header", ImmutableList.of("some header content"), + "Another-Header", ImmutableList.of("another header content", "even more header content")), + new URI("http://example.org"), + ImmutableMap.of("Org-Header", ImmutableList.of("org header content", "and a second one", "and a third one"))), + com.google.common.base.Optional.of( + Checksum.fromSubresourceIntegrity( + "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")), + "canonical ID", + /* includeAllHeaders= */ true); + + final String expectedAuthHeadersJson = + "{" + + "\"http://example.com\":{" + + "\"Another-Header\":[\"another header content\",\"even more header content\"]," + + "\"Some-Header\":[\"some header content\"]" + + "}," + + "\"http://example.org\":{" + + "\"Org-Header\":[\"org header content\",\"and a second one\",\"and a third one\"]" + + "}" + + "}"; + + assertThat(request) + .isEqualTo( + FetchBlobRequest.newBuilder() + .setInstanceName("instance name") + .addUris("http://example.com/a") + .addUris("http://example.com/b") + .addUris("file:/not/limited/to/http") + .addQualifiers( + Qualifier.newBuilder() + .setName("checksum.sri") + .setValue("sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")) + .addQualifiers( + Qualifier.newBuilder().setName("bazel.canonical_id").setValue("canonical ID")) + .addQualifiers( + Qualifier.newBuilder() + .setName("bazel.auth_headers") + .setValue(expectedAuthHeadersJson)) + .build()); + } }