mirror of
https://github.com/apple/pkl.git
synced 2026-05-25 16:19:20 +02:00
Improve HTTP headers logic (#1584)
* Relax forbidden headers constraints - remove restriction on browser-related headers - allow any glob pattern (no need to end with `/` or `*`, because glob patterns already require users to explicitly declare prefix matches if that's the intention) * Replace `List<Pair<, ...>>`; use `Map<String, ...>` instead * Use glob pattern strings as an API throughout, instead of `Pattern` (e.g. in `HttpClientBuilder`) * Add HTTP headers to message passing API * Add HTTP headers to executor API (introduces `ExecutorSpiOptions4`) * Add tests for Gradle, CLI, and pkl-executor invocations * Improve documentation * Add `isGlobPattern` API to class `String` for in-language validation of http headers * Behavior change: make sure explicitly configured `User-Agent` in `HttpClientBuilder` can be shadowed by headers (allows users to set `--http-header "**=User-Agent: My User Agent"` and for this to be the only user agent). CC @kyokuping
This commit is contained in:
@@ -0,0 +1,68 @@
|
||||
amends "../snippetTest.pkl"
|
||||
|
||||
import "pkl:EvaluatorSettings"
|
||||
|
||||
local function force(http: EvaluatorSettings.Http) = new PcfRenderer {}.renderValue(http)
|
||||
|
||||
examples {
|
||||
["http headers"] {
|
||||
new EvaluatorSettings.Http {
|
||||
headers {
|
||||
["**"] {
|
||||
["X-Foo"] = "Foo"
|
||||
}
|
||||
["https://example.com/**"] {
|
||||
["X-Bar"] {
|
||||
"bar"
|
||||
"baz"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
["http headers -- invalid glob pattern"] {
|
||||
module.catch(() -> force(new EvaluatorSettings.Http {
|
||||
headers {
|
||||
["{{}}"] {
|
||||
["X-Foo"] = "Foo"
|
||||
}
|
||||
}
|
||||
}))
|
||||
}
|
||||
|
||||
["http headers -- invalid header name"] {
|
||||
module.catch(() -> force(new EvaluatorSettings.Http {
|
||||
headers {
|
||||
["**"] {
|
||||
["Connection"] = "Foo"
|
||||
}
|
||||
}
|
||||
}))
|
||||
module.catch(() -> force(new EvaluatorSettings.Http {
|
||||
headers {
|
||||
["**"] {
|
||||
["Sec-Foo-Bar"] = "Foo"
|
||||
}
|
||||
}
|
||||
}))
|
||||
|
||||
module.catch(() -> force(new EvaluatorSettings.Http {
|
||||
headers {
|
||||
["**"] {
|
||||
["Foo:Bar"] = "Foo"
|
||||
}
|
||||
}
|
||||
}))
|
||||
}
|
||||
|
||||
["http headers -- invalid header value"] {
|
||||
module.catch(() -> force(new EvaluatorSettings.Http {
|
||||
headers {
|
||||
["**"] {
|
||||
["My-Header"] = "🙃"
|
||||
}
|
||||
}
|
||||
}))
|
||||
}
|
||||
}
|
||||
@@ -85,6 +85,12 @@ facts {
|
||||
!"(abc".isRegex
|
||||
}
|
||||
|
||||
["isGlobPattern"] {
|
||||
"**".isGlobPattern
|
||||
"hello".isGlobPattern
|
||||
!"{{}}".isGlobPattern
|
||||
}
|
||||
|
||||
["toBoolean()"] {
|
||||
"true".toBoolean()
|
||||
"tRuE".toBoolean()
|
||||
|
||||
+28
@@ -0,0 +1,28 @@
|
||||
examples {
|
||||
["http headers"] {
|
||||
new {
|
||||
headers {
|
||||
["**"] {
|
||||
["X-Foo"] = "Foo"
|
||||
}
|
||||
["https://example.com/**"] {
|
||||
["X-Bar"] {
|
||||
"bar"
|
||||
"baz"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
["http headers -- invalid glob pattern"] {
|
||||
"Type constraint `isGlobPattern` violated. Value: \"{{}}\""
|
||||
}
|
||||
["http headers -- invalid header name"] {
|
||||
"Type constraint `isNotReservedHeaderName` violated. Value: \"Connection\""
|
||||
"Type constraint `doesNotStartWithReservedPrefix` violated. Value: \"Sec-Foo-Bar\""
|
||||
"Type constraint `hasValidHeaderNameSyntax` violated. Value: \"Foo:Bar\""
|
||||
}
|
||||
["http headers -- invalid header value"] {
|
||||
"Expected value of type `*Listing<HttpHeaderValue> | HttpHeaderValue`, but got a different `String`. Value: \"🙃\""
|
||||
}
|
||||
}
|
||||
+42
@@ -914,6 +914,27 @@ alias {
|
||||
name = "isRegex"
|
||||
allModifiers = Set()
|
||||
allAnnotations = List()
|
||||
}, "isGlobPattern", new {
|
||||
location {
|
||||
line = XXXX
|
||||
column = 3
|
||||
displayUri = "https://github.com/apple/pkl/blob/$commitId/stdlib/base.pkl#LXXXX"
|
||||
}
|
||||
docComment = """
|
||||
Tells if this string is a valid glob pattern.
|
||||
|
||||
For reference, see
|
||||
<https://pkl-lang.org/main/current/language-reference/index.html#glob-patterns>.
|
||||
"""
|
||||
annotations = List(new {
|
||||
version = "0.32.0"
|
||||
})
|
||||
modifiers = Set()
|
||||
name = "isGlobPattern"
|
||||
allModifiers = Set()
|
||||
allAnnotations = List(new {
|
||||
version = "0.32.0"
|
||||
})
|
||||
}, "isBase64", new {
|
||||
location {
|
||||
line = XXXX
|
||||
@@ -1258,6 +1279,27 @@ alias {
|
||||
name = "isRegex"
|
||||
allModifiers = Set()
|
||||
allAnnotations = List()
|
||||
}, "isGlobPattern", new {
|
||||
location {
|
||||
line = XXXX
|
||||
column = 3
|
||||
displayUri = "https://github.com/apple/pkl/blob/$commitId/stdlib/base.pkl#LXXXX"
|
||||
}
|
||||
docComment = """
|
||||
Tells if this string is a valid glob pattern.
|
||||
|
||||
For reference, see
|
||||
<https://pkl-lang.org/main/current/language-reference/index.html#glob-patterns>.
|
||||
"""
|
||||
annotations = List(new {
|
||||
version = "0.32.0"
|
||||
})
|
||||
modifiers = Set()
|
||||
name = "isGlobPattern"
|
||||
allModifiers = Set()
|
||||
allAnnotations = List(new {
|
||||
version = "0.32.0"
|
||||
})
|
||||
}, "isBase64", new {
|
||||
location {
|
||||
line = XXXX
|
||||
|
||||
@@ -65,6 +65,11 @@ facts {
|
||||
true
|
||||
true
|
||||
}
|
||||
["isGlobPattern"] {
|
||||
true
|
||||
true
|
||||
true
|
||||
}
|
||||
["toBoolean()"] {
|
||||
true
|
||||
true
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
|
||||
* 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.
|
||||
@@ -15,13 +15,17 @@
|
||||
*/
|
||||
package org.pkl.core
|
||||
|
||||
import java.net.URI
|
||||
import java.nio.file.Path
|
||||
import java.time.Duration
|
||||
import org.assertj.core.api.Assertions.assertThat
|
||||
import org.junit.jupiter.api.Test
|
||||
import org.junit.jupiter.api.assertThrows
|
||||
import org.pkl.core.http.LazyHttpClient
|
||||
import org.pkl.core.http.RequestRewritingClient
|
||||
import org.pkl.core.project.Project
|
||||
import org.pkl.core.resource.TestResourceReader
|
||||
import org.pkl.core.util.IoUtils
|
||||
|
||||
class EvaluatorBuilderTest {
|
||||
@Test
|
||||
@@ -93,5 +97,10 @@ class EvaluatorBuilderTest {
|
||||
.isEqualTo(3) // two external readers, one module path
|
||||
assertThat(builder.resourceReaders.find { it.uriScheme == "scheme3" }).isNotNull
|
||||
assertThat(builder.resourceReaders.find { it.uriScheme == "scheme4" }).isNotNull
|
||||
val client = (builder.httpClient as LazyHttpClient).orCreateClient as RequestRewritingClient
|
||||
assertThat(client.headers)
|
||||
.isEqualTo(mapOf(IoUtils.doubleStarGlob to mapOf("X-Foo" to listOf("Foo"))))
|
||||
assertThat(client.rewritesMap)
|
||||
.isEqualTo(mapOf(URI("https://foo.com/") to URI("https://bar.com/")))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
/*
|
||||
* Copyright © 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.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
package org.pkl.core.http
|
||||
|
||||
import org.assertj.core.api.Assertions.assertThat
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
class HttpClientBuilderTest {
|
||||
@Test
|
||||
fun `addHeader merges values for duplicate header names`() {
|
||||
val client =
|
||||
HttpClient.builder()
|
||||
.setHeaders(mapOf("**" to mapOf("X-Foo" to listOf("v1"))))
|
||||
.addHeaders("**", mapOf("X-Foo" to listOf("v2")))
|
||||
.build() as RequestRewritingClient
|
||||
|
||||
val headerValues = client.headers.values.single()["X-Foo"]
|
||||
assertThat(headerValues).containsExactly("v1", "v2")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `addHeader preserves non-overlapping header names`() {
|
||||
val client =
|
||||
HttpClient.builder()
|
||||
.addHeaders("**", mapOf("X-Foo" to listOf("v1")))
|
||||
.addHeaders("**", mapOf("X-Bar" to listOf("v2")))
|
||||
.build() as RequestRewritingClient
|
||||
|
||||
val headerMap = client.headers.values.single()
|
||||
assertThat(headerMap["X-Foo"]).containsExactly("v1")
|
||||
assertThat(headerMap["X-Bar"]).containsExactly("v2")
|
||||
}
|
||||
}
|
||||
@@ -21,12 +21,13 @@ import java.net.http.HttpRequest
|
||||
import java.net.http.HttpRequest.BodyPublishers
|
||||
import java.net.http.HttpResponse.BodyHandlers
|
||||
import java.time.Duration
|
||||
import java.util.regex.Pattern
|
||||
import org.assertj.core.api.Assertions.assertThat
|
||||
import org.assertj.core.api.Assertions.assertThatList
|
||||
import org.junit.jupiter.api.Test
|
||||
import org.pkl.core.Pair as PPair
|
||||
import org.pkl.core.util.GlobResolver
|
||||
import org.pkl.core.util.IoUtils
|
||||
|
||||
@Suppress("UastIncorrectHttpHeaderInspection")
|
||||
class RequestRewritingClientTest {
|
||||
private val captured = RequestCapturingClient()
|
||||
private val client =
|
||||
@@ -36,7 +37,7 @@ class RequestRewritingClientTest {
|
||||
-1,
|
||||
captured,
|
||||
mapOf(URI("https://foo/") to URI("https://bar/")),
|
||||
listOf(),
|
||||
mapOf(),
|
||||
)
|
||||
private val exampleUri = URI("https://example.com/foo/bar.html")
|
||||
private val exampleRequest = HttpRequest.newBuilder(exampleUri).build()
|
||||
@@ -48,6 +49,21 @@ class RequestRewritingClientTest {
|
||||
assertThatList(captured.request.headers().allValues("User-Agent")).containsOnly("Pkl")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `User-Agent from configured headers takes precedence`() {
|
||||
val client =
|
||||
RequestRewritingClient(
|
||||
"Pkl",
|
||||
Duration.ofSeconds(42),
|
||||
-1,
|
||||
captured,
|
||||
mapOf(URI("https://foo/") to URI("https://bar/")),
|
||||
mapOf(IoUtils.doubleStarGlob to mapOf("User-Agent" to listOf("My-User-Agent"))),
|
||||
)
|
||||
client.send(exampleRequest, BodyHandlers.discarding())
|
||||
assertThatList(captured.request.headers().allValues("User-Agent")).containsOnly("My-User-Agent")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `overrides existing User-Agent headers`() {
|
||||
val request =
|
||||
@@ -125,7 +141,7 @@ class RequestRewritingClientTest {
|
||||
fun `rewrites port 0 if test port is set`() {
|
||||
val captured = RequestCapturingClient()
|
||||
val client =
|
||||
RequestRewritingClient("Pkl", Duration.ofSeconds(42), 5000, captured, mapOf(), listOf())
|
||||
RequestRewritingClient("Pkl", Duration.ofSeconds(42), 5000, captured, mapOf(), mapOf())
|
||||
val request = HttpRequest.newBuilder(URI("https://example.com:0")).build()
|
||||
|
||||
client.send(request, BodyHandlers.discarding())
|
||||
@@ -307,8 +323,7 @@ class RequestRewritingClientTest {
|
||||
|
||||
private fun rewrittenRequest(uri: String, rules: Map<URI, URI>): String {
|
||||
val captured = RequestCapturingClient()
|
||||
val client =
|
||||
RequestRewritingClient("Pkl", Duration.ofSeconds(42), -1, captured, rules, listOf())
|
||||
val client = RequestRewritingClient("Pkl", Duration.ofSeconds(42), -1, captured, rules, mapOf())
|
||||
val request = HttpRequest.newBuilder(URI(uri)).build()
|
||||
client.send(request, BodyHandlers.discarding())
|
||||
return captured.request.uri().toString()
|
||||
@@ -324,12 +339,10 @@ class RequestRewritingClientTest {
|
||||
-1,
|
||||
captured,
|
||||
mapOf(),
|
||||
listOf(
|
||||
PPair(Pattern.compile("^https://example\\.com/.*"), listOf(PPair("x-one", "one"))),
|
||||
PPair(
|
||||
Pattern.compile("^https://example\\.com/foo/.*"),
|
||||
listOf(PPair("x-two", "two-a"), PPair("x-two", "two-b")),
|
||||
),
|
||||
mapOf(
|
||||
GlobResolver.toRegexPattern("https://example.com/**") to mapOf("x-one" to listOf("one")),
|
||||
GlobResolver.toRegexPattern("https://example.com/foo/**") to
|
||||
mapOf("x-two" to listOf("two-a", "two-b")),
|
||||
),
|
||||
)
|
||||
val request = HttpRequest.newBuilder(URI("https://example.com/foo/bar")).build()
|
||||
@@ -350,9 +363,9 @@ class RequestRewritingClientTest {
|
||||
-1,
|
||||
captured,
|
||||
mapOf(),
|
||||
listOf(
|
||||
PPair(Pattern.compile("^https://foo\\.com/.*"), listOf(PPair("x-foo", "foo"))),
|
||||
PPair(Pattern.compile("^https://bar\\.com/.*"), listOf(PPair("x-bar", "bar"))),
|
||||
mapOf(
|
||||
GlobResolver.toRegexPattern("https://foo.com/**") to mapOf("x-foo" to listOf("foo")),
|
||||
GlobResolver.toRegexPattern("https://bar.com/**") to mapOf("x-bar" to listOf("bar")),
|
||||
),
|
||||
)
|
||||
val request = HttpRequest.newBuilder(URI("https://example.com/foo/bar")).build()
|
||||
@@ -373,11 +386,9 @@ class RequestRewritingClientTest {
|
||||
-1,
|
||||
captured,
|
||||
mapOf(),
|
||||
listOf(
|
||||
PPair(
|
||||
Pattern.compile("^https://example\\.com/.*"),
|
||||
listOf(PPair("x-foo", "rule-a"), PPair("x-foo", "rule-b")),
|
||||
)
|
||||
mapOf(
|
||||
GlobResolver.toRegexPattern("https://example.com/**") to
|
||||
mapOf("x-foo" to listOf("rule-a", "rule-b"))
|
||||
),
|
||||
)
|
||||
val request =
|
||||
@@ -388,4 +399,27 @@ class RequestRewritingClientTest {
|
||||
assertThatList(captured.request.headers().allValues("x-foo"))
|
||||
.containsExactly("request", "rule-a", "rule-b")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `configured headers wins over configured user-agent header`() {
|
||||
val captured = RequestCapturingClient()
|
||||
val client =
|
||||
RequestRewritingClient(
|
||||
"Pkl",
|
||||
Duration.ofSeconds(42),
|
||||
-1,
|
||||
captured,
|
||||
mapOf(),
|
||||
mapOf(
|
||||
GlobResolver.toRegexPattern("https://example.com/**") to
|
||||
mapOf("user-agent" to listOf("My User Agent"))
|
||||
),
|
||||
)
|
||||
val request = HttpRequest.newBuilder(URI("https://example.com/foo/bar")).build()
|
||||
|
||||
client.send(request, BodyHandlers.discarding())
|
||||
|
||||
assertThatList(captured.request.headers().allValues("user-agent"))
|
||||
.containsExactly("My User Agent")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -26,10 +26,8 @@ import org.pkl.commons.writeString
|
||||
import org.pkl.core.Evaluator
|
||||
import org.pkl.core.ModuleSource
|
||||
import org.pkl.core.PObject
|
||||
import org.pkl.core.Pair as PPair
|
||||
import org.pkl.core.evaluatorSettings.PklEvaluatorSettings
|
||||
import org.pkl.core.settings.PklSettings.Editor
|
||||
import org.pkl.core.util.GlobResolver
|
||||
|
||||
class PklSettingsTest {
|
||||
@Test
|
||||
@@ -67,9 +65,15 @@ class PklSettingsTest {
|
||||
["https://foo.com/"] = "https://bar.com/"
|
||||
}
|
||||
headers {
|
||||
["https://foo.com/"] {
|
||||
["https://foo.com/**"] {
|
||||
["x-foo"] = "bar"
|
||||
}
|
||||
["https://bar.com/**"] {
|
||||
["x-bar"] {
|
||||
"bar"
|
||||
"baz"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
"""
|
||||
@@ -84,8 +88,9 @@ class PklSettingsTest {
|
||||
listOf("example.com", "pkg.pkl-lang.org"),
|
||||
),
|
||||
mapOf(URI("https://foo.com/") to URI("https://bar.com/")),
|
||||
listOf(
|
||||
PPair(GlobResolver.toRegexPattern("https://foo.com/"), listOf(PPair("x-foo", "bar")))
|
||||
mapOf(
|
||||
"https://foo.com/**" to mapOf("x-foo" to listOf("bar")),
|
||||
"https://bar.com/**" to mapOf("x-bar" to listOf("bar", "baz")),
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
@@ -41,4 +41,20 @@ evaluatorSettings {
|
||||
arguments { "with"; "args" }
|
||||
}
|
||||
}
|
||||
http {
|
||||
headers {
|
||||
["**"] {
|
||||
["X-Foo"] = "Foo"
|
||||
}
|
||||
}
|
||||
rewrites {
|
||||
["https://foo.com/"] = "https://bar.com/"
|
||||
}
|
||||
proxy {
|
||||
noProxy {
|
||||
"my-no-proxy"
|
||||
}
|
||||
address = "http://localhost:5619"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user