Skip to content

Commit 9378d35

Browse files
nictasnictas
committed
Apply stricter encoding to URI query params
Fixes cloudfoundry#1034.
1 parent 8638939 commit 9378d35

28 files changed

+177
-53
lines changed

‎cloudfoundry-client-reactor/src/main/java/org/cloudfoundry/reactor/util/Operator.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,8 @@ class OperatorContextAware {
313313
protected String transformRoot(Function<UriComponentsBuilder, UriComponentsBuilder> uriTransformer) {
314314
UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUriString(this.context.getRoot());
315315
return uriTransformer.apply(uriComponentsBuilder)
316-
.build()
317316
.encode()
317+
.build()
318318
.toUriString();
319319
}
320320

‎cloudfoundry-client-reactor/src/main/java/org/cloudfoundry/reactor/util/UriQueryParameters.java

+8-2
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,14 @@
2323

2424
public class UriQueryParameters {
2525

26-
public static void set(UriComponentsBuilder builder, Stream<UriQueryParameter> parameters) {
27-
parameters.forEach(parameter -> builder.queryParam(parameter.getKey(), parameter.getValue()));
26+
public static void set(UriComponentsBuilder builder, Stream<UriQueryParameter> uriQueryParameters) {
27+
// Replace all literal values with URI variables to apply more strict encoding:
28+
UriVariablesRegistry uriVariablesRegistry = new UriVariablesRegistry();
29+
uriQueryParameters.forEach(uriQueryParameter -> {
30+
UriVariable uriVariable = uriVariablesRegistry.register(uriQueryParameter.getValue());
31+
builder.queryParam(uriQueryParameter.getKey(), uriVariable.getPlaceholder());
32+
});
33+
builder.uriVariables(uriVariablesRegistry.getUriVariablesMap());
2834
}
2935

3036
}

‎cloudfoundry-client-reactor/src/main/java/org/cloudfoundry/reactor/util/UriVariablesRegistry.java

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright 2013-2020 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+
* http://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.cloudfoundry.reactor.util;
18+
19+
import java.util.ArrayList;
20+
import java.util.List;
21+
import java.util.Map;
22+
import java.util.stream.Collectors;
23+
24+
public class UriVariablesRegistry {
25+
26+
private final List<UriVariable> uriVariables = new ArrayList<>();
27+
28+
public Map<String, Object> getUriVariablesMap() {
29+
return uriVariables.stream().collect(Collectors.toMap(UriVariable::getKey, UriVariable::getValue));
30+
}
31+
32+
public UriVariable register(Object value) {
33+
UriVariable uriVariable = UriVariable.of(getNextUriVariableKey(), value);
34+
uriVariables.add(uriVariable);
35+
return uriVariable;
36+
}
37+
38+
private String getNextUriVariableKey() {
39+
return Integer.toString(uriVariables.size());
40+
}
41+
42+
}

‎cloudfoundry-client-reactor/src/main/java/org/cloudfoundry/reactor/util/_UriVariable.java

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright 2013-2020 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+
* http://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.cloudfoundry.reactor.util;
18+
19+
import org.immutables.value.Value;
20+
21+
@Value.Immutable
22+
public interface _UriVariable {
23+
24+
String PLACEHOLDER_PATTERN = "{%s}";
25+
26+
@Value.Parameter
27+
String getKey();
28+
29+
@Value.Parameter
30+
Object getValue();
31+
32+
@Value.Derived
33+
default String getPlaceholder() {
34+
return String.format(PLACEHOLDER_PATTERN, getKey());
35+
}
36+
37+
}

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/QueryExtractorTest.java

+9-3
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,15 @@ public void test() {
3939
Stream<UriQueryParameter> parameters = new QueryExtractor().extract(new StubQueryParamsSubClass());
4040
UriQueryParameters.set(builder, parameters);
4141

42-
MultiValueMap<String, String> queryParams = builder.build().encode().getQueryParams();
42+
MultiValueMap<String, String> queryParams = builder.encode().build().getQueryParams();
4343

44-
assertThat(queryParams).hasSize(7);
44+
assertThat(queryParams).hasSize(8);
4545
assertThat(queryParams.getFirst("test-single")).isEqualTo("test-value-1");
46-
assertThat(queryParams.getFirst("test-collection")).isEqualTo("test-value-2,test-value-3");
46+
assertThat(queryParams.getFirst("test-collection")).isEqualTo("test-value-2%2Ctest-value-3");
4747
assertThat(queryParams.getFirst("test-collection-custom-delimiter")).isEqualTo("test-value-4%20test-value-5");
4848
assertThat(queryParams.getFirst("test-subclass")).isEqualTo("test-value-6");
4949
assertThat(queryParams.getFirst("test-override")).isEqualTo("test-value-7");
50+
assertThat(queryParams.getFirst("test-reserved-characters")).isEqualTo("%3A%2F%3F%23%5B%5D%40%21%24%26%27%28%29%2A%2B%2C%3B%3D");
5051
}
5152

5253
public static abstract class StubQueryParams {
@@ -81,6 +82,11 @@ public final String getSingle() {
8182
return "test-value-1";
8283
}
8384

85+
@QueryParameter("test-reserved-characters")
86+
public final String getReservedCharacters() {
87+
return ":/?#[]@!$&'()*+,;=";
88+
}
89+
8490
@QueryParameter("test-override")
8591
abstract String getOverride();
8692

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/FilterExtractorTest.java

+13-7
Original file line numberDiff line numberDiff line change
@@ -43,20 +43,21 @@ public void test() {
4343
Stream<UriQueryParameter> parameters = new FilterExtractor().extract(new StubFilterParamsSubClass());
4444
UriQueryParameters.set(builder, parameters);
4545

46-
MultiValueMap<String, String> queryParams = builder.build().encode().getQueryParams();
46+
MultiValueMap<String, String> queryParams = builder.encode().build().getQueryParams();
4747
List<String> q = queryParams.get("q");
4848

4949
assertThat(q)
50-
.hasSize(9)
51-
.containsOnly("test-empty-value:",
50+
.hasSize(10)
51+
.containsOnly("test-empty-value%3A",
5252
"test-greater-than%3Etest-value-1",
5353
"test-greater-than-or-equal-to%3E%3Dtest-value-2",
54-
"test-in%20IN%20test-value-3,test-value-4",
55-
"test-is:test-value-5",
54+
"test-in%20IN%20test-value-3%2Ctest-value-4",
55+
"test-is%3Atest-value-5",
5656
"test-less-than%3Ctest-value-6",
5757
"test-less-than-or-equal-to%3C%3Dtest-value-7",
58-
"test-default%20IN%20test-value-8,test-value-9",
59-
"test-override:test-value-10");
58+
"test-default%20IN%20test-value-8%2Ctest-value-9",
59+
"test-override%3Atest-value-10",
60+
"test-reserved-characters%3A%3A%2F%3F%23%5B%5D%40%21%24%26%27%28%29%2A%2B%2C%3B%3D");
6061
}
6162

6263
public static abstract class StubFilterParams {
@@ -101,6 +102,11 @@ public final String getLessThanOrEqualTo() {
101102
return "test-value-7";
102103
}
103104

105+
@FilterParameter("test-reserved-characters")
106+
public final String getReservedCharacters() {
107+
return ":/?#[]@!$&'()*+,;=";
108+
}
109+
104110
@FilterParameter("test-null")
105111
public final String getNull() {
106112
return null;

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/applications/ReactorApplicationsV2Test.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ public void instances() {
477477
public void list() {
478478
mockRequest(InteractionContext.builder()
479479
.request(TestRequest.builder()
480-
.method(GET).path("/apps?q=name:test-name&page=-1")
480+
.method(GET).path("/apps?q=name%3Atest-name&page=-1")
481481
.build())
482482
.response(TestResponse.builder()
483483
.status(OK)
@@ -649,7 +649,7 @@ public void listRoutes() {
649649
public void listServiceBindings() {
650650
mockRequest(InteractionContext.builder()
651651
.request(TestRequest.builder()
652-
.method(GET).path("/apps/test-application-id/service_bindings?q=service_instance_guid:test-instance-id&page=-1")
652+
.method(GET).path("/apps/test-application-id/service_bindings?q=service_instance_guid%3Atest-instance-id&page=-1")
653653
.build())
654654
.response(TestResponse.builder()
655655
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/buildpacks/ReactorBuildpacksTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public void get() {
168168
public void list() {
169169
mockRequest(InteractionContext.builder()
170170
.request(TestRequest.builder()
171-
.method(GET).path("/buildpacks?q=name:test-name&page=-1")
171+
.method(GET).path("/buildpacks?q=name%3Atest-name&page=-1")
172172
.build())
173173
.response(TestResponse.builder()
174174
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/events/ReactorEventsTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public void get() {
8585
public void list() {
8686
mockRequest(InteractionContext.builder()
8787
.request(TestRequest.builder()
88-
.method(GET).path("/events?q=actee:test-actee&page=-1")
88+
.method(GET).path("/events?q=actee%3Atest-actee&page=-1")
8989
.build())
9090
.response(TestResponse.builder()
9191
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/organizations/ReactorOrganizationsTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,7 @@ public void getUserRoles() {
776776
public void list() {
777777
mockRequest(InteractionContext.builder()
778778
.request(TestRequest.builder()
779-
.method(GET).path("/organizations?q=name:test-name&page=-1")
779+
.method(GET).path("/organizations?q=name%3Atest-name&page=-1")
780780
.build())
781781
.response(TestResponse.builder()
782782
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/privatedomains/ReactorPrivateDomainsTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ public void get() {
180180
public void list() {
181181
mockRequest(InteractionContext.builder()
182182
.request(TestRequest.builder()
183-
.method(GET).path("/private_domains?q=name:test-name.com&page=-1")
183+
.method(GET).path("/private_domains?q=name%3Atest-name.com&page=-1")
184184
.build())
185185
.response(TestResponse.builder()
186186
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/servicebindings/ReactorServiceBindingsV2Test.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ public void list() {
232232
mockRequest(InteractionContext.builder()
233233
.request(TestRequest.builder()
234234
.method(GET)
235-
.path("/service_bindings?q=app_guid:dd44fd4f-5e20-4c52-b66d-7af6e201f01e&page=-1")
235+
.path("/service_bindings?q=app_guid%3Add44fd4f-5e20-4c52-b66d-7af6e201f01e&page=-1")
236236
.build())
237237
.response(TestResponse.builder()
238238
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/servicebrokers/ReactorServiceBrokersTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ public void get() {
146146
public void list() {
147147
mockRequest(InteractionContext.builder()
148148
.request(TestRequest.builder()
149-
.method(GET).path("/service_brokers?q=name:test-name&page=-1")
149+
.method(GET).path("/service_brokers?q=name%3Atest-name&page=-1")
150150
.build())
151151
.response(TestResponse.builder()
152152
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/serviceinstances/ReactorServiceInstancesTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ public void list() {
398398
mockRequest(InteractionContext.builder()
399399
.request(TestRequest.builder()
400400
.method(GET)
401-
.path("/service_instances?q=name:test-name&page=-1")
401+
.path("/service_instances?q=name%3Atest-name&page=-1")
402402
.build())
403403
.response(TestResponse.builder()
404404
.status(OK)
@@ -498,7 +498,7 @@ public void listServiceBindings() {
498498
mockRequest(InteractionContext.builder()
499499
.request(TestRequest.builder()
500500
.method(GET)
501-
.path("/service_instances/test-service-instance-id/service_bindings?q=app_guid:test-application-id&page=-1")
501+
.path("/service_instances/test-service-instance-id/service_bindings?q=app_guid%3Atest-application-id&page=-1")
502502
.build())
503503
.response(TestResponse.builder()
504504
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/servicekeys/ReactorServiceKeysTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ public void get() {
143143
public void list() {
144144
mockRequest(InteractionContext.builder()
145145
.request(TestRequest.builder()
146-
.method(GET).path("/service_keys?q=name:test-name&page=-1")
146+
.method(GET).path("/service_keys?q=name%3Atest-name&page=-1")
147147
.build())
148148
.response(TestResponse.builder()
149149
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/serviceplans/ReactorServicePlansTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public void get() {
178178
public void list() {
179179
mockRequest(InteractionContext.builder()
180180
.request(TestRequest.builder()
181-
.method(GET).path("/service_plans?q=service_guid:test-service-id&page=-1")
181+
.method(GET).path("/service_plans?q=service_guid%3Atest-service-id&page=-1")
182182
.build())
183183
.response(TestResponse.builder()
184184
.status(OK)
@@ -222,7 +222,7 @@ public void list() {
222222
public void listServiceInstances() {
223223
mockRequest(InteractionContext.builder()
224224
.request(TestRequest.builder()
225-
.method(GET).path("/service_plans/test-service-plan-id/service_instances?q=space_guid:test-space-id&page=-1")
225+
.method(GET).path("/service_plans/test-service-plan-id/service_instances?q=space_guid%3Atest-space-id&page=-1")
226226
.build())
227227
.response(TestResponse.builder()
228228
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/serviceplanvisibilities/ReactorServicePlanVisibilitiesTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ public void get() {
181181
public void list() {
182182
mockRequest(InteractionContext.builder()
183183
.request(TestRequest.builder()
184-
.method(GET).path("/service_plan_visibilities?q=organization_guid:test-organization-id&page=-1")
184+
.method(GET).path("/service_plan_visibilities?q=organization_guid%3Atest-organization-id&page=-1")
185185
.build())
186186
.response(TestResponse.builder()
187187
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/services/ReactorServicesTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ public void get() {
149149
public void list() {
150150
mockRequest(InteractionContext.builder()
151151
.request(TestRequest.builder()
152-
.method(GET).path("/services?q=label:test-label&page=-1")
152+
.method(GET).path("/services?q=label%3Atest-label&page=-1")
153153
.build())
154154
.response(TestResponse.builder()
155155
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/spaces/ReactorSpacesTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,7 @@ public void getSummary() {
697697
public void list() {
698698
mockRequest(InteractionContext.builder()
699699
.request(TestRequest.builder()
700-
.method(GET).path("/spaces?q=name:test-name&page=-1")
700+
.method(GET).path("/spaces?q=name%3Atest-name&page=-1")
701701
.build())
702702
.response(TestResponse.builder()
703703
.status(OK)
@@ -747,7 +747,7 @@ public void list() {
747747
public void listApplications() {
748748
mockRequest(InteractionContext.builder()
749749
.request(TestRequest.builder()
750-
.method(GET).path("/spaces/test-space-id/apps?q=name:test-name&page=-1")
750+
.method(GET).path("/spaces/test-space-id/apps?q=name%3Atest-name&page=-1")
751751
.build())
752752
.response(TestResponse.builder()
753753
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v2/stacks/ReactorStacksTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public void get() {
172172
public void list() {
173173
mockRequest(InteractionContext.builder()
174174
.request(TestRequest.builder()
175-
.method(GET).path("/stacks?q=name:test-name&page=-1")
175+
.method(GET).path("/stacks?q=name%3Atest-name&page=-1")
176176
.build())
177177
.response(TestResponse.builder()
178178
.status(OK)

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v3/FilterExtractorTest.java

+9-3
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,14 @@ public void test() {
3939
Stream<UriQueryParameter> parameters = new FilterExtractor().extract(new StubFilterParamsSubClass());
4040
UriQueryParameters.set(builder, parameters);
4141

42-
MultiValueMap<String, String> queryParams = builder.build().encode().getQueryParams();
42+
MultiValueMap<String, String> queryParams = builder.encode().build().getQueryParams();
4343

44-
assertThat(queryParams).hasSize(5);
44+
assertThat(queryParams).hasSize(6);
4545
assertThat(queryParams.getFirst("test-single")).isEqualTo("test-value-1");
46-
assertThat(queryParams.getFirst("test-collection")).isEqualTo("test-value-2,test-value-3");
46+
assertThat(queryParams.getFirst("test-collection")).isEqualTo("test-value-2%2Ctest-value-3");
4747
assertThat(queryParams.getFirst("test-subclass")).isEqualTo("test-value-4");
4848
assertThat(queryParams.getFirst("test-override")).isEqualTo("test-value-7");
49+
assertThat(queryParams.getFirst("test-reserved-characters")).isEqualTo("%3A%2F%3F%23%5B%5D%40%21%24%26%27%28%29%2A%2B%2C%3B%3D");
4950
}
5051

5152
public static abstract class StubFilterParams {
@@ -75,6 +76,11 @@ public final String getSingle() {
7576
return "test-value-1";
7677
}
7778

79+
@FilterParameter("test-reserved-characters")
80+
public final String getReservedCharacters() {
81+
return ":/?#[]@!$&'()*+,;=";
82+
}
83+
7884
@FilterParameter("test-override")
7985
abstract String getOverride();
8086

‎cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/client/v3/servicebindings/ReactorServiceBindingsV3Test.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ public void get() {
166166
public void list() {
167167
mockRequest(InteractionContext.builder()
168168
.request(TestRequest.builder()
169-
.method(GET).path("/service_bindings?app_guids=test-application-id&order_by=+created_at&page=1")
169+
.method(GET).path("/service_bindings?app_guids=test-application-id&order_by=%2Bcreated_at&page=1")
170170
.build())
171171
.response(TestResponse.builder()
172172
.status(OK)

0 commit comments

Comments
 (0)