Add support for customizing HTTP headers #1196
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* | ||
| * Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
|
|
@@ -23,7 +23,9 @@ | |
| import java.nio.file.Path; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.regex.Pattern; | ||
| import javax.net.ssl.SSLContext; | ||
| import org.pkl.core.Pair; | ||
| import org.pkl.core.util.Nullable; | ||
|
|
||
| /** | ||
|
|
@@ -150,6 +152,14 @@ interface Builder { | |
| */ | ||
| Builder addRewrite(URI sourcePrefix, URI targetPrefix); | ||
|
|
||
| /** | ||
| * Sets the HTTP headers for the request, replacing any previously configured headers. | ||
| * | ||
| * <p>This method clears all existing headers and replaces them with the contents of the | ||
| * provided map. | ||
| */ | ||
| Builder setHeaders(List<Pair<Pattern, List<Pair<String, String>>>> headers); | ||
|
|
||
| /** | ||
| * Creates a new {@code HttpClient} from the current state of this builder. | ||
| * | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* | ||
| * Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
|
|
@@ -27,6 +27,8 @@ | |
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.function.Supplier; | ||
| import java.util.regex.Pattern; | ||
| import org.pkl.core.Pair; | ||
| import org.pkl.core.Release; | ||
| import org.pkl.core.http.HttpClient.Builder; | ||
|
|
||
|
|
@@ -39,6 +41,7 @@ final class HttpClientBuilder implements HttpClient.Builder { | |
| private int testPort = -1; | ||
| private ProxySelector proxySelector; | ||
| private Map<URI, URI> rewrites = new HashMap<>(); | ||
| private List<Pair<Pattern, List<Pair<String, String>>>> headers = new ArrayList<>(); | ||
|
|
||
| HttpClientBuilder() { | ||
| var release = Release.current(); | ||
|
|
@@ -110,6 +113,12 @@ public Builder addRewrite(URI sourcePrefix, URI targetPrefix) { | |
| return this; | ||
| } | ||
|
|
||
| @Override | ||
| public Builder setHeaders(List<Pair<Pattern, List<Pair<String, String>>>> headers) { | ||
|
Member
There was a problem hiding this comment. I think it would be better if this signature matched the in-language evaluator settings. I think this should be something like: setHeaderRules(Map<String, Map<String, List<String>>> headerRules)Where the top-level map key is the glob pattern. And, I think we should also have: addHeaderRule(String globPattern, Map<String, List<String>> headers)It'd also be good to avoid using |
||
| this.headers = headers; | ||
| return this; | ||
| } | ||
|
|
||
| @Override | ||
| public HttpClient build() { | ||
| return doBuild().get(); | ||
|
|
@@ -128,7 +137,8 @@ private Supplier<HttpClient> doBuild() { | |
| return () -> { | ||
| var jdkClient = | ||
| new JdkHttpClient(certificateFiles, certificateBytes, connectTimeout, proxySelector); | ||
| return new RequestRewritingClient( | ||
| userAgent, requestTimeout, testPort, jdkClient, rewrites, headers); | ||
| }; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* | ||
| * Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
|
|
@@ -28,7 +28,10 @@ | |
| import java.util.Map.Entry; | ||
| import java.util.Objects; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import java.util.regex.Pattern; | ||
| import java.util.stream.Stream; | ||
| import javax.annotation.concurrent.ThreadSafe; | ||
| import org.pkl.core.Pair; | ||
| import org.pkl.core.PklBugException; | ||
| import org.pkl.core.util.HttpUtils; | ||
| import org.pkl.core.util.Nullable; | ||
|
|
@@ -54,6 +57,7 @@ final class RequestRewritingClient implements HttpClient { | |
| final int testPort; | ||
| final HttpClient delegate; | ||
| private final List<Entry<URI, URI>> rewrites; | ||
| private final List<Pair<Pattern, List<Pair<String, String>>>> headers; | ||
|
|
||
| private final AtomicBoolean closed = new AtomicBoolean(); | ||
|
|
||
|
|
@@ -62,7 +66,8 @@ final class RequestRewritingClient implements HttpClient { | |
| Duration requestTimeout, | ||
| int testPort, | ||
| HttpClient delegate, | ||
| Map<URI, URI> rewrites, | ||
| List<Pair<Pattern, List<Pair<String, String>>>> headers) { | ||
| this.userAgent = userAgent; | ||
| this.requestTimeout = requestTimeout; | ||
| this.testPort = testPort; | ||
|
|
@@ -72,6 +77,7 @@ final class RequestRewritingClient implements HttpClient { | |
| .map((it) -> Map.entry(normalizeRewrite(it.getKey()), normalizeRewrite(it.getValue()))) | ||
| .sorted(Comparator.comparingInt((it) -> -it.getKey().toString().length())) | ||
| .toList(); | ||
| this.headers = headers; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -112,6 +118,9 @@ private HttpRequest rewriteRequest(HttpRequest original) { | |
| .map() | ||
| .forEach((name, values) -> values.forEach(value -> builder.header(name, value))); | ||
| builder.setHeader("User-Agent", userAgent); | ||
| for (var header : this.getHeaders(original.uri())) { | ||
| builder.header(header.getFirst(), header.getSecond()); | ||
| } | ||
|
|
||
| var method = original.method(); | ||
| original | ||
|
|
@@ -216,6 +225,16 @@ private URI rewriteUri(URI uri) { | |
| return ret; | ||
| } | ||
|
|
||
| private List<Pair<String, String>> getHeaders(URI uri) { | ||
| return headers.stream() | ||
| .flatMap( | ||
| rule -> | ||
| rule.getFirst().asPredicate().test(uri.toString()) | ||
| ? rule.getSecond().stream() | ||
| : Stream.empty()) | ||
| .toList(); | ||
| } | ||
|
|
||
| private void checkNotClosed(HttpRequest request) { | ||
| if (closed.get()) { | ||
| throw new IllegalStateException( | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more of a nit, but:
List<PPair<Pattern, List<PPair<String, String>>>>is a quite generic type that doesn't convey much information. It would be better to have a properly-named record that stores the URL glob and the header name-value pairs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this type's gotten a bit out of hand and a record would be a welcome improvement. I'm not eager to block this PR more than necessary, so I'm fine leaving this as follow-up work.