"GET must not have a request body" exception with RestTemplate.exchange(...) and OkHttp3ClientHttpRequest.executeInternal()

[kamabulletone]

Out team recently upgraded out project from spring boot 3.1.9 to 3.2.3 (spring-framework from 6.0.17 to 6.1.6). After this upgrade we start getting "GET must not have a request body" on GET calls. We havent changed any configuration but versions of libs. We have the following RestTemplate configuration now:

    @Bean
    fun restTemplateBuilder(
        restTemplateBuilderConfigurer: RestTemplateBuilderConfigurer,
        meterRegistry: MeterRegistry,
        logbook: Logbook,
    ): RestTemplateBuilder {
        val requestFactorySupplier = {
            BufferingClientHttpRequestFactory(OkHttp3ClientHttpRequestFactory(OkHttpClient.Builder().build()))
        }
        val interceptors = listOf(
            LogbookClientHttpRequestInterceptor(logbook),
            ClientRequestMetricsInterceptor(meterRegistry)
        )
        val builder = RestTemplateBuilder()
            .requestFactory(requestFactorySupplier)
            .interceptors(interceptors)

        return restTemplateBuilderConfigurer.configure(builder)
    }

I've erased some exception related messages. Code of out rest-service:

    fun <S> get(
        path: String,
        responseClass: Class<S>,
        uriVariables: Map<String, *> = emptyMap<String, Any>(),
        queryParams: MultiValueMap<String, String> = LinkedMultiValueMap(),
    ): S {
        val requestMetaData = RequestMetaData(HttpMethod.GET, path, uriVariables, queryParams)
        return exchange(requestMetaData, body = null, responseClass = responseClass)
    }

    fun <Q, S> post(
        body: Q,
        path: String,
        responseClass: Class<S>,
        uriVariables: Map<String, *> = emptyMap<String, Any>(),
        queryParams: MultiValueMap<String, String> = LinkedMultiValueMap()
    ): S {
        val requestMetaData = RequestMetaData(HttpMethod.POST, path, uriVariables, queryParams)
        return exchange(requestMetaData, body, responseClass)
    }

    private fun <Q, S> exchange(
        requestMetaData: RequestMetaData,
        body: Q,
        responseClass: Class<S>
    ): S {
        return try {
            val uriString = UriComponentsBuilder
                .fromHttpUrl(config.host)
                .uriVariables(requestMetaData.uriVariables)
                .queryParams(requestMetaData.queryParams)
                .path(requestMetaData.path)
                .toUriString()
            val bodyEntity = HttpEntity(body, withMetricHeaders(requestMetaData.path, config.host, ExternalSystems.MOCK_SYSTEM))
            rest
                .exchange(
                    uriString,
                    requestMetaData.method,
                    bodyEntity,
                    responseClass
                ).body ?: throw CallFailedException("ERROR")
        } catch (e: HttpClientErrorException) {
            if (e.statusCode.is4xxClientError) {
                throw nvalidRequestException("ERROR", e)
            } else {
                throw CallFailedException("ERROR", e)
            }
        } catch (e: RestClientException) {
            throw EcpFnsCallFailedException("ERROR", e)
        }
    }

Using spring-boot 3.1.9 (spring-framework 6.0.17) worked just fine. But after moving to spring-boot 3.2.3 (spring-framework 6.1.6) we got following errors with stacktrace:

java.lang.IllegalArgumentException: method GET must not have a request body.
	at okhttp3.Request$Builder.method(Request.kt:258)
	at org.springframework.http.client.OkHttp3ClientHttpRequest.executeInternal(OkHttp3ClientHttpRequest.java:88)
	at org.springframework.http.client.AbstractStreamingClientHttpRequest.executeInternal(AbstractStreamingClientHttpRequest.java:70)
	at org.springframework.http.client.AbstractClientHttpRequest.execute(AbstractClientHttpRequest.java:66)
	at org.springframework.http.client.BufferingClientHttpRequestWrapper.executeInternal(BufferingClientHttpRequestWrapper.java:75)
	at org.springframework.http.client.AbstractBufferingClientHttpRequest.executeInternal(AbstractBufferingClientHttpRequest.java:48)
	at org.springframework.http.client.AbstractClientHttpRequest.execute(AbstractClientHttpRequest.java:66)
	at org.springframework.http.client.InterceptingClientHttpRequest$InterceptingRequestExecution.execute(InterceptingClientHttpRequest.java:112)
	at ru.tinkoff.sme.qds.backend.restclient.config.AbstractAuthorizationTokenInterceptor.intercept(AbstractAuthorizationTokenInterceptor.kt:133)
	at org.springframework.http.client.InterceptingClientHttpRequest$InterceptingRequestExecution.execute(InterceptingClientHttpRequest.java:88)
	at ru.tinkoff.sme.qds.backend.restclient.config.ClientRequestMetricsInterceptor.intercept(ClientRequestMetricsInterceptor.kt:26)
	at org.springframework.http.client.InterceptingClientHttpRequest$InterceptingRequestExecution.execute(InterceptingClientHttpRequest.java:88)
	at org.zalando.logbook.spring.LogbookClientHttpRequestInterceptor.intercept(LogbookClientHttpRequestInterceptor.java:25)
	at org.springframework.http.client.InterceptingClientHttpRequest$InterceptingRequestExecution.execute(InterceptingClientHttpRequest.java:88)
	at org.springframework.http.client.InterceptingClientHttpRequest.executeInternal(InterceptingClientHttpRequest.java:72)
	at org.springframework.http.client.AbstractBufferingClientHttpRequest.executeInternal(AbstractBufferingClientHttpRequest.java:48)
	at org.springframework.http.client.AbstractClientHttpRequest.execute(AbstractClientHttpRequest.java:66)
	at org.springframework.web.client.RestTemplate.doExecute(RestTemplate.java:889)
	at org.springframework.web.client.RestTemplate.execute(RestTemplate.java:790)
	at org.springframework.web.client.RestTemplate.exchange(RestTemplate.java:672)
    ...

I've looked up changes made between these 2 spring boot and spring framework version and in spring-framework (spring-web) there was an issue that affected OkHttp3ClientHttpRequest.java

Before this issue spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequest.java had executeInternal(HttpHeaders headers, byte[] content) signature and implemented like this:

	protected ClientHttpResponse executeInternal(HttpHeaders headers, byte[] content) throws IOException {
		Request request = OkHttp3ClientHttpRequestFactory.buildRequest(headers, content, this.uri, this.method);
		return new OkHttp3ClientHttpResponse(this.client.newCall(request).execute());
	}

And OkHttp3ClientHttpRequestFactory.buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method) created request body when and only when content.length > 0 or http method requires request body:

	static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method)
			throws MalformedURLException {

		okhttp3.MediaType contentType = getContentType(headers);
		RequestBody body = (content.length > 0 ||
				okhttp3.internal.http.HttpMethod.requiresRequestBody(method.name()) ?
				RequestBody.create(contentType, content) : null);

		Request.Builder builder = new Request.Builder().url(uri.toURL()).method(method.name(), body);
		headers.forEach((headerName, headerValues) -> {
			for (String headerValue : headerValues) {
				builder.addHeader(headerName, headerValue);
			}
		});
		return builder.build();
	}

But after resolving of that issue implementation changed and new request body wrapper was implemented. THe OkHttp3ClientHttpRequest.executeInternal(...) method signature changed to ClientHttpResponse executeInternal(HttpHeaders headers, @Nullable Body body):

	protected ClientHttpResponse executeInternal(HttpHeaders headers, @Nullable Body body) throws IOException {

		RequestBody requestBody;
		if (body != null) {
			requestBody = new BodyRequestBody(headers, body);
		}
		else if (okhttp3.internal.http.HttpMethod.requiresRequestBody(getMethod().name())) {
			String header = headers.getFirst(HttpHeaders.CONTENT_TYPE);
			MediaType contentType = (header != null) ? MediaType.parse(header) : null;
			requestBody = RequestBody.create(contentType, new byte[0]);
		}
		else {
			requestBody = null;
		}
		Request.Builder builder = new Request.Builder()
				.url(this.uri.toURL());
		builder.method(this.method.name(), requestBody);
		headers.forEach((headerName, headerValues) -> {
			for (String headerValue : headerValues) {
				builder.addHeader(headerName, headerValue);
			}
		});
		Request request = builder.build();
		return new OkHttp3ClientHttpResponse(this.client.newCall(request).execute());

This implementation doesnt check Body.bufferedOutput if it's empty so creates request body event if there isn't any content bytes. I've run debugging to this point during GET request and this is the result:

As you can see bufferedOutput is empty but request body created. I assume just add extra condition after checking if body is null like this:

...
		if (body != null and body.bufferedOutput > 0) {
			requestBody = new BodyRequestBody(headers, body);
		}
...

[bclozel]

This sounds similar to #32612 but this should have been resolved in 6.1.6. Rather than discussing the possible reasons, could you provide us with a minimal sample application that reproduces the problem?

[kamabulletone]

This sounds similar to #32612 but this should have been resolved in 6.1.6. Rather than discussing the possible reasons, could you provide us with a minimal sample application that reproduces the problem?

Ok, will do a demo app repo and link it. Is that ok?

[bclozel]

Yes please!

[kamabulletone]

I've put together a complete minimal demo project that reproduces the problem: https://github.com/kamabulletone/okhttp-issue-demo

Also I've created an endpoint GET http://localhost:8082/tryrest to test the problem

[bclozel]

Your sample application is using Spring Boot 3.2.3, which is using Spring Framework 6.1.4. Upgrading your sample application to Spring Boot 3.2.5 (and thus Spring Framework 6.1.6) works for me. I'm tempted to close this as a duplicate of #32612. Let me know if I'm missing something.

[kamabulletone]

Turns out we configured out service not correctly. Thank you for cooperation and investing your time!

[bclozel]

Thanks for letting us know.