Skip to content

bugfix: encoded query parameter (web) #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 29, 2023
Merged

bugfix: encoded query parameter (web) #18

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d22dc27
Fix problem with encoded comma separated list in query string
pboos Jun 28, 2023
a74634e
Improve handling of query params for some edge cases
pboos Jun 28, 2023
6b5255a
Updated test
pboos Jun 28, 2023
f064a2c
Sample rate for examples to 1 (100%)
pboos Jun 28, 2023
28147dd
Do urldecode on the params when building the "SimpleRequest"
pboos Jun 29, 2023
3b22f8a
Revert also in spring2.7
pboos Jun 29, 2023
1ea62e8
Update comment in build.gradle
pboos Jun 29, 2023
3dfb542
Add test
pboos Jun 29, 2023
cdb7e81
Remove unused import
pboos Jun 29, 2023
f22fb52
Remove unused import
pboos Jun 29, 2023
3629173
Rename method & pass in only value
pboos Jun 29, 2023
4c323f3
Test more special characters
pboos Jun 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/example-spring-boot-starter-web/src/main/resources/application.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
openapi.validation.sample-rate=0.5
openapi.validation.sample-rate=1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Unrelated to fix. Increasing to 1 for better example.

openapi.validation.specification-file-path=openapi.yaml
openapi.validation.excluded-paths=/_readiness,/_liveness,/_metrics
openapi.validation.validation-report-throttle-wait-seconds=10
Expand Down
2 changes: 1 addition & 1 deletion examples/example-spring-boot-starter-webflux/src/main/resources/application.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
openapi.validation.sample-rate=0.5
openapi.validation.sample-rate=1
openapi.validation.specification-file-path=openapi.yaml
openapi.validation.excluded-paths=/_readiness,/_liveness,/_metrics
openapi.validation.validation-report-throttle-wait-seconds=10
Expand Down
12 changes: 11 additions & 1 deletion ...-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
import com.getyourguide.openapi.validation.api.model.ValidationResult;
import com.getyourguide.openapi.validation.api.model.ValidatorConfiguration;
import com.getyourguide.openapi.validation.core.validator.OpenApiInteractionValidatorWrapper;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.ThreadPoolExecutor;
import lombok.extern.slf4j.Slf4j;
import org.apache.http.NameValuePair;
import org.apache.http.client.utils.URLEncodedUtils;

@Slf4j
Expand Down Expand Up @@ -79,14 +81,22 @@ public ValidationResult validateRequestObject(final RequestMetaData request, Str
private static SimpleRequest buildSimpleRequest(RequestMetaData request, String requestBody) {
var requestBuilder = new SimpleRequest.Builder(request.getMethod(), request.getUri().getPath());
URLEncodedUtils.parse(request.getUri(), StandardCharsets.UTF_8)
.forEach(p -> requestBuilder.withQueryParam(p.getName(), p.getValue()));
.forEach(p -> requestBuilder.withQueryParam(p.getName(), getQueryParameterValueWithOptionalDecode(p)));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ This is the actual fix. All the rest is just other small improvements.

if (requestBody != null) {
requestBuilder.withBody(requestBody);
}
request.getHeaders().forEach(requestBuilder::withHeader);
return requestBuilder.build();
}

private static String getQueryParameterValueWithOptionalDecode(NameValuePair p) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why we pass in the NameValuePair instead of directly the value. Why not pass in p.getValue() and rename the method to nullSafeURLDecode?

if (p.getValue() == null) {
return null;
}

return URLDecoder.decode(p.getValue(), StandardCharsets.UTF_8);
}

public ValidationResult validateResponseObject(
final RequestMetaData request,
ResponseMetaData response,
Expand Down
3 changes: 1 addition & 2 deletions ...2.7/src/main/java/com/getyourguide/openapi/validation/factory/ServletMetaDataFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.getyourguide.openapi.validation.api.model.RequestMetaData;
import com.getyourguide.openapi.validation.api.model.ResponseMetaData;
import java.util.Map;
import java.util.TreeMap;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
Expand All @@ -13,7 +12,7 @@ public class ServletMetaDataFactory {
public static final String HEADER_CONTENT_TYPE = "Content-Type";

public RequestMetaData buildRequestMetaData(HttpServletRequest request) {
var uri = ServletUriComponentsBuilder.fromRequest(request).build(Map.of());
var uri = ServletUriComponentsBuilder.fromRequest(request).build().toUri();
Copy link
Contributor Author

@pboos pboos Jun 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Unrelated to fix. Not strictly needed, but this simplifies the code as internally the old call did this + additional unnecessary logic.

return new RequestMetaData(request.getMethod(), uri, getHeaders(request));
}

Expand Down
1 change: 1 addition & 0 deletions spring-boot-starter/spring-boot-starter-web/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ dependencies {

testImplementation 'org.springframework.boot:spring-boot-starter-test'
testImplementation 'org.springframework:spring-web'
testImplementation 'org.springframework:spring-webmvc'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Unrelated to fix. Not needed right now. But this dependency is useful for writing tests in the future.

Background:

I wrote some tests that were for a previous fix of the problem, but they got reverted as I found a nicer way to fix it. When I wrote those tests this dependency was needed for the tests to run. And it took a while to find this dependency to make the tests work. So better leave them for now so we save time in the future :)

testImplementation 'org.apache.tomcat.embed:tomcat-embed-core' // For jakarta.servlet.ServletContext
}
3 changes: 1 addition & 2 deletions ...web/src/main/java/com/getyourguide/openapi/validation/factory/ServletMetaDataFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import com.getyourguide.openapi.validation.api.model.ResponseMetaData;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.util.Map;
import java.util.TreeMap;
import org.springframework.web.servlet.support.ServletUriComponentsBuilder;

Expand All @@ -13,7 +12,7 @@ public class ServletMetaDataFactory {
public static final String HEADER_CONTENT_TYPE = "Content-Type";

public RequestMetaData buildRequestMetaData(HttpServletRequest request) {
var uri = ServletUriComponentsBuilder.fromRequest(request).build(Map.of());
var uri = ServletUriComponentsBuilder.fromRequest(request).build().toUri();
return new RequestMetaData(request.getMethod(), uri, getHeaders(request));
}

Expand Down