Skip to content

Commit 67ea430

Browse files
nosannosan
committed
Update RestTemplateBuilder and TestRestTemplate to use a custom request factory to add
authentication headers. Prior to this commit, the `RestTemplateBuilder` and `TestRestTemplate` used the `BasicAuthenticationInterceptor` interceptor to add headers. Unfortunately, adding any interceptor causes the entire message body to be read into a byte array. This causes an `OutOfMemoryError` whenever a large file is uploaded. Closes gh-15078
1 parent 23a923d commit 67ea430

File tree

7 files changed

+331
-76
lines changed

7 files changed

+331
-76
lines changed

‎spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/web/client/TestRestTemplate.java

+25-22
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,8 @@
1919
import java.io.IOException;
2020
import java.lang.reflect.Field;
2121
import java.net.URI;
22-
import java.util.ArrayList;
2322
import java.util.Arrays;
24-
import java.util.Collections;
2523
import java.util.HashSet;
26-
import java.util.List;
2724
import java.util.Map;
2825
import java.util.Set;
2926
import java.util.function.Supplier;
@@ -41,6 +38,8 @@
4138

4239
import org.springframework.beans.BeanInstantiationException;
4340
import org.springframework.beans.BeanUtils;
41+
import org.springframework.boot.web.client.BasicAuthentication;
42+
import org.springframework.boot.web.client.BasicAuthenticationClientHttpRequestFactory;
4443
import org.springframework.boot.web.client.ClientHttpRequestFactorySupplier;
4544
import org.springframework.boot.web.client.RestTemplateBuilder;
4645
import org.springframework.boot.web.client.RootUriTemplateHandler;
@@ -50,12 +49,11 @@
5049
import org.springframework.http.HttpMethod;
5150
import org.springframework.http.RequestEntity;
5251
import org.springframework.http.ResponseEntity;
52+
import org.springframework.http.client.AbstractClientHttpRequestFactoryWrapper;
5353
import org.springframework.http.client.ClientHttpRequestFactory;
54-
import org.springframework.http.client.ClientHttpRequestInterceptor;
5554
import org.springframework.http.client.ClientHttpResponse;
5655
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
5756
import org.springframework.http.client.InterceptingClientHttpRequestFactory;
58-
import org.springframework.http.client.support.BasicAuthenticationInterceptor;
5957
import org.springframework.util.Assert;
6058
import org.springframework.util.ReflectionUtils;
6159
import org.springframework.web.client.DefaultResponseErrorHandler;
@@ -86,6 +84,7 @@
8684
* @author Phillip Webb
8785
* @author Andy Wilkinson
8886
* @author Kristine Jetzke
87+
* @author Dmytro Nosan
8988
* @since 1.4.0
9089
*/
9190
public class TestRestTemplate {
@@ -154,31 +153,35 @@ private TestRestTemplate(RestTemplate restTemplate, String username, String pass
154153

155154
private Class<? extends ClientHttpRequestFactory> getRequestFactoryClass(
156155
RestTemplate restTemplate) {
156+
return getRequestFactory(restTemplate).getClass();
157+
}
158+
159+
private ClientHttpRequestFactory getRequestFactory(RestTemplate restTemplate) {
157160
ClientHttpRequestFactory requestFactory = restTemplate.getRequestFactory();
158-
if (InterceptingClientHttpRequestFactory.class
159-
.isAssignableFrom(requestFactory.getClass())) {
160-
Field requestFactoryField = ReflectionUtils.findField(RestTemplate.class,
161-
"requestFactory");
162-
ReflectionUtils.makeAccessible(requestFactoryField);
163-
requestFactory = (ClientHttpRequestFactory) ReflectionUtils
164-
.getField(requestFactoryField, restTemplate);
161+
while (requestFactory instanceof InterceptingClientHttpRequestFactory
162+
|| requestFactory instanceof BasicAuthenticationClientHttpRequestFactory) {
163+
requestFactory = unwrapRequestFactory(
164+
((AbstractClientHttpRequestFactoryWrapper) requestFactory));
165165
}
166-
return requestFactory.getClass();
166+
return requestFactory;
167+
}
168+
169+
private ClientHttpRequestFactory unwrapRequestFactory(
170+
AbstractClientHttpRequestFactoryWrapper requestFactory) {
171+
Field field = ReflectionUtils.findField(
172+
AbstractClientHttpRequestFactoryWrapper.class, "requestFactory");
173+
ReflectionUtils.makeAccessible(field);
174+
return (ClientHttpRequestFactory) ReflectionUtils.getField(field, requestFactory);
167175
}
168176

169177
private void addAuthentication(RestTemplate restTemplate, String username,
170178
String password) {
171-
if (username == null) {
179+
if (username == null || password == null) {
172180
return;
173181
}
174-
List<ClientHttpRequestInterceptor> interceptors = restTemplate.getInterceptors();
175-
if (interceptors == null) {
176-
interceptors = Collections.emptyList();
177-
}
178-
interceptors = new ArrayList<>(interceptors);
179-
interceptors.removeIf(BasicAuthenticationInterceptor.class::isInstance);
180-
interceptors.add(new BasicAuthenticationInterceptor(username, password));
181-
restTemplate.setInterceptors(interceptors);
182+
ClientHttpRequestFactory requestFactory = getRequestFactory(restTemplate);
183+
restTemplate.setRequestFactory(new BasicAuthenticationClientHttpRequestFactory(
184+
new BasicAuthentication(username, password), requestFactory));
182185
}
183186

184187
/**

‎spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/web/client/TestRestTemplateTests.java

+18-25
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,13 @@
2020
import java.lang.reflect.Method;
2121
import java.lang.reflect.Modifier;
2222
import java.net.URI;
23-
import java.util.List;
2423

2524
import org.apache.http.client.config.RequestConfig;
2625
import org.junit.jupiter.api.Test;
2726

2827
import org.springframework.boot.test.web.client.TestRestTemplate.CustomHttpComponentsClientHttpRequestFactory;
2928
import org.springframework.boot.test.web.client.TestRestTemplate.HttpClientOption;
29+
import org.springframework.boot.web.client.BasicAuthenticationClientHttpRequestFactory;
3030
import org.springframework.boot.web.client.RestTemplateBuilder;
3131
import org.springframework.core.ParameterizedTypeReference;
3232
import org.springframework.http.HttpEntity;
@@ -35,12 +35,9 @@
3535
import org.springframework.http.RequestEntity;
3636
import org.springframework.http.client.ClientHttpRequest;
3737
import org.springframework.http.client.ClientHttpRequestFactory;
38-
import org.springframework.http.client.ClientHttpRequestInterceptor;
3938
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
40-
import org.springframework.http.client.InterceptingClientHttpRequestFactory;
4139
import org.springframework.http.client.OkHttp3ClientHttpRequestFactory;
4240
import org.springframework.http.client.SimpleClientHttpRequestFactory;
43-
import org.springframework.http.client.support.BasicAuthenticationInterceptor;
4441
import org.springframework.mock.env.MockEnvironment;
4542
import org.springframework.mock.http.client.MockClientHttpRequest;
4643
import org.springframework.mock.http.client.MockClientHttpResponse;
@@ -150,7 +147,7 @@ public void getRootUriRootUriNotSet() {
150147
public void authenticated() {
151148
assertThat(new TestRestTemplate("user", "password").getRestTemplate()
152149
.getRequestFactory())
153-
.isInstanceOf(InterceptingClientHttpRequestFactory.class);
150+
.isInstanceOf(BasicAuthenticationClientHttpRequestFactory.class);
154151
}
155152

156153
@Test
@@ -227,43 +224,42 @@ private Object mockArgument(Class<?> type) throws Exception {
227224
}
228225

229226
@Test
230-
public void withBasicAuthAddsBasicAuthInterceptorWhenNotAlreadyPresent() {
227+
public void withBasicAuthAddsBasicAuthClientFactoryWhenNotAlreadyPresent() {
231228
TestRestTemplate originalTemplate = new TestRestTemplate();
232229
TestRestTemplate basicAuthTemplate = originalTemplate.withBasicAuth("user",
233230
"password");
234231
assertThat(basicAuthTemplate.getRestTemplate().getMessageConverters())
235232
.containsExactlyElementsOf(
236233
originalTemplate.getRestTemplate().getMessageConverters());
237234
assertThat(basicAuthTemplate.getRestTemplate().getRequestFactory())
238-
.isInstanceOf(InterceptingClientHttpRequestFactory.class);
235+
.isInstanceOf(BasicAuthenticationClientHttpRequestFactory.class);
239236
assertThat(ReflectionTestUtils.getField(
240237
basicAuthTemplate.getRestTemplate().getRequestFactory(),
241238
"requestFactory"))
242239
.isInstanceOf(CustomHttpComponentsClientHttpRequestFactory.class);
243240
assertThat(basicAuthTemplate.getRestTemplate().getUriTemplateHandler())
244241
.isSameAs(originalTemplate.getRestTemplate().getUriTemplateHandler());
245-
assertThat(basicAuthTemplate.getRestTemplate().getInterceptors()).hasSize(1);
246-
assertBasicAuthorizationInterceptorCredentials(basicAuthTemplate, "user",
247-
"password");
242+
assertThat(basicAuthTemplate.getRestTemplate().getInterceptors()).isEmpty();
243+
assertBasicAuthorizationCredentials(basicAuthTemplate, "user", "password");
248244
}
249245

250246
@Test
251-
public void withBasicAuthReplacesBasicAuthInterceptorWhenAlreadyPresent() {
247+
public void withBasicAuthReplacesBasicAuthClientFactoryWhenAlreadyPresent() {
252248
TestRestTemplate original = new TestRestTemplate("foo", "bar")
253249
.withBasicAuth("replace", "replace");
254250
TestRestTemplate basicAuth = original.withBasicAuth("user", "password");
255251
assertThat(basicAuth.getRestTemplate().getMessageConverters())
256252
.containsExactlyElementsOf(
257253
original.getRestTemplate().getMessageConverters());
258254
assertThat(basicAuth.getRestTemplate().getRequestFactory())
259-
.isInstanceOf(InterceptingClientHttpRequestFactory.class);
255+
.isInstanceOf(BasicAuthenticationClientHttpRequestFactory.class);
260256
assertThat(ReflectionTestUtils.getField(
261257
basicAuth.getRestTemplate().getRequestFactory(), "requestFactory"))
262258
.isInstanceOf(CustomHttpComponentsClientHttpRequestFactory.class);
263259
assertThat(basicAuth.getRestTemplate().getUriTemplateHandler())
264260
.isSameAs(original.getRestTemplate().getUriTemplateHandler());
265-
assertThat(basicAuth.getRestTemplate().getInterceptors()).hasSize(1);
266-
assertBasicAuthorizationInterceptorCredentials(basicAuth, "user", "password");
261+
assertThat(basicAuth.getRestTemplate().getInterceptors()).isEmpty();
262+
assertBasicAuthorizationCredentials(basicAuth, "user", "password");
267263
}
268264

269265
@Test
@@ -394,17 +390,14 @@ private void verifyRelativeUriHandling(TestRestTemplateCallback callback)
394390
verify(requestFactory).createRequest(eq(absoluteUri), any(HttpMethod.class));
395391
}
396392

397-
private void assertBasicAuthorizationInterceptorCredentials(
398-
TestRestTemplate testRestTemplate, String username, String password) {
399-
@SuppressWarnings("unchecked")
400-
List<ClientHttpRequestInterceptor> requestFactoryInterceptors = (List<ClientHttpRequestInterceptor>) ReflectionTestUtils
401-
.getField(testRestTemplate.getRestTemplate().getRequestFactory(),
402-
"interceptors");
403-
assertThat(requestFactoryInterceptors).hasSize(1);
404-
ClientHttpRequestInterceptor interceptor = requestFactoryInterceptors.get(0);
405-
assertThat(interceptor).isInstanceOf(BasicAuthenticationInterceptor.class);
406-
assertThat(interceptor).hasFieldOrPropertyWithValue("username", username);
407-
assertThat(interceptor).hasFieldOrPropertyWithValue("password", password);
393+
private void assertBasicAuthorizationCredentials(TestRestTemplate testRestTemplate,
394+
String username, String password) {
395+
ClientHttpRequestFactory requestFactory = testRestTemplate.getRestTemplate()
396+
.getRequestFactory();
397+
Object authentication = ReflectionTestUtils.getField(requestFactory,
398+
"authentication");
399+
assertThat(authentication).hasFieldOrPropertyWithValue("username", username);
400+
assertThat(authentication).hasFieldOrPropertyWithValue("password", password);
408401

409402
}
410403

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/BasicAuthentication.java

+86
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright 2012-2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.web.client;
18+
19+
import java.nio.charset.Charset;
20+
21+
import org.springframework.util.Assert;
22+
23+
/**
24+
* Basic authentication properties which are used by
25+
* {@link BasicAuthenticationClientHttpRequestFactory}.
26+
*
27+
* @author Dmytro Nosan
28+
* @since 2.2.0
29+
* @see BasicAuthenticationClientHttpRequestFactory
30+
*/
31+
public class BasicAuthentication {
32+
33+
private final String username;
34+
35+
private final String password;
36+
37+
private final Charset charset;
38+
39+
/**
40+
* Create a new {@link BasicAuthentication}.
41+
* @param username the username to use
42+
* @param password the password to use
43+
*/
44+
public BasicAuthentication(String username, String password) {
45+
this(username, password, null);
46+
}
47+
48+
/**
49+
* Create a new {@link BasicAuthentication}.
50+
* @param username the username to use
51+
* @param password the password to use
52+
* @param charset the charset to use
53+
*/
54+
public BasicAuthentication(String username, String password, Charset charset) {
55+
Assert.notNull(username, "Username must not be null");
56+
Assert.notNull(password, "Password must not be null");
57+
this.username = username;
58+
this.password = password;
59+
this.charset = charset;
60+
}
61+
62+
/**
63+
* The username to use.
64+
* @return the username, never {@code null} or {@code empty}.
65+
*/
66+
public String getUsername() {
67+
return this.username;
68+
}
69+
70+
/**
71+
* The password to use.
72+
* @return the password, never {@code null} or {@code empty}.
73+
*/
74+
public String getPassword() {
75+
return this.password;
76+
}
77+
78+
/**
79+
* The charset to use.
80+
* @return the charset, or {@code null}.
81+
*/
82+
public Charset getCharset() {
83+
return this.charset;
84+
}
85+
86+
}

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/BasicAuthenticationClientHttpRequestFactory.java

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Copyright 2012-2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.web.client;
18+
19+
import java.io.IOException;
20+
import java.net.URI;
21+
22+
import org.springframework.http.HttpHeaders;
23+
import org.springframework.http.HttpMethod;
24+
import org.springframework.http.client.AbstractClientHttpRequestFactoryWrapper;
25+
import org.springframework.http.client.ClientHttpRequest;
26+
import org.springframework.http.client.ClientHttpRequestFactory;
27+
import org.springframework.util.Assert;
28+
29+
/**
30+
* {@link ClientHttpRequestFactory} to apply a given HTTP Basic Authentication
31+
* username/password pair, unless a custom Authorization header has been set before.
32+
*
33+
* @author Dmytro Nosan
34+
* @since 2.2.0
35+
*/
36+
public class BasicAuthenticationClientHttpRequestFactory
37+
extends AbstractClientHttpRequestFactoryWrapper {
38+
39+
private final BasicAuthentication authentication;
40+
41+
/**
42+
* Create a new {@link BasicAuthenticationClientHttpRequestFactory} which adds
43+
* {@link HttpHeaders#AUTHORIZATION} header for the given authentication.
44+
* @param authentication the authentication to use
45+
* @param clientHttpRequestFactory the factory to use
46+
*/
47+
public BasicAuthenticationClientHttpRequestFactory(BasicAuthentication authentication,
48+
ClientHttpRequestFactory clientHttpRequestFactory) {
49+
super(clientHttpRequestFactory);
50+
Assert.notNull(authentication, "Authentication must not be null");
51+
this.authentication = authentication;
52+
}
53+
54+
@Override
55+
protected ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod,
56+
ClientHttpRequestFactory requestFactory) throws IOException {
57+
BasicAuthentication authentication = this.authentication;
58+
ClientHttpRequest request = requestFactory.createRequest(uri, httpMethod);
59+
HttpHeaders headers = request.getHeaders();
60+
if (!headers.containsKey(HttpHeaders.AUTHORIZATION)) {
61+
headers.setBasicAuth(authentication.getUsername(),
62+
authentication.getPassword(), authentication.getCharset());
63+
}
64+
return request;
65+
}
66+
67+
}

0 commit comments

Comments
 (0)