Skip to content

Commit 9e93935

Browse files
author
Corneil du Plessis
committed
Replace RedirectExec with modified chain handler.
Fixes spring-attic#5989
1 parent 88ddf2b commit 9e93935

File tree

4 files changed

+315
-98
lines changed

4 files changed

+315
-98
lines changed

spring-cloud-dataflow-configuration-metadata/src/test/java/org/springframework/cloud/dataflow/container/registry/authorization/support/S3SignedRedirectRequestController.java

+9-2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
import java.util.Collections;
2020
import java.util.Map;
2121

22+
import org.slf4j.Logger;
23+
import org.slf4j.LoggerFactory;
24+
2225
import org.springframework.cloud.dataflow.container.registry.ContainerRegistryProperties;
2326
import org.springframework.core.io.ByteArrayResource;
2427
import org.springframework.core.io.Resource;
@@ -36,6 +39,7 @@
3639
*/
3740
@RestController
3841
public class S3SignedRedirectRequestController {
42+
private final static Logger logger = LoggerFactory.getLogger(S3SignedRedirectRequestController.class);
3943

4044
@RequestMapping("/service/token")
4145
public ResponseEntity<Map<String, String>> getToken() {
@@ -53,6 +57,7 @@ public ResponseEntity<Resource> getManifests(@RequestHeader("Authorization") Str
5357
@RequestMapping("/v2/test/s3-redirect-image/blobs/signed_redirect_digest")
5458
public ResponseEntity<Map<String, String>> getBlobRedirect(@RequestHeader("Authorization") String token) {
5559
if (!"bearer my_token_999".equals(token.trim().toLowerCase())) {
60+
logger.info("getBlobRedirect=BAD_REQUEST");
5661
return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
5762
}
5863
HttpHeaders redirectHeaders = new HttpHeaders();
@@ -63,13 +68,15 @@ public ResponseEntity<Map<String, String>> getBlobRedirect(@RequestHeader("Autho
6368
"&X-Amz-Expires=1200" +
6469
"&X-Amz-SignedHeaders=host" +
6570
"&X-Amz-Signature=test");
66-
71+
logger.info("getBlobRedirect:{}", redirectHeaders);
6772
return new ResponseEntity<>(redirectHeaders, HttpStatus.TEMPORARY_REDIRECT);
6873
}
6974

7075
@RequestMapping("/test/docker/registry/v2/blobs/test/data")
7176
public ResponseEntity<Resource> getSignedBlob(@RequestHeader Map<String, String> headers) {
72-
if (!headers.containsKey("authorization")) {
77+
logger.info("getSignedBlob:{}", headers);
78+
if (headers.containsKey("authorization")) {
79+
logger.info("getSignedBlob=BAD_REQUEST");
7380
return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
7481
}
7582
return buildFromString("{\"config\": {\"Labels\": {\"foo\": \"bar\"} } }");

spring-cloud-dataflow-container-registry/src/main/java/org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java

+21-9
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,13 @@
3131

3232
import org.apache.hc.client5.http.config.RequestConfig;
3333
import org.apache.hc.client5.http.cookie.StandardCookieSpec;
34+
import org.apache.hc.client5.http.impl.ChainElement;
35+
import org.apache.hc.client5.http.impl.DefaultSchemePortResolver;
3436
import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
3537
import org.apache.hc.client5.http.impl.classic.HttpClients;
3638
import org.apache.hc.client5.http.impl.io.BasicHttpClientConnectionManager;
39+
import org.apache.hc.client5.http.impl.routing.DefaultProxyRoutePlanner;
40+
import org.apache.hc.client5.http.impl.routing.DefaultRoutePlanner;
3741
import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
3842
import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory;
3943
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
@@ -44,12 +48,12 @@
4448

4549
import org.springframework.boot.web.client.RestTemplateBuilder;
4650
import org.springframework.cloud.dataflow.container.registry.authorization.DropAuthorizationHeaderRequestRedirectStrategy;
51+
import org.springframework.cloud.dataflow.container.registry.authorization.SpecialRedirectExec;
4752
import org.springframework.http.MediaType;
4853
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
4954
import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
5055
import org.springframework.web.client.RestTemplate;
5156

52-
5357
/**
5458
* On demand creates a cacheable {@link RestTemplate} instances for the purpose of the Container Registry access.
5559
* Created RestTemplates can be configured to use Http Proxy and/or bypassing the SSL verification.
@@ -83,6 +87,7 @@
8387
*
8488
* @author Christian Tzolov
8589
* @author Cheng Guan Poh
90+
* @author Corneil du Plessis
8691
*/
8792
public class ContainerImageRestTemplateFactory {
8893

@@ -197,23 +202,30 @@ private HttpClientBuilder httpClientBuilder(SSLContext sslContext) {
197202
private RestTemplate initRestTemplate(HttpClientBuilder clientBuilder, boolean withHttpProxy, Map<String, String> extra) {
198203

199204
clientBuilder.setDefaultRequestConfig(RequestConfig.custom().setCookieSpec(StandardCookieSpec.RELAXED).build());
200-
205+
DefaultRoutePlanner routePlanner;
201206
// Set the HTTP proxy if configured.
202207
if (withHttpProxy) {
203208
if (!properties.getHttpProxy().isEnabled()) {
204209
throw new ContainerRegistryException("Registry Configuration uses a HttpProxy but non is configured!");
205210
}
206211
HttpHost proxy = new HttpHost(properties.getHttpProxy().getHost(), properties.getHttpProxy().getPort());
207212
clientBuilder.setProxy(proxy);
213+
routePlanner = new DefaultProxyRoutePlanner(proxy, DefaultSchemePortResolver.INSTANCE);
214+
}
215+
else {
216+
routePlanner = new DefaultRoutePlanner(DefaultSchemePortResolver.INSTANCE);
208217
}
209218

210-
HttpComponentsClientHttpRequestFactory customRequestFactory =
211-
new HttpComponentsClientHttpRequestFactory(
212-
clientBuilder
213-
.setRedirectStrategy(new DropAuthorizationHeaderRequestRedirectStrategy(extra))
214-
// Azure redirects may contain double slashes and on default those are normilised
215-
.setDefaultRequestConfig(RequestConfig.custom().build())
216-
.build());
219+
DropAuthorizationHeaderRequestRedirectStrategy redirectStrategy = new DropAuthorizationHeaderRequestRedirectStrategy(
220+
extra);
221+
HttpComponentsClientHttpRequestFactory customRequestFactory = new HttpComponentsClientHttpRequestFactory(
222+
clientBuilder.setRedirectStrategy(redirectStrategy)
223+
.replaceExecInterceptor(ChainElement.REDIRECT.name(),
224+
new SpecialRedirectExec(routePlanner, redirectStrategy))
225+
// Azure redirects may contain double slashes and on default those are
226+
// normilised
227+
.setDefaultRequestConfig(RequestConfig.custom().build())
228+
.build());
217229

218230
// DockerHub response's media-type is application/octet-stream although the content is in JSON.
219231
// Similarly the Github CR response's media-type is always text/plain although the content is in JSON.

spring-cloud-dataflow-container-registry/src/main/java/org/springframework/cloud/dataflow/container/registry/authorization/DropAuthorizationHeaderRequestRedirectStrategy.java

+59-87
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,10 @@
1818

1919
import java.net.URI;
2020
import java.net.URISyntaxException;
21-
import java.util.Arrays;
2221
import java.util.Map;
2322

2423
import org.apache.hc.client5.http.classic.methods.HttpGet;
2524
import org.apache.hc.client5.http.classic.methods.HttpHead;
26-
import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
2725
import org.apache.hc.client5.http.impl.DefaultRedirectStrategy;
2826
import org.apache.hc.core5.http.Header;
2927
import org.apache.hc.core5.http.HttpException;
@@ -62,6 +60,7 @@
6260
* @author Janne Valkealahti
6361
* @author Christian Tzolov
6462
* @author Cheng Guan Poh
63+
* @author Corneil du Plessis
6564
*/
6665
public class DropAuthorizationHeaderRequestRedirectStrategy extends DefaultRedirectStrategy {
6766

@@ -92,109 +91,82 @@ public URI getLocationURI(final HttpRequest request, final HttpResponse response
9291
URI httpUriRequest = super.getLocationURI(request, response, context);
9392
String query = httpUriRequest.getQuery();
9493
String method = request.getMethod();
95-
9694
// Handle Amazon requests
9795
if (StringUtils.hasText(query) && query.contains(AMZ_CREDENTIAL)) {
9896
if (isHeadOrGetMethod(method)) {
99-
try {
100-
return new DropAuthorizationHeaderHttpRequestBase(httpUriRequest, method).getUri();
101-
} catch (URISyntaxException e) {
102-
throw new HttpException("Unable to get location URI", e);
103-
}
104-
}
97+
removeAuthorizationHeader(request, response, false);
98+
try {
99+
if (isHeadMethod(method)) {
100+
return new HttpHead(httpUriRequest).getUri();
101+
}
102+
else {
103+
return new HttpGet(httpUriRequest).getUri();
104+
}
105+
}
106+
catch (URISyntaxException e) {
107+
throw new HttpException("Unable to get location URI", e);
108+
}
109+
}
105110
}
106111

107112
// Handle Azure requests
108-
try {
109-
if (request.getUri().getRawPath().contains(AZURECR_URI_SUFFIX)) {
110-
if (isHeadOrGetMethod(method)) {
111-
return (new DropAuthorizationHeaderHttpRequestBase(httpUriRequest, method) {
112-
// Drop headers only for the Basic Auth and leave unchanged for OAuth2
113-
@Override
114-
protected boolean isDropHeader(String name, Object value) {
115-
return name.equalsIgnoreCase(AUTHORIZATION_HEADER) && StringUtils.hasText((String) value) && ((String)value).contains(BASIC_AUTH);
116-
}
117-
}).getUri();
118-
}
119-
}
120-
121-
122-
// Handle Custom requests
123-
if (extra.containsKey(CUSTOM_REGISTRY) && request.getUri().getRawPath().contains(extra.get(CUSTOM_REGISTRY))) {
124-
if (isHeadOrGetMethod(method)) {
125-
return new DropAuthorizationHeaderHttpRequestBase(httpUriRequest, method).getUri();
113+
try {
114+
if (request.getUri().getRawPath().contains(AZURECR_URI_SUFFIX)) {
115+
if (isHeadOrGetMethod(method)) {
116+
removeAuthorizationHeader(request, response, true);
117+
if (isHeadMethod(method)) {
118+
return new HttpHead(httpUriRequest).getUri();
119+
}
120+
else {
121+
return new HttpGet(httpUriRequest).getUri();
122+
}
123+
}
124+
}
125+
126+
// Handle Custom requests
127+
if (extra.containsKey(CUSTOM_REGISTRY)
128+
&& request.getUri().getRawPath().contains(extra.get(CUSTOM_REGISTRY))) {
129+
if (isHeadOrGetMethod(method)) {
130+
removeAuthorizationHeader(request, response, false);
131+
if (isHeadMethod(method)) {
132+
return new HttpHead(httpUriRequest).getUri();
133+
}
134+
else {
135+
return new HttpGet(httpUriRequest).getUri();
136+
}
137+
}
126138
}
127139
}
128-
} catch (URISyntaxException e) {
140+
catch (URISyntaxException e) {
129141
throw new HttpException("Unable to get Locaction URI", e);
130142
}
131143
return httpUriRequest;
132144
}
133145

134-
private boolean isHeadOrGetMethod(String method) {
135-
return StringUtils.hasText(method)
136-
&& (method.equalsIgnoreCase(HttpHead.METHOD_NAME) || method.equalsIgnoreCase(HttpGet.METHOD_NAME));
137-
}
138-
139-
/**
140-
* Overrides all header setter methods to filter out the Authorization headers.
141-
*/
142-
static class DropAuthorizationHeaderHttpRequestBase extends HttpUriRequestBase {
143-
144-
private final String method;
145-
146-
DropAuthorizationHeaderHttpRequestBase(URI uri, String method) {
147-
super(method, uri);
148-
this.method = method;
149-
}
150-
151-
@Override
152-
public String getMethod() {
153-
return this.method;
154-
}
155-
156-
@Override
157-
public void addHeader(Header header) {
158-
if (!isDropHeader(header)) {
159-
super.addHeader(header);
146+
private static void removeAuthorizationHeader(HttpRequest request, HttpResponse response, boolean onlyBasicAuth) {
147+
for (Header header : response.getHeaders()) {
148+
if (header.getName().equalsIgnoreCase(AUTHORIZATION_HEADER)
149+
&& (!onlyBasicAuth || (onlyBasicAuth && header.getValue().contains(BASIC_AUTH)))) {
150+
response.removeHeaders(header.getName());
151+
break;
160152
}
161153
}
162-
163-
@Override
164-
public void addHeader(String name, Object value) {
165-
if (!isDropHeader(name, value)) {
166-
super.addHeader(name, value);
154+
for (Header header : request.getHeaders()) {
155+
if (header.getName().equalsIgnoreCase(AUTHORIZATION_HEADER)
156+
&& (!onlyBasicAuth || (onlyBasicAuth && header.getValue().contains(BASIC_AUTH)))) {
157+
request.removeHeaders(header.getName());
158+
break;
167159
}
168160
}
161+
}
169162

170-
@Override
171-
public void setHeader(Header header) {
172-
if (!isDropHeader(header)) {
173-
super.setHeader(header);
174-
}
175-
}
176-
177-
@Override
178-
public void setHeader(String name, Object value) {
179-
if (!isDropHeader(name, value)) {
180-
super.setHeader(name, value);
181-
}
182-
}
183-
184-
@Override
185-
public void setHeaders(Header[] headers) {
186-
Header[] filteredHeaders = Arrays.stream(headers)
187-
.filter(header -> !isDropHeader(header))
188-
.toArray(Header[]::new);
189-
super.setHeaders(filteredHeaders);
190-
}
191-
192-
protected boolean isDropHeader(Header header) {
193-
return isDropHeader(header.getName(), header.getValue());
194-
}
163+
private boolean isHeadOrGetMethod(String method) {
164+
return StringUtils.hasText(method)
165+
&& (method.equalsIgnoreCase(HttpHead.METHOD_NAME) || method.equalsIgnoreCase(HttpGet.METHOD_NAME));
166+
}
195167

196-
protected boolean isDropHeader(String name, Object value) {
197-
return name.equalsIgnoreCase(AUTHORIZATION_HEADER);
198-
}
168+
private boolean isHeadMethod(String method) {
169+
return StringUtils.hasText(method) && method.equalsIgnoreCase(HttpHead.METHOD_NAME);
199170
}
171+
200172
}

0 commit comments

Comments
 (0)