Skip to content

Commit ec8391a

Browse files
committed
Fix Netty4ClientHttpRequestFactory POST/PUT requests
This commit ensures that POST/PUT requests sent by the Netty client have a Content-Length header set. Integration tests have been refactored to use mockwebserver instead of Jetty and have been parameterized to run on all available supported clients. Issue: SPR-14860
1 parent 2c2de82 commit ec8391a

9 files changed

+407
-603
lines changed

spring-web/src/main/java/org/springframework/http/client/Netty4ClientHttpRequest.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
*
4949
* @author Arjen Poutsma
5050
* @author Rossen Stoyanchev
51+
* @author Brian Clozel
5152
* @since 4.1.2
5253
*/
5354
class Netty4ClientHttpRequest extends AbstractAsyncClientHttpRequest implements ClientHttpRequest {
@@ -130,10 +131,15 @@ private FullHttpRequest createFullHttpRequest(HttpHeaders headers) {
130131
io.netty.handler.codec.http.HttpMethod nettyMethod =
131132
io.netty.handler.codec.http.HttpMethod.valueOf(this.method.name());
132133

134+
String authority = this.uri.getRawAuthority();
135+
String path = this.uri.toString().substring(this.uri.toString().indexOf(authority) + authority.length());
133136
FullHttpRequest nettyRequest = new DefaultFullHttpRequest(
134-
HttpVersion.HTTP_1_1, nettyMethod, this.uri.toString(), this.body.buffer());
137+
HttpVersion.HTTP_1_1, nettyMethod, path, this.body.buffer());
135138

136139
nettyRequest.headers().set(HttpHeaders.HOST, this.uri.getHost());
140+
if (this.body.buffer().readableBytes() > 0) {
141+
nettyRequest.headers().set(HttpHeaders.CONTENT_LENGTH, this.body.buffer().readableBytes());
142+
}
137143
nettyRequest.headers().set(HttpHeaders.CONNECTION, "close");
138144

139145
for (Map.Entry<String, List<String>> entry : headers.entrySet()) {

spring-web/src/test/java/org/springframework/http/client/AbstractAsyncHttpRequestFactoryTestCase.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
import org.springframework.util.concurrent.ListenableFuture;
4040
import org.springframework.util.concurrent.ListenableFutureCallback;
4141

42-
public abstract class AbstractAsyncHttpRequestFactoryTestCase extends AbstractJettyServerTestCase {
42+
public abstract class AbstractAsyncHttpRequestFactoryTestCase extends AbstractMockWebServerTestCase {
4343

4444
protected AsyncClientHttpRequestFactory factory;
4545

spring-web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTestCase.java

+4-7
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
/**
4040
* @author Arjen Poutsma
4141
*/
42-
public abstract class AbstractHttpRequestFactoryTestCase extends AbstractJettyServerTestCase {
42+
public abstract class AbstractHttpRequestFactoryTestCase extends AbstractMockWebServerTestCase {
4343

4444
protected ClientHttpRequestFactory factory;
4545

@@ -127,6 +127,8 @@ public void multipleWrites() throws Exception {
127127
@Override
128128
public void writeTo(OutputStream outputStream) throws IOException {
129129
StreamUtils.copy(body, outputStream);
130+
outputStream.flush();
131+
outputStream.close();
130132
}
131133
});
132134
}
@@ -135,12 +137,7 @@ public void writeTo(OutputStream outputStream) throws IOException {
135137
}
136138

137139
ClientHttpResponse response = request.execute();
138-
try {
139-
FileCopyUtils.copy(body, request.getBody());
140-
}
141-
finally {
142-
response.close();
143-
}
140+
FileCopyUtils.copy(body, request.getBody());
144141
}
145142

146143
@Test(expected = UnsupportedOperationException.class)

spring-web/src/test/java/org/springframework/http/client/AbstractJettyServerTestCase.java

-202
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package org.springframework.http.client;
2+
3+
import java.util.Collections;
4+
5+
import okhttp3.mockwebserver.Dispatcher;
6+
import okhttp3.mockwebserver.MockResponse;
7+
import okhttp3.mockwebserver.MockWebServer;
8+
import okhttp3.mockwebserver.RecordedRequest;
9+
import org.hamcrest.Matchers;
10+
import org.junit.After;
11+
import org.junit.Before;
12+
13+
import org.springframework.http.MediaType;
14+
import org.springframework.util.StringUtils;
15+
16+
import static org.hamcrest.MatcherAssert.assertThat;
17+
18+
/**
19+
* @author Brian Clozel
20+
*/
21+
public class AbstractMockWebServerTestCase {
22+
23+
private MockWebServer server;
24+
25+
protected int port;
26+
27+
protected String baseUrl;
28+
29+
protected static final MediaType textContentType =
30+
new MediaType("text", "plain", Collections.singletonMap("charset", "UTF-8"));
31+
32+
@Before
33+
public void setUp() throws Exception {
34+
this.server = new MockWebServer();
35+
this.server.setDispatcher(new TestDispatcher());
36+
this.server.start();
37+
this.port = this.server.getPort();
38+
this.baseUrl = "http://localhost:" + this.port;
39+
}
40+
41+
@After
42+
public void tearDown() throws Exception {
43+
this.server.shutdown();
44+
}
45+
46+
protected class TestDispatcher extends Dispatcher {
47+
@Override
48+
public MockResponse dispatch(RecordedRequest request) throws InterruptedException {
49+
try {
50+
if (request.getPath().equals("/echo")) {
51+
MockResponse response = new MockResponse()
52+
.setHeaders(request.getHeaders())
53+
.setHeader("Content-Length", request.getBody().size())
54+
.setResponseCode(200)
55+
.setBody(request.getBody());
56+
request.getBody().flush();
57+
return response;
58+
}
59+
else if(request.getPath().equals("/status/ok")) {
60+
return new MockResponse();
61+
}
62+
else if(request.getPath().equals("/status/notfound")) {
63+
return new MockResponse().setResponseCode(404);
64+
}
65+
else if(request.getPath().startsWith("/params")) {
66+
assertThat(request.getPath(), Matchers.containsString("param1=value"));
67+
assertThat(request.getPath(), Matchers.containsString("param2=value1&param2=value2"));
68+
return new MockResponse();
69+
}
70+
else if(request.getPath().equals("/methods/post")) {
71+
assertThat(request.getMethod(), Matchers.is("POST"));
72+
String transferEncoding = request.getHeader("Transfer-Encoding");
73+
if(StringUtils.hasLength(transferEncoding)) {
74+
assertThat(transferEncoding, Matchers.is("chunked"));
75+
}
76+
else {
77+
long contentLength = Long.parseLong(request.getHeader("Content-Length"));
78+
assertThat("Invalid content-length",
79+
request.getBody().size(), Matchers.is(contentLength));
80+
}
81+
return new MockResponse().setResponseCode(200);
82+
}
83+
else if(request.getPath().startsWith("/methods/")) {
84+
String expectedMethod = request.getPath().replace("/methods/","").toUpperCase();
85+
assertThat(request.getMethod(), Matchers.is(expectedMethod));
86+
return new MockResponse();
87+
}
88+
return new MockResponse().setResponseCode(404);
89+
}
90+
catch (Throwable exc) {
91+
return new MockResponse().setResponseCode(500).setBody(exc.toString());
92+
}
93+
}
94+
}
95+
}

0 commit comments

Comments
 (0)