Use java.net.http.HttpClient instead of java.net.Http(s)URLConnection (#217)

Moving to java.net.http.HttpClient brings many benefits, including
HTTP/2 support and the ability to make asynchronous requests.

Major additions and changes:
- Introduce a lightweight org.pkl.core.http.HttpClient API.
  This keeps some flexibility and allows to enforce behavior
  such as setting the User-Agent header.
- Provide an implementation that delegates to java.net.http.HttpClient.
- Use HttpClient for all HTTP(s) requests across the codebase.
  This required adding an HttpClient parameter to constructors and
  factory methods of multiple classes, some of which are public APIs.
- Manage CA certificates per HTTP client instead of per JVM.
  This makes it unnecessary to set JVM-wide system/security properties
  and default SSLSocketFactory's.
- Add executor v2 options to the executor SPI
- Add pkl-certs as a new artifact, and remove certs from pkl-commons-cli artifact

Each HTTP client maintains its own connection pool and SSLContext.
For efficiency reasons, It's best to reuse clients whenever feasible.
To avoid memory leaks, clients are not stored in static fields.

HTTP clients are expensive to create. For this reason,
EvaluatorBuilder defaults to a "lazy" client that creates the underlying
java.net.http.HttpClient on the first send (which may never happen).
This commit is contained in:
translatenix
2024-03-06 10:25:56 -08:00
committed by GitHub
parent 106743354c
commit 3f3dfdeb1e
79 changed files with 2376 additions and 395 deletions

View File

@@ -3,6 +3,7 @@ package org.pkl.core
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.pkl.commons.toPath
import org.pkl.core.http.HttpClient
import org.pkl.core.module.ModuleKeyFactories
import org.pkl.core.repl.ReplRequest
import org.pkl.core.repl.ReplResponse
@@ -12,6 +13,7 @@ import org.pkl.core.resource.ResourceReaders
class ReplServerTest {
private val server = ReplServer(
SecurityManagers.defaultManager,
HttpClient.dummyClient(),
Loggers.stdErr(),
listOf(
ModuleKeyFactories.standardLibrary,

View File

@@ -0,0 +1,34 @@
package org.pkl.core.http
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertDoesNotThrow
import org.junit.jupiter.api.assertThrows
import java.net.URI
import java.net.http.HttpRequest
import java.net.http.HttpResponse
class DummyHttpClientTest {
@Test
fun `refuses to send messages`() {
val client = HttpClient.dummyClient()
val request = HttpRequest.newBuilder(URI("https://example.com")).build()
assertThrows<AssertionError> {
client.send(request, HttpResponse.BodyHandlers.discarding())
}
assertThrows<AssertionError> {
client.send(request, HttpResponse.BodyHandlers.discarding())
}
}
@Test
fun `can be closed`() {
val client = HttpClient.dummyClient()
assertDoesNotThrow {
client.close()
client.close()
}
}
}

View File

@@ -0,0 +1,148 @@
package org.pkl.core.http
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertDoesNotThrow
import org.junit.jupiter.api.assertThrows
import org.junit.jupiter.api.io.TempDir
import org.pkl.commons.test.FileTestUtils
import org.pkl.core.Release
import java.net.URI
import java.net.http.HttpRequest
import java.net.http.HttpResponse
import java.nio.file.Path
import java.time.Duration
import kotlin.io.path.copyTo
import kotlin.io.path.createDirectories
import kotlin.io.path.createFile
class HttpClientTest {
@Test
fun `can build default client`() {
val client = assertDoesNotThrow {
HttpClient.builder().build()
}
assertThat(client).isInstanceOf(RequestRewritingClient::class.java)
client as RequestRewritingClient
val release = Release.current()
assertThat(client.userAgent).isEqualTo("Pkl/${release.version()} (${release.os()}; ${release.flavor()})")
assertThat(client.requestTimeout).isEqualTo(Duration.ofSeconds(60))
assertThat(client.delegate).isInstanceOf(JdkHttpClient::class.java)
val delegate = client.delegate as JdkHttpClient
assertThat(delegate.underlying.connectTimeout()).hasValue(Duration.ofSeconds(60))
}
@Test
fun `can build custom client`() {
val client = HttpClient.builder()
.setUserAgent("Agent 1")
.setRequestTimeout(Duration.ofHours(86))
.setConnectTimeout(Duration.ofMinutes(42))
.build() as RequestRewritingClient
assertThat(client.userAgent).isEqualTo("Agent 1")
assertThat(client.requestTimeout).isEqualTo(Duration.ofHours(86))
val delegate = client.delegate as JdkHttpClient
assertThat(delegate.underlying.connectTimeout()).hasValue(Duration.ofMinutes(42))
}
@Test
fun `can load certificates from file system`() {
assertDoesNotThrow {
HttpClient.builder().addCertificates(FileTestUtils.selfSignedCertificate).build()
}
}
@Test
fun `certificate file located on file system cannot be empty`(@TempDir tempDir: Path) {
val file = tempDir.resolve("certs.pem").createFile()
val e = assertThrows<HttpClientInitException> {
HttpClient.builder().addCertificates(file).build()
}
assertThat(e).hasMessageContaining("empty")
}
@Test
fun `can load certificates from class path`() {
assertDoesNotThrow {
HttpClient.builder().addCertificates(javaClass.getResource("/org/pkl/certs/PklCARoots.pem")!!.toURI()).build()
}
}
@Test
fun `only allows loading jar and file certificate URIs`() {
assertThrows<HttpClientInitException> {
HttpClient.builder().addCertificates(URI("https://example.com"))
}
}
@Test
fun `certificate file located on class path cannot be empty`() {
val uri = javaClass.getResource("emptyCerts.pem")!!.toURI()
val e = assertThrows<HttpClientInitException> {
HttpClient.builder().addCertificates(uri).build()
}
assertThat(e).hasMessageContaining("empty")
}
@Test
fun `can load built-in certificates`() {
assertDoesNotThrow {
HttpClient.builder().addBuiltInCertificates().build()
}
}
@Test
fun `can load certificates from Pkl user home cacerts directory`(@TempDir tempDir: Path) {
val certFile = tempDir.resolve(".pkl")
.resolve("cacerts")
.createDirectories()
.resolve("certs.pem")
FileTestUtils.selfSignedCertificate.copyTo(certFile)
assertDoesNotThrow {
HttpClientBuilder(tempDir).addDefaultCliCertificates().build()
}
}
@Test
fun `loading certificates from cacerts directory falls back to built-in certificates`(@TempDir userHome: Path) {
assertDoesNotThrow {
HttpClientBuilder(userHome).addDefaultCliCertificates().build()
}
}
@Test
fun `can be closed multiple times`() {
val client = HttpClient.builder().build()
assertDoesNotThrow {
client.close()
client.close()
}
}
@Test
fun `refuses to send messages once closed`() {
val client = HttpClient.builder().build()
val request = HttpRequest.newBuilder(URI("https://example.com")).build()
client.close()
assertThrows<IllegalStateException> {
client.send(request, HttpResponse.BodyHandlers.discarding())
}
assertThrows<IllegalStateException> {
client.send(request, HttpResponse.BodyHandlers.discarding())
}
}
}

View File

@@ -0,0 +1,34 @@
package org.pkl.core.http
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertDoesNotThrow
import org.junit.jupiter.api.assertThrows
import java.net.URI
import java.net.http.HttpRequest
import java.net.http.HttpResponse.BodyHandlers
class LazyHttpClientTest {
@Test
fun `builds underlying client on first send`() {
val client = HttpClient.builder()
.addCertificates(javaClass.getResource("brokenCerts.pem")!!.toURI())
.buildLazily()
val request = HttpRequest.newBuilder(URI("https://example.com")).build()
assertThrows<HttpClientInitException> {
client.send(request, BodyHandlers.discarding())
}
}
@Test
fun `does not build underlying client unnecessarily`() {
val client = HttpClient.builder()
.addCertificates(javaClass.getResource("brokenCerts.pem")!!.toURI())
.buildLazily()
assertDoesNotThrow {
client.close()
client.close()
}
}
}

View File

@@ -0,0 +1,18 @@
package org.pkl.core.http
import java.net.http.HttpRequest
import java.net.http.HttpResponse
class RequestCapturingClient : HttpClient {
lateinit var request: HttpRequest
override fun <T : Any?> send(
request: HttpRequest,
responseBodyHandler: HttpResponse.BodyHandler<T>
): HttpResponse<T>? {
this.request = request
return null
}
override fun close() {}
}

View File

@@ -0,0 +1,108 @@
package org.pkl.core.http
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatList
import org.junit.jupiter.api.Test
import java.net.URI
import java.net.http.HttpRequest
import java.net.http.HttpRequest.BodyPublishers
import java.net.http.HttpResponse
import java.net.http.HttpResponse.BodyHandlers
import java.time.Duration
import java.net.http.HttpClient as JdkHttpClient
class RequestRewritingClientTest {
private val captured = RequestCapturingClient()
private val client = RequestRewritingClient("Pkl", Duration.ofSeconds(42), captured)
private val exampleUri = URI("https://example.com/foo/bar.html")
private val exampleRequest = HttpRequest.newBuilder(exampleUri).build()
@Test
fun `fills in missing User-Agent header`() {
client.send(exampleRequest, BodyHandlers.discarding())
assertThatList(captured.request.headers().allValues("User-Agent")).containsOnly("Pkl")
}
@Test
fun `overrides existing User-Agent headers`() {
val request = HttpRequest.newBuilder(exampleUri)
.header("User-Agent", "Agent 1")
.header("User-Agent", "Agent 2")
.build()
client.send(request, BodyHandlers.discarding())
assertThatList(captured.request.headers().allValues("User-Agent")).containsOnly("Pkl")
}
@Test
fun `fills in missing request timeout`() {
client.send(exampleRequest, BodyHandlers.discarding())
assertThat(captured.request.timeout()).hasValue(Duration.ofSeconds(42))
}
@Test
fun `leaves existing request timeout intact`() {
val request = HttpRequest.newBuilder(exampleUri)
.timeout(Duration.ofMinutes(33))
.build()
client.send(request, BodyHandlers.discarding())
assertThat(captured.request.timeout()).hasValue(Duration.ofMinutes(33))
}
@Test
fun `fills in missing HTTP version`() {
client.send(exampleRequest, BodyHandlers.discarding())
assertThat(captured.request.version()).hasValue(JdkHttpClient.Version.HTTP_2)
}
@Test
fun `leaves existing HTTP version intact`() {
val request = HttpRequest.newBuilder(exampleUri)
.version(JdkHttpClient.Version.HTTP_1_1)
.build()
client.send(request, BodyHandlers.discarding())
assertThat(captured.request.version()).hasValue(JdkHttpClient.Version.HTTP_1_1)
}
@Test
fun `leaves default method intact`() {
val request = HttpRequest.newBuilder(exampleUri).build()
client.send(request, BodyHandlers.discarding())
assertThat(captured.request.method()).isEqualTo("GET")
}
@Test
fun `leaves explicit method intact`() {
val request = HttpRequest.newBuilder(exampleUri)
.DELETE()
.build()
client.send(request, BodyHandlers.discarding())
assertThat(captured.request.method()).isEqualTo("DELETE")
}
@Test
fun `leaves body publisher intact`() {
val publisher = BodyPublishers.ofString("body")
val request = HttpRequest.newBuilder(exampleUri)
.PUT(publisher)
.build()
client.send(request, BodyHandlers.discarding())
assertThat(captured.request.bodyPublisher().get()).isSameAs(publisher)
}
}

View File

@@ -13,9 +13,9 @@ import org.pkl.commons.readString
import org.pkl.commons.test.FileTestUtils
import org.pkl.commons.test.PackageServer
import org.pkl.commons.test.listFilesRecursively
import org.pkl.core.http.HttpClient
import org.pkl.core.SecurityManagers
import org.pkl.core.module.PathElement
import org.pkl.core.runtime.CertificateUtils
import java.io.FileNotFoundException
import java.io.IOException
import java.nio.charset.StandardCharsets
@@ -34,9 +34,12 @@ class PackageResolversTest {
@JvmStatic
@BeforeAll
fun beforeAll() {
CertificateUtils.setupAllX509CertificatesGlobally(listOf(FileTestUtils.selfSignedCertificate))
PackageServer.ensureStarted()
}
val httpClient: HttpClient = HttpClient.builder()
.addCertificates(FileTestUtils.selfSignedCertificate)
.build()
}
@Test
@@ -196,16 +199,17 @@ class PackageResolversTest {
@BeforeAll
@JvmStatic
fun beforeAll() {
CertificateUtils.setupAllX509CertificatesGlobally(listOf(FileTestUtils.selfSignedCertificate))
PackageServer.ensureStarted()
cacheDir.deleteRecursively()
}
}
override val resolver: PackageResolver = PackageResolvers.DiskCachedPackageResolver(SecurityManagers.defaultManager, cacheDir)
override val resolver: PackageResolver = PackageResolvers.DiskCachedPackageResolver(
SecurityManagers.defaultManager, httpClient, cacheDir)
}
class InMemoryPackageResolverTest : AbstractPackageResolverTest() {
override val resolver: PackageResolver = PackageResolvers.InMemoryPackageResolver(SecurityManagers.defaultManager)
override val resolver: PackageResolver = PackageResolvers.InMemoryPackageResolver(
SecurityManagers.defaultManager, httpClient)
}
}

View File

@@ -6,10 +6,10 @@ import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
import org.pkl.commons.test.FileTestUtils
import org.pkl.commons.test.PackageServer
import org.pkl.core.http.HttpClient
import org.pkl.core.PklException
import org.pkl.core.SecurityManagers
import org.pkl.core.packages.PackageResolver
import org.pkl.core.runtime.CertificateUtils
import java.io.ByteArrayOutputStream
import java.nio.charset.StandardCharsets
import java.nio.file.Path
@@ -19,16 +19,19 @@ class ProjectDependenciesResolverTest {
@JvmStatic
@BeforeAll
fun beforeAll() {
CertificateUtils.setupAllX509CertificatesGlobally(listOf(FileTestUtils.selfSignedCertificate))
PackageServer.ensureStarted()
}
val httpClient: HttpClient = HttpClient.builder()
.addCertificates(FileTestUtils.selfSignedCertificate)
.build()
}
@Test
fun resolveDependencies() {
val project2Path = Path.of(javaClass.getResource("project2/PklProject")!!.path)
val project = Project.loadFromPath(project2Path)
val packageResolver = PackageResolver.getInstance(SecurityManagers.defaultManager, null)
val packageResolver = PackageResolver.getInstance(SecurityManagers.defaultManager, httpClient, null)
val deps = ProjectDependenciesResolver(project, packageResolver, System.out.writer()).resolve()
val strDeps = ByteArrayOutputStream()
.apply { deps.writeTo(this) }
@@ -66,7 +69,7 @@ class ProjectDependenciesResolverTest {
fun `fails if project declares a package with an incorrect checksum`() {
val projectPath = Path.of(javaClass.getResource("badProjectChecksum/PklProject")!!.path)
val project = Project.loadFromPath(projectPath)
val packageResolver = PackageResolver.getInstance(SecurityManagers.defaultManager, null)
val packageResolver = PackageResolver.getInstance(SecurityManagers.defaultManager, httpClient, null)
val e = assertThrows<PklException> {
ProjectDependenciesResolver(project, packageResolver, System.err.writer()).resolve()
}

View File

@@ -9,6 +9,8 @@ import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatCode
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.io.TempDir
import org.pkl.commons.test.FileTestUtils
import org.pkl.core.http.HttpClient
import java.net.URI
import java.nio.file.Path
import java.util.regex.Pattern
@@ -137,9 +139,13 @@ class ProjectTest {
PackageServer.ensureStarted()
val projectDir = Path.of(javaClass.getResource("badProjectChecksum2/")!!.path)
val project = Project.loadFromPath(projectDir.resolve("PklProject"))
val httpClient = HttpClient.builder()
.addCertificates(FileTestUtils.selfSignedCertificate)
.build()
val evaluator = EvaluatorBuilder.preconfigured()
.applyFromProject(project)
.setModuleCacheDir(null)
.setHttpClient(httpClient)
.build()
assertThatCode { evaluator.evaluate(ModuleSource.path(projectDir.resolve("bug.pkl"))) }
.hasMessageStartingWith("""

View File

@@ -0,0 +1,48 @@
package org.pkl.core.util
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import java.io.IOException
import java.lang.Error
class ExceptionsTest {
@Test
fun `get root cause of simple exception`() {
val e = IOException("io")
assertThat(Exceptions.getRootCause(e)).isSameAs(e)
}
@Test
fun `get root cause of nested exception`() {
val e = IOException("io")
val e2 = RuntimeException("runtime")
val e3 = Error("error")
e.initCause(e2)
e2.initCause(e3)
assertThat(Exceptions.getRootCause(e)).isSameAs(e3)
}
@Test
fun `get root reason`() {
val e = IOException("io")
val e2 = RuntimeException("the root reason")
e.initCause(e2)
assertThat(Exceptions.getRootReason(e)).isEqualTo("the root reason")
}
@Test
fun `get root reason if null`() {
val e = IOException("io")
val e2 = RuntimeException(null as String?)
e.initCause(e2)
assertThat(Exceptions.getRootReason(e)).isEqualTo("(unknown reason)")
}
@Test
fun `get root reason if empty`() {
val e = IOException("io")
val e2 = RuntimeException("")
e.initCause(e2)
assertThat(Exceptions.getRootReason(e)).isEqualTo("(unknown reason)")
}
}

View File

@@ -0,0 +1,42 @@
package org.pkl.core.util
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertDoesNotThrow
import org.junit.jupiter.api.assertThrows
import org.pkl.commons.test.FakeHttpResponse
import java.io.IOException
import java.net.URI
import java.net.URL
class HttpUtilsTest {
@Test
fun isHttpUrl() {
assertThat(HttpUtils.isHttpUrl(URI("http://example.com"))).isTrue
assertThat(HttpUtils.isHttpUrl(URI("https://example.com"))).isTrue
assertThat(HttpUtils.isHttpUrl(URI("HtTpS://example.com"))).isTrue
assertThat(HttpUtils.isHttpUrl(URI("file://example.com"))).isFalse
assertThat(HttpUtils.isHttpUrl(URL("http://example.com"))).isTrue
assertThat(HttpUtils.isHttpUrl(URL("https://example.com"))).isTrue
assertThat(HttpUtils.isHttpUrl(URL("HtTpS://example.com"))).isTrue
assertThat(HttpUtils.isHttpUrl(URL("file://example.com"))).isFalse
}
@Test
fun checkHasStatusCode200() {
val response = FakeHttpResponse.withoutBody {
statusCode = 200
}
assertDoesNotThrow {
HttpUtils.checkHasStatusCode200(response)
}
val response2 = FakeHttpResponse.withoutBody {
statusCode = 404
}
assertThrows<IOException> {
HttpUtils.checkHasStatusCode200(response2)
}
}
}

View File

@@ -14,6 +14,7 @@ import org.pkl.core.runtime.ModuleResolver
import java.io.FileNotFoundException
import java.net.URI
import java.net.URISyntaxException
import java.net.URL
import java.nio.file.Path
import kotlin.io.path.createFile
@@ -410,4 +411,24 @@ class IoUtilsTest {
IoUtils.resolve(FakeSecurityManager, key2, URI("...NamedModuleResolversTest.pkl"))
}
}
@Test
fun `readBytes(URL) does not support HTTP URLs`() {
assertThrows<IllegalArgumentException> {
IoUtils.readBytes(URL("https://example.com"))
}
assertThrows<IllegalArgumentException> {
IoUtils.readBytes(URL("http://example.com"))
}
}
@Test
fun `readString(URL) does not support HTTP URLs`() {
assertThrows<IllegalArgumentException> {
IoUtils.readString(URL("https://example.com"))
}
assertThrows<IllegalArgumentException> {
IoUtils.readString(URL("http://example.com"))
}
}
}

View File

@@ -0,0 +1 @@
broken