Skip to content

Commit

Permalink
Add missing "Content-Length: 0" header with HttpComponents
Browse files Browse the repository at this point in the history
Prior to this commit, HTTP requests sent with the
`HttpComponentsClientHttpRequestFactory` would not set a
"Content-Length" header for empty request bodies. Setting a request
entity is the expected behavior for unsafe HTTP methods, and this would
align the behavior with other HTTP clients.
Developers would often rely on `BufferingClientHttpRequestFactory` to
set this information on the request.

This commit ensures that a `NullEntity` is used for unsafe HTTP methods,
when no body has been set for the request. This result in a
"Content-Length:0" request header.

Fixes gh-32678
  • Loading branch information
bclozel committed May 2, 2024
1 parent 5a24e94 commit 47c5cd2
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.Method;
import org.apache.hc.core5.http.io.entity.NullEntity;
import org.apache.hc.core5.http.protocol.HttpContext;

import org.springframework.http.HttpHeaders;
Expand Down Expand Up @@ -89,8 +91,10 @@ protected ClientHttpResponse executeInternal(HttpHeaders headers, @Nullable Body
addHeaders(this.httpRequest, headers);

if (body != null) {
HttpEntity requestEntity = new BodyEntity(headers, body);
this.httpRequest.setEntity(requestEntity);
this.httpRequest.setEntity(new BodyEntity(headers, body));
}
else if (!Method.isSafe(this.httpRequest.getMethod())) {
this.httpRequest.setEntity(NullEntity.INSTANCE);
}
ClassicHttpResponse httpResponse = this.httpClient.executeOpen(null, this.httpRequest, this.httpContext);
return new HttpComponentsClientHttpResponse(httpResponse);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ else if(request.getPath().startsWith("/methods/")) {
assertThat(request.getMethod()).isEqualTo(expectedMethod);
return new MockResponse();
}
else if(request.getPath().startsWith("/header/")) {
String headerName = request.getPath().replace("/header/","");
return new MockResponse().setBody(headerName + ":" + request.getHeader(headerName)).setResponseCode(200);
}
return new MockResponse().setResponseCode(404);
}
catch (Throwable exc) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,7 +16,9 @@

package org.springframework.http.client;

import java.io.InputStreamReader;
import java.net.URI;
import java.util.stream.Stream;

import org.apache.hc.client5.http.classic.HttpClient;
import org.apache.hc.client5.http.config.Configurable;
Expand All @@ -26,8 +28,12 @@
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.util.Timeout;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.util.FileCopyUtils;

import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -37,6 +43,7 @@

/**
* @author Stephane Nicoll
* @author Brian Clozel
*/
class HttpComponentsClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests {

Expand Down Expand Up @@ -145,6 +152,36 @@ public HttpClient getHttpClient() {
assertThat(requestConfig2.getConnectionRequestTimeout()).isEqualTo(Timeout.of(7000, MILLISECONDS));
}

@ParameterizedTest
@MethodSource("unsafeHttpMethods")
void shouldSetContentLengthWhenEmptyBody(HttpMethod method) throws Exception {
ClientHttpRequest request = factory.createRequest(URI.create(baseUrl + "/header/Content-Length"), method);
try (ClientHttpResponse response = request.execute()) {
assertThat(response.getStatusCode()).as("Invalid status code").isEqualTo(HttpStatus.OK);
String result = FileCopyUtils.copyToString(new InputStreamReader(response.getBody()));
assertThat(result).as("Invalid body").isEqualTo("Content-Length:0");
}
}

static Stream<HttpMethod> unsafeHttpMethods() {
return Stream.of(HttpMethod.POST, HttpMethod.PUT, HttpMethod.DELETE, HttpMethod.PATCH);
}

@ParameterizedTest
@MethodSource("safeHttpMethods")
void shouldNotSetContentLengthWhenEmptyBodyAndSafeMethod(HttpMethod method) throws Exception {
ClientHttpRequest request = factory.createRequest(URI.create(baseUrl + "/header/Content-Length"), method);
try (ClientHttpResponse response = request.execute()) {
assertThat(response.getStatusCode()).as("Invalid status code").isEqualTo(HttpStatus.OK);
String result = FileCopyUtils.copyToString(new InputStreamReader(response.getBody()));
assertThat(result).as("Invalid body").isEqualTo("Content-Length:null");
}
}

static Stream<HttpMethod> safeHttpMethods() {
return Stream.of(HttpMethod.GET, HttpMethod.OPTIONS, HttpMethod.TRACE);
}

private RequestConfig retrieveRequestConfig(HttpComponentsClientHttpRequestFactory factory) throws Exception {
URI uri = URI.create(baseUrl + "/status/ok");
HttpComponentsClientHttpRequest request = (HttpComponentsClientHttpRequest)
Expand Down

0 comments on commit 47c5cd2

Please sign in to comment.