Skip to content

Commit 1585ffa

Browse files
committed
GH-5723: address further feedback
JDKHttpClientResponse getReasonPhrase() now returns the standard IANA phrase for all common codes (covering everything likely to appear in SPARQL/RDF4J error paths), and falls back to "HTTP <statusCode>" for anything non-standard — always meaningful rather than empty. SPARQLProtocolSessionTest: added sessionManager field; createProtocolSession() now assigns this.sessionManager; tearDown() calls sessionManager.shutDown() after closing the session. - RDF4JProtocolSessionTest: override does the same — assigns sessionManager (inherited field) rather than discarding the manager reference — so the base tearDown() picks it up and shuts it down. The executor and the HTTP client are now properly released after every test method. UriBuilder The logic: - indexOf('#') finds the fragment separator (first occurrence, per RFC 3986) - beforeFragment holds the scheme/authority/path/query portion — ? vs & detection runs against this - Query parameters are appended to beforeFragment - fragment (the #... part, or empty string) is re-appended last, keeping the URI well-formed JdkRDF4JHttpClient Repeatable bodies (isRepeatable() == true — ofBytes, ofString, ofFormData): reads into byte[] inside a try-with-resources, ensuring the InputStream is closed. ofByteArray preserves the content-length hint and redirect-retry safety. - Non-repeatable bodies (isRepeatable() == false — ofStream): passes the stream directly via BodyPublishers.ofInputStream(supplier). The JDK HttpClient owns and closes the stream after sending, so no buffering and no leak. The IOException from getContent() is wrapped in UncheckedIOException as required by the Supplier interface. Support for Producer<String> in BearerTokenAuthenticationHandler
1 parent f88fd61 commit 1585ffa

12 files changed

Lines changed: 214 additions & 30 deletions

File tree

core/http/client-apache5/src/main/java/org/eclipse/rdf4j/http/client/apache5/ApacheHC5HttpClientResponse.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@
2525
* <p>
2626
* Closing this response returns the connection to the pool.
2727
*/
28-
class ApacheHC5HttpClientResponse implements HttpResponse {
28+
public class ApacheHC5HttpClientResponse implements HttpResponse {
2929

3030
private final ClassicHttpResponse response;
3131
private final List<HttpHeader> headers;
3232

33-
ApacheHC5HttpClientResponse(ClassicHttpResponse response) {
33+
public ApacheHC5HttpClientResponse(ClassicHttpResponse response) {
3434
this.response = response;
3535
this.headers = Arrays.stream(response.getHeaders())
3636
.map(h -> HttpHeader.of(h.getName(), h.getValue()))

core/http/client-apache5/src/main/java/org/eclipse/rdf4j/http/client/apache5/ApacheHC5RDF4JHttpClient.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public class ApacheHC5RDF4JHttpClient implements RDF4JHttpClient {
4545
*/
4646
private final int maxRetries408;
4747

48-
ApacheHC5RDF4JHttpClient(CloseableHttpClient httpClient, int maxConnectionsPerRoute) {
48+
public ApacheHC5RDF4JHttpClient(CloseableHttpClient httpClient, int maxConnectionsPerRoute) {
4949
this.httpClient = httpClient;
5050
this.maxRetries408 = maxConnectionsPerRoute + 1;
5151
}
@@ -123,7 +123,7 @@ public void close() {
123123
try {
124124
httpClient.close();
125125
} catch (IOException e) {
126-
// ignore on close
126+
logger.trace("Error while closing http client: " + e.getMessage(), e);
127127
}
128128
}
129129
}

core/http/client-api/src/main/java/org/eclipse/rdf4j/http/client/spi/BearerTokenAuthenticationHandler.java

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,35 +16,65 @@
1616
* {@link AuthenticationHandler} that adds an HTTP Bearer token to every request.
1717
*
1818
* <p>
19-
* The {@code Authorization: Bearer <token>} header is set from the token supplied at construction time and appended to
20-
* every request passed to {@link #authenticate(HttpRequest)}.
19+
* The {@code Authorization: Bearer <token>} header is added to every request passed to
20+
* {@link #authenticate(HttpRequest)}. The token can be supplied either as a static string or as a
21+
* {@link Producer}{@code <String>} that is invoked on each request, allowing dynamic tokens such as short-lived OAuth
22+
* access tokens to be refreshed transparently.
2123
*
2224
* <p>
23-
* Example usage:
25+
* Example usage with a static token:
2426
*
2527
* <pre>
26-
* SPARQLProtocolSession session = ...;
2728
* session.setAuthenticationHandler(new BearerTokenAuthenticationHandler("my-token"));
2829
* </pre>
2930
*
31+
* <p>
32+
* Example usage with a dynamic token producer:
33+
*
34+
* <pre>
35+
* session.setAuthenticationHandler(new BearerTokenAuthenticationHandler(tokenStore::currentToken));
36+
* </pre>
37+
*
3038
* @see AuthenticationHandler
3139
*/
3240
public class BearerTokenAuthenticationHandler implements AuthenticationHandler {
3341

34-
private final String authorizationHeaderValue;
42+
private final Producer<String> tokenProducer;
3543

3644
/**
37-
* Creates a new {@link BearerTokenAuthenticationHandler} for the given token.
45+
* Creates a new {@link BearerTokenAuthenticationHandler} for the given static token.
3846
*
3947
* @param token the bearer token; must not be {@code null}
4048
*/
4149
public BearerTokenAuthenticationHandler(String token) {
4250
Objects.requireNonNull(token, "token must not be null");
43-
this.authorizationHeaderValue = "Bearer " + token;
51+
this.tokenProducer = () -> token;
52+
}
53+
54+
/**
55+
* Creates a new {@link BearerTokenAuthenticationHandler} that obtains the bearer token by invoking the given
56+
* {@link Producer} on each request.
57+
*
58+
* <p>
59+
* Use this constructor when the token may change over time (e.g. a short-lived OAuth access token that is refreshed
60+
* by the producer). The producer is called once per request; any checked exception it throws is wrapped in an
61+
* {@link IllegalStateException}.
62+
*
63+
* @param tokenProducer a producer that returns the current bearer token; must not be {@code null}
64+
*/
65+
public BearerTokenAuthenticationHandler(Producer<String> tokenProducer) {
66+
Objects.requireNonNull(tokenProducer, "tokenProducer must not be null");
67+
this.tokenProducer = tokenProducer;
4468
}
4569

4670
@Override
4771
public void authenticate(HttpRequest request) {
48-
request.addHeader("Authorization", authorizationHeaderValue);
72+
try {
73+
request.addHeader("Authorization", "Bearer " + tokenProducer.produce());
74+
} catch (RuntimeException e) {
75+
throw e;
76+
} catch (Exception e) {
77+
throw new IllegalStateException("Failed to produce bearer token", e);
78+
}
4979
}
5080
}

core/http/client-api/src/main/java/org/eclipse/rdf4j/http/client/spi/HttpRequest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
* @see RDF4JHttpClient
4141
* @see HttpRequestBody
4242
*/
43-
public final class HttpRequest {
43+
public class HttpRequest {
4444

4545
private final String method;
4646
private final URI uri;
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2026 Eclipse RDF4J contributors.
3+
*
4+
* All rights reserved. This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Distribution License v1.0
6+
* which accompanies this distribution, and is available at
7+
* http://www.eclipse.org/org/documents/edl-v10.php.
8+
*
9+
* SPDX-License-Identifier: BSD-3-Clause
10+
*******************************************************************************/
11+
package org.eclipse.rdf4j.http.client.spi;
12+
13+
/**
14+
* A supplier of values that may throw a checked exception.
15+
*
16+
* <p>
17+
* Unlike {@link java.util.function.Supplier}, this interface allows implementations to throw checked exceptions, making
18+
* it suitable for token sources that perform I/O, call external services, or otherwise fail in checked ways (e.g.
19+
* fetching an OAuth access token or reading a secret from a vault).
20+
*
21+
* @param <T> the type of value produced
22+
*/
23+
@FunctionalInterface
24+
public interface Producer<T> {
25+
26+
/**
27+
* Produces a value.
28+
*
29+
* @return the produced value
30+
* @throws Exception if production fails
31+
*/
32+
T produce() throws Exception;
33+
}

core/http/client-api/src/main/java/org/eclipse/rdf4j/http/client/spi/RDF4JHttpClientConfig.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
* @see RDF4JHttpClient
3737
* @see RDF4JHttpClientFactory
3838
*/
39-
public final class RDF4JHttpClientConfig {
39+
public class RDF4JHttpClientConfig {
4040

4141
private final int connectTimeoutMs;
4242
private final int socketTimeoutMs;
@@ -50,7 +50,7 @@ public final class RDF4JHttpClientConfig {
5050
private final boolean disableHostnameVerification;
5151
private final List<HttpHeader> defaultHeaders;
5252

53-
private RDF4JHttpClientConfig(Builder builder) {
53+
protected RDF4JHttpClientConfig(Builder builder) {
5454
this.connectTimeoutMs = builder.connectTimeoutMs;
5555
this.socketTimeoutMs = builder.socketTimeoutMs;
5656
this.connectionRequestTimeoutMs = builder.connectionRequestTimeoutMs;
@@ -208,7 +208,7 @@ public static Builder newBuilder() {
208208
* <li>{@code disableHostnameVerification} = {@code false}</li>
209209
* </ul>
210210
*/
211-
public static final class Builder {
211+
public static class Builder {
212212
private int connectTimeoutMs = 30_000;
213213
private int socketTimeoutMs = 0;
214214
private int connectionRequestTimeoutMs = 3_600_000;
@@ -221,7 +221,7 @@ public static final class Builder {
221221
private boolean disableHostnameVerification = false;
222222
private List<HttpHeader> defaultHeaders = List.of();
223223

224-
private Builder() {
224+
protected Builder() {
225225
}
226226

227227
/**

core/http/client-api/src/main/java/org/eclipse/rdf4j/http/client/spi/UriBuilder.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,13 @@ public URI build() {
7171
if (params.isEmpty()) {
7272
return URI.create(baseUrl);
7373
}
74-
StringBuilder sb = new StringBuilder(baseUrl);
75-
boolean first = !baseUrl.contains("?");
74+
// Split off any fragment so query parameters are inserted before it
75+
int fragmentIdx = baseUrl.indexOf('#');
76+
String beforeFragment = fragmentIdx >= 0 ? baseUrl.substring(0, fragmentIdx) : baseUrl;
77+
String fragment = fragmentIdx >= 0 ? baseUrl.substring(fragmentIdx) : "";
78+
79+
StringBuilder sb = new StringBuilder(beforeFragment);
80+
boolean first = !beforeFragment.contains("?");
7681
for (NameValuePair nvp : params) {
7782
if (nvp.getValue() == null) {
7883
continue;
@@ -83,6 +88,7 @@ public URI build() {
8388
sb.append('=');
8489
sb.append(URLEncoder.encode(nvp.getValue(), StandardCharsets.UTF_8));
8590
}
91+
sb.append(fragment);
8692
return URI.create(sb.toString());
8793
}
8894

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2026 Eclipse RDF4J contributors.
3+
*
4+
* All rights reserved. This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Distribution License v1.0
6+
* which accompanies this distribution, and is available at
7+
* http://www.eclipse.org/org/documents/edl-v10.php.
8+
*
9+
* SPDX-License-Identifier: BSD-3-Clause
10+
*******************************************************************************/
11+
package org.eclipse.rdf4j.http.client.spi;
12+
13+
import static org.assertj.core.api.Assertions.assertThat;
14+
15+
import java.net.URI;
16+
17+
import org.junit.jupiter.api.Test;
18+
19+
public class UriBuilderTest {
20+
21+
@Test
22+
public void testFragmentInBaseUrl_queryInsertedBeforeFragment() {
23+
URI uri = UriBuilder.from("http://example.com/path#section1")
24+
.addParameter("foo", "bar")
25+
.build();
26+
27+
assertThat(uri.toString()).isEqualTo("http://example.com/path?foo=bar#section1");
28+
assertThat(uri.getFragment()).isEqualTo("section1");
29+
assertThat(uri.getQuery()).isEqualTo("foo=bar");
30+
}
31+
32+
@Test
33+
public void testFragmentInBaseUrl_existingQueryAndFragment() {
34+
URI uri = UriBuilder.from("http://example.com/path?existing=1#section1")
35+
.addParameter("foo", "bar")
36+
.build();
37+
38+
assertThat(uri.toString()).isEqualTo("http://example.com/path?existing=1&foo=bar#section1");
39+
assertThat(uri.getFragment()).isEqualTo("section1");
40+
assertThat(uri.getQuery()).isEqualTo("existing=1&foo=bar");
41+
}
42+
43+
@Test
44+
public void testNoFragment_queryAppendedNormally() {
45+
URI uri = UriBuilder.from("http://example.com/path")
46+
.addParameter("foo", "bar")
47+
.build();
48+
49+
assertThat(uri.toString()).isEqualTo("http://example.com/path?foo=bar");
50+
assertThat(uri.getFragment()).isNull();
51+
}
52+
53+
@Test
54+
public void testFragmentInBaseUrl_noParams_returnsBaseUrlUnchanged() {
55+
URI uri = UriBuilder.from("http://example.com/path#section1").build();
56+
57+
assertThat(uri.toString()).isEqualTo("http://example.com/path#section1");
58+
}
59+
}

core/http/client-jdk/src/main/java/org/eclipse/rdf4j/http/client/jdk/JdkHttpClientResponse.java

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,55 @@
1515
import java.io.OutputStream;
1616
import java.net.http.HttpResponse;
1717
import java.util.List;
18+
import java.util.Map;
1819
import java.util.stream.Collectors;
1920

2021
import org.eclipse.rdf4j.http.client.spi.HttpHeader;
2122

2223
/**
2324
* {@link HttpResponse} backed by a JDK {@link HttpResponse}.
2425
*/
25-
class JdkHttpClientResponse implements org.eclipse.rdf4j.http.client.spi.HttpResponse {
26+
public class JdkHttpClientResponse implements org.eclipse.rdf4j.http.client.spi.HttpResponse {
27+
28+
// The JDK HttpClient does not expose reason phrases (HTTP/2 removed them).
29+
// This table covers the standard codes most relevant for error reporting.
30+
private static final Map<Integer, String> REASON_PHRASES = Map.ofEntries(
31+
Map.entry(100, "Continue"),
32+
Map.entry(101, "Switching Protocols"),
33+
Map.entry(200, "OK"),
34+
Map.entry(201, "Created"),
35+
Map.entry(204, "No Content"),
36+
Map.entry(206, "Partial Content"),
37+
Map.entry(301, "Moved Permanently"),
38+
Map.entry(302, "Found"),
39+
Map.entry(303, "See Other"),
40+
Map.entry(304, "Not Modified"),
41+
Map.entry(307, "Temporary Redirect"),
42+
Map.entry(308, "Permanent Redirect"),
43+
Map.entry(400, "Bad Request"),
44+
Map.entry(401, "Unauthorized"),
45+
Map.entry(403, "Forbidden"),
46+
Map.entry(404, "Not Found"),
47+
Map.entry(405, "Method Not Allowed"),
48+
Map.entry(406, "Not Acceptable"),
49+
Map.entry(408, "Request Timeout"),
50+
Map.entry(409, "Conflict"),
51+
Map.entry(410, "Gone"),
52+
Map.entry(413, "Content Too Large"),
53+
Map.entry(414, "URI Too Long"),
54+
Map.entry(415, "Unsupported Media Type"),
55+
Map.entry(429, "Too Many Requests"),
56+
Map.entry(500, "Internal Server Error"),
57+
Map.entry(501, "Not Implemented"),
58+
Map.entry(502, "Bad Gateway"),
59+
Map.entry(503, "Service Unavailable"),
60+
Map.entry(504, "Gateway Timeout")
61+
);
2662

2763
private final HttpResponse<InputStream> response;
2864
private final List<HttpHeader> headers;
2965

30-
JdkHttpClientResponse(HttpResponse<InputStream> response) {
66+
public JdkHttpClientResponse(HttpResponse<InputStream> response) {
3167
this.response = response;
3268
this.headers = response.headers()
3369
.map()
@@ -44,7 +80,7 @@ public int getStatusCode() {
4480

4581
@Override
4682
public String getReasonPhrase() {
47-
return "";
83+
return REASON_PHRASES.getOrDefault(response.statusCode(), "HTTP " + response.statusCode());
4884
}
4985

5086
@Override

core/http/client-jdk/src/main/java/org/eclipse/rdf4j/http/client/jdk/JdkRDF4JHttpClient.java

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
package org.eclipse.rdf4j.http.client.jdk;
1212

1313
import java.io.IOException;
14+
import java.io.InputStream;
15+
import java.io.UncheckedIOException;
1416
import java.net.http.HttpClient;
1517
import java.net.http.HttpRequest;
1618
import java.net.http.HttpRequest.BodyPublisher;
@@ -32,7 +34,7 @@ public class JdkRDF4JHttpClient implements RDF4JHttpClient {
3234
private final HttpClient httpClient;
3335
private final RDF4JHttpClientConfig config;
3436

35-
JdkRDF4JHttpClient(HttpClient httpClient, RDF4JHttpClientConfig config) {
37+
public JdkRDF4JHttpClient(HttpClient httpClient, RDF4JHttpClientConfig config) {
3638
this.httpClient = httpClient;
3739
this.config = config;
3840
}
@@ -60,10 +62,23 @@ public org.eclipse.rdf4j.http.client.spi.HttpResponse execute(org.eclipse.rdf4j.
6062
BodyPublisher bodyPublisher;
6163
if (request.getBody().isPresent()) {
6264
HttpRequestBody body = request.getBody().get();
63-
// Read to byte[] for re-readability on redirect
64-
byte[] bytes = body.getContent().readAllBytes();
6565
builder.header("Content-Type", body.getContentType());
66-
bodyPublisher = BodyPublishers.ofByteArray(bytes);
66+
if (body.isRepeatable()) {
67+
// Buffered body: read into byte[] (safe for redirect retries) and close the stream
68+
try (InputStream in = body.getContent()) {
69+
bodyPublisher = BodyPublishers.ofByteArray(in.readAllBytes());
70+
}
71+
} else {
72+
// Streaming body: pass directly to avoid buffering large payloads into memory;
73+
// the JDK HttpClient closes the stream after the request is sent
74+
bodyPublisher = BodyPublishers.ofInputStream(() -> {
75+
try {
76+
return body.getContent();
77+
} catch (IOException e) {
78+
throw new UncheckedIOException(e);
79+
}
80+
});
81+
}
6782
} else {
6883
bodyPublisher = BodyPublishers.noBody();
6984
}

0 commit comments

Comments
 (0)