Skip to content

Transports mishandle a request body set on a body-forbidden method (GET/HEAD) #117

@OmarAlJarrah

Description

@OmarAlJarrah

Summary

The SDK Request model makes body optional for every method and performs no method/body compatibility check at build time — RequestBuilder.build() only requires method and url, and passes body through unconditionally. A GET (or HEAD) request carrying a body is therefore publicly constructible:

Request.builder()
    .method(Method.GET)
    .url(URL("https://example.com/x"))
    .body(RequestBody.create("x".toByteArray()))
    .build()

The two transports handle this case inconsistently, and neither is correct.

OkHttp transport

RequestAdapter.adapt passes the adapted body to Request.Builder.method("GET", body), which throws IllegalArgumentException("method GET must not have a request body."). That exception is unchecked, so it escapes the transport's @Throws(IOException) handling and the retry pipeline.

This is the same failure class as the body-less POST/PUT/PATCH defect just fixed in #82, in the opposite direction: a body present on a method that forbids one, rather than a body absent on a method that requires one.

JDK transport

RequestAdapter.attachMethod builds the publisher via BodyPublishers.adaptBody(request.body) and then calls builder.GET(), which ignores it. The body is silently dropped — and for a small body the adaptBody eager path has already drained writeTo into a byte array that is then discarded, so a consume-once body is consumed for nothing.

Expected

Pick one policy and apply it consistently across both transports:

  1. Reject at construction — have RequestBuilder.build() fail fast with a clear, documented error when a body is set on a body-forbidden method (GET/HEAD); or
  2. Define a single documented transport behavior — e.g. both transports drop the body with a DEBUG log, or both raise the same exception type that respects the execute / executeAsync contract.

Either way: the OkHttp unchecked IllegalArgumentException should not escape execute's IOException contract, and the JDK side should not silently consume a body it never sends.

Code

  • sdk-transport-okhttp/.../internal/RequestAdapter.kt (adapt)
  • sdk-transport-jdkhttp/.../internal/RequestAdapter.kt (attachMethod)
  • sdk-core/.../http/request/Request.kt (RequestBuilder.build)

Surfaced alongside the body-handling work in #82, which covered the inverse (body-less POST/PUT/PATCH) case.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsdk-coresdk-core toolkit

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions