Skip to content

Commit 1b63a9a

Browse files
author
Mark Pollack
committed
Probe cleartext HTTP/2 with a bodiless GET instead of OPTIONS
The interop re-run showed the Python SDK's Hypercorn server upgrades a GET to h2c but answers OPTIONS with 405 on HTTP/1.1, so the probe pinned that pairing to HTTP/1.1 although the server speaks HTTP/2. The probe is now a GET without Acp-Connection-Id: every server answers it with a 4xx without opening a stream, and only the negotiated version is used. Servers without h2c still pin the client to HTTP/1.1.
1 parent e0dcb07 commit 1b63a9a

3 files changed

Lines changed: 10 additions & 6 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ building a second client on an already-connected transport now fails at construc
3636
- **HTTP/2 over plain `http://`.** The RFD requires HTTP/2, and localhost without TLS is a first-class
3737
deployment. Over cleartext the JDK client only offers the h2c upgrade on a request without a body, so
3838
`initialize`, a POST, went out on HTTP/1.1. `StreamableHttpAcpClientTransport` now sends a bodiless
39-
OPTIONS first on `http://` endpoints, so against a server that speaks h2c (the SDK's own does) every
39+
GET first on `http://` endpoints, so against a server that speaks h2c (the SDK's own does) every
4040
request, streams included, runs on HTTP/2. Against one that does not, the probe settles on HTTP/1.1
4141
and later requests stop offering the upgrade, which some servers route to their WebSocket handler.
4242
- **Client sessions learn that their transport died.** `AcpClientTransport.awaitTermination()` (default:

‎acp-core/src/main/java/com/agentclientprotocol/sdk/client/transport/StreamableHttpAcpClientTransport.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,16 +314,19 @@ public Mono<Void> sendMessage(JSONRPCMessage message) {
314314
* The transport requires HTTP/2 (RFD). Over {@code https} ALPN negotiates it. Over
315315
* cleartext {@code http} the JDK offers an h2c upgrade on every request, but servers
316316
* (Jetty among them) only honour it on a request without a body, and {@code initialize}
317-
* is a POST. A bodiless OPTIONS first upgrades the connection when the server speaks
317+
* is a POST. A bodiless GET first upgrades the connection when the server speaks
318318
* h2c; every later request reuses it over HTTP/2. When it does not (answer on HTTP/1.1,
319319
* an error, or no answer within five seconds), every later request is pinned to HTTP/1.1.
320+
* A GET rather than OPTIONS because some h2c servers (Hypercorn) upgrade a GET but answer
321+
* OPTIONS with 405 on HTTP/1.1. It carries no connection id, so servers answer it with a
322+
* 4xx without opening a stream; only the negotiated version is used.
320323
*/
321324
private Mono<Void> upgradeCleartextToHttp2() {
322325
if (!"http".equalsIgnoreCase(endpointUri.getScheme()) || httpClient.version() != HttpClient.Version.HTTP_2) {
323326
return Mono.empty();
324327
}
325328
HttpRequest probe = HttpRequest.newBuilder(endpointUri)
326-
.method("OPTIONS", HttpRequest.BodyPublishers.noBody())
329+
.GET()
327330
.build();
328331
// Cancelling the Mono on timeout cancels the HTTP exchange (see sendAsync).
329332
return sendAsync(probe, HttpResponse.BodyHandlers.discarding())

‎acp-core/src/test/java/com/agentclientprotocol/sdk/client/transport/StreamableHttpAcpClientTransportTest.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -805,8 +805,8 @@ void cleartextServerWithoutH2cGetsPlainHttp11Requests() throws Exception {
805805
when(httpClient.sendAsync(any(), any())).thenAnswer(invocation -> {
806806
HttpRequest request = invocation.getArgument(0);
807807
requests.add(request);
808-
if ("OPTIONS".equals(request.method())) {
809-
HttpResponse<Object> probe = response(405, Map.of(), null);
808+
if ("GET".equals(request.method()) && request.headers().firstValue("Acp-Connection-Id").isEmpty()) {
809+
HttpResponse<Object> probe = response(400, Map.of(), null);
810810
when(probe.version()).thenReturn(HttpClient.Version.HTTP_1_1);
811811
return CompletableFuture.completedFuture(probe);
812812
}
@@ -828,7 +828,8 @@ void cleartextServerWithoutH2cGetsPlainHttp11Requests() throws Exception {
828828
transport.sendMessage(AcpTestFixtures.createJsonRpcRequest(AcpSchema.METHOD_INITIALIZE, "init-1",
829829
AcpTestFixtures.createInitializeRequest())).block();
830830

831-
assertThat(requests.get(0).method()).isEqualTo("OPTIONS");
831+
assertThat(requests.get(0).method()).as("the probe").isEqualTo("GET");
832+
assertThat(requests.get(0).headers().firstValue("Acp-Connection-Id")).isEmpty();
832833
assertThat(requests.subList(1, requests.size()))
833834
.as("every request after the probe is pinned to HTTP/1.1")
834835
.isNotEmpty()

0 commit comments

Comments
 (0)