From 8b50dc4874b3d90ad790ce42b35883a09e1e3f3f Mon Sep 17 00:00:00 2001 From: Aaron Whiteside Date: Mon, 19 Apr 2021 21:43:05 +1000 Subject: [PATCH] Introduce path encoding (fixes #1561) HttpRequestBuilder.java - replaced armeria QueryParamsBuilder with apache http `URIBuilder`. - minor generics cleanup - `URIBuilder` now handles path segment encoding and query parameters. - we now check if any paths are prefixed with `/` and issue a warning log, that this is probably a mistake encoding.feature - added more demo/test cases for path encoding HttpUtils.java - minor generics cleanup - final is redundant on static methods karate-demo/pom.xml - scope = import only works in the dependencyManagement section, this fixes the maven warning. Request.java - minor generics cleanup - use computeIfAbsent() HttpClientFactory.java - public static final are redundant on interface fields - also use method reference KarateMockHandlerTest.java KarateHttpMockHandlerTest.java - removed all `/` path prefixes README.md - updated with details on how to correctly use path - changes any erroneous examples of path usage --- README.md | 12 +++-- .../karate/http/HttpRequestBuilder.java | 44 +++++----------- .../core/KarateHttpMockHandlerTest.java | 20 +++---- .../karate/core/KarateMockHandlerTest.java | 52 +++++++++---------- .../karate/http/RequestHandlerTest.java | 4 +- .../src/test/java/demo/DemoTestParallel.java | 2 +- .../test/java/demo/encoding/encoding.feature | 9 +--- .../src/test/java/demo/error/no-url.feature | 4 +- 8 files changed, 65 insertions(+), 82 deletions(-) diff --git a/README.md b/README.md index 6f64d4c1d..e3a9d62f6 100755 --- a/README.md +++ b/README.md @@ -1589,17 +1589,23 @@ Given url 'https://' + e2eHostName + '/v1/api' If you are trying to build dynamic URLs including query-string parameters in the form: `http://myhost/some/path?foo=bar&search=true` - please refer to the [`param`](#param) keyword. ## `path` -REST-style path parameters. Can be expressions that will be evaluated. Comma delimited values are supported which can be more convenient, and takes care of URL-encoding and appending '/' where needed. +REST-style path parameters. Can be expressions that will be evaluated. Comma delimited values are supported which can be more convenient, and takes care of URL-encoding and appending '/' between path segments as needed. ```cucumber +# this is invalid and will result in / being encoded as %2F when sent to the remote server +# eg. given a documentId of 1234 the path will be: /documents%2F1234%2Fdownload Given path 'documents/' + documentId + '/download' -# this is equivalent to the above +# this is the correct way to specify multiple paths Given path 'documents', documentId, 'download' # or you can do the same on multiple lines if you wish Given path 'documents' And path documentId And path 'download' + +# you can also ensure that the constructed url has a trailing / by appending an empty path segment +# eg. given a documentId of 1234 the path will be: /documents/1234/download/ +Given path 'documents', documentId, 'download', '' ``` Note that the `path` 'resets' after any HTTP request is made but not the `url`. The [Hello World](#hello-world) is a great example of 'REST-ful' use of the `url` when the test focuses on a single REST 'resource'. Look at how the `path` did not need to be specified for the second HTTP `get` call since `/cats` is part of the `url`. @@ -1820,7 +1826,7 @@ Use this for multipart content items that don't have field-names. Here below is also demonstrates using the [`multipart/related`](https://tools.ietf.org/html/rfc2387) content-type. ```cucumber -Given path '/v2/documents' +Given path 'v2', 'documents' And multipart entity read('foo.json') And multipart field image = read('bar.jpg') And header Content-Type = 'multipart/related' diff --git a/karate-core/src/main/java/com/intuit/karate/http/HttpRequestBuilder.java b/karate-core/src/main/java/com/intuit/karate/http/HttpRequestBuilder.java index 5f77a769a..6a9697f89 100644 --- a/karate-core/src/main/java/com/intuit/karate/http/HttpRequestBuilder.java +++ b/karate-core/src/main/java/com/intuit/karate/http/HttpRequestBuilder.java @@ -93,7 +93,6 @@ public class HttpRequestBuilder implements ProxyObject { private String method; private List paths; private Map> params; - private String fragment; private Map> headers; private MultiPartBuilder multiPart; private Object body; @@ -241,11 +240,6 @@ public HttpRequestBuilder method(String method) { return this; } - public HttpRequestBuilder fragment(String fragment) { - this.fragment = fragment; - return this; - } - public HttpRequestBuilder paths(String... paths) { for (String path : paths) { path(path); @@ -264,36 +258,26 @@ public HttpRequestBuilder path(String path) { return this; } - private List backwardsCompatiblePaths() { - if (paths == null) { - return Collections.emptyList(); - } - - List result = new ArrayList<>(paths.size()); - for (int i = 0; i < paths.size(); i++) { - String path = paths.get(i); - if (i == 0 && path.startsWith("/")) { - path = path.substring(1); - logger.warn("the first path segment starts with a '/', this will be stripped off for now, but in the future this may be escaped and cause your request to fail."); - } - result.add(path); - } - return result; - } - private URI getUri() { try { URIBuilder builder = url == null ? new URIBuilder() : new URIBuilder(url); if (params != null) { params.forEach((key, values) -> values.forEach(value -> builder.addParameter(key, value))); } - // merge paths from the base url with additional paths supplied to this builder - List merged = Stream.of(builder.getPathSegments(), backwardsCompatiblePaths()) - .flatMap(List::stream) - .collect(Collectors.toList()); - return builder.setPathSegments(merged) - .setFragment(fragment) - .build(); + if (paths != null) { + paths.forEach(path -> { + if (path.startsWith("/")) { + logger.warn("Path segment: '{}' starts with a '/', this is probably a mistake. The '/' character will be escaped and sent to the remote server as '%2F'. " + + "If you want to include multiple paths please separate them using commas. Ie. 'hello', 'world' instead of '/hello/world'.", path); + } + }); + // merge paths from the supplied url with additional paths supplied to this builder + List merged = Stream.of(builder.getPathSegments(), paths) + .flatMap(List::stream) + .collect(Collectors.toList()); + builder.setPathSegments(merged); + } + return builder.build(); } catch (URISyntaxException e) { throw new RuntimeException(e); } diff --git a/karate-core/src/test/java/com/intuit/karate/core/KarateHttpMockHandlerTest.java b/karate-core/src/test/java/com/intuit/karate/core/KarateHttpMockHandlerTest.java index f17022426..2d6556218 100644 --- a/karate-core/src/test/java/com/intuit/karate/core/KarateHttpMockHandlerTest.java +++ b/karate-core/src/test/java/com/intuit/karate/core/KarateHttpMockHandlerTest.java @@ -68,7 +68,7 @@ void testSimpleGet() { startMockServer(); run( urlStep(), - "path '/hello'", + "path 'hello'", "method get" ); matchVar("response", "hello world"); @@ -82,7 +82,7 @@ void testThatCookieIsPartOfRequest() { startMockServer(); run( urlStep(), - "path '/hello'", + "path 'hello'", "cookie foo = 'bar'", "method get" ); @@ -97,7 +97,7 @@ void testSameSiteSecureCookieRequest() { startMockServer(); run( urlStep(), - "path '/hello'", + "path 'hello'", "cookie foo = { value: 'bar', samesite: 'Strict', secure: true }", "method get" ); @@ -112,7 +112,7 @@ void testSameSiteSecureCookieResponse() { startMockServer(); run( urlStep(), - "path '/hello'", + "path 'hello'", "method get" ); matchVarContains("responseHeaders", "{ set-cookie: ['foo=bar; expires=Wed, 30-Dec-20 09:25:45 GMT; path=/; domain=.example.com; HttpOnly; SameSite=Lax; Secure'] }"); @@ -126,7 +126,7 @@ void testThatExoticContentTypeIsPreserved() { startMockServer(); run( urlStep(), - "path '/hello'", + "path 'hello'", "header Content-Type = 'application/xxx.pingixxxxxx.checkUsernamePassword+json'", "method post" ); @@ -142,7 +142,7 @@ void testInspectRequestInHeadersFunction() { run( urlStep(), "configure headers = function(request){ return { 'api-key': request.bodyAsString } }", - "path '/hello'", + "path 'hello'", "request 'some text'", "method post" ); @@ -159,7 +159,7 @@ void testKarateRemove() { startMockServer(); run( urlStep(), - "path '/hello', '1'", + "path 'hello', '1'", "method get" ); matchVarContains("response", "{ '2': 'bar' }"); @@ -173,7 +173,7 @@ void testTransferEncoding() { startMockServer(); run( urlStep(), - "path '/hello'", + "path 'hello'", "header Transfer-Encoding = 'chunked'", "request { foo: 'bar' }", "method post" @@ -189,7 +189,7 @@ void testMalformedMockResponse() { startMockServer(); run( urlStep(), - "path '/hello'", + "path 'hello'", "method get", "match response == '{ \"id\" \"123\" }'", "match responseType == 'string'" @@ -210,7 +210,7 @@ void testRedirectAfterPostWithCookie() { startMockServer(); run( urlStep(), - "path '/first'", + "path 'first'", "form fields { username: 'blah', password: 'blah' }", "method post" ); diff --git a/karate-core/src/test/java/com/intuit/karate/core/KarateMockHandlerTest.java b/karate-core/src/test/java/com/intuit/karate/core/KarateMockHandlerTest.java index 55ec32084..a2fcc34a9 100644 --- a/karate-core/src/test/java/com/intuit/karate/core/KarateMockHandlerTest.java +++ b/karate-core/src/test/java/com/intuit/karate/core/KarateMockHandlerTest.java @@ -52,7 +52,7 @@ void testSimpleGet() { "def response = 'hello world'"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "method get" ); matchVar("response", "hello world"); @@ -65,7 +65,7 @@ void testSimplePost() { "def response = requestHeaders"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "request { foo: 'bar' }", "method post" ); @@ -94,7 +94,7 @@ void testParam() { run( URL_STEP, "param foo = 'bar'", - "path '/hello'", + "path 'hello'", "method get" ); matchVar("response", "{ foo: ['bar'] }"); @@ -108,7 +108,7 @@ void testParams() { run( URL_STEP, "params { foo: 'bar' }", - "path '/hello'", + "path 'hello'", "method get" ); matchVar("response", "{ foo: ['bar'] }"); @@ -122,7 +122,7 @@ void testParamWithEmbeddedCommas() { run( URL_STEP, "param foo = 'bar,baz'", - "path '/hello'", + "path 'hello'", "method get" ); matchVar("response", "{ foo: ['bar,baz'] }"); @@ -136,7 +136,7 @@ void testParamMultiValue() { run( URL_STEP, "param foo = ['bar', 'baz']", - "path '/hello'", + "path 'hello'", "method get" ); matchVar("response", "{ foo: ['bar', 'baz'] }"); @@ -149,7 +149,7 @@ void testHeaders() { "def response = requestHeaders"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "header foo = 'bar'", "method get" ); @@ -163,7 +163,7 @@ void testHeaderMultiValue() { "def response = requestHeaders"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "def fun = function(arg){ return [arg.first, arg.second] }", "header Authorization = call fun { first: 'foo', second: 'bar' }", "method get" @@ -178,7 +178,7 @@ void testRequestContentTypeForJson() { "def response = requestHeaders"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "request { foo: 'bar' }", "method post" ); @@ -193,7 +193,7 @@ void testResponseContentTypeForJson() { "def response = '{ \"foo\": \"bar\"}'"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "method get", "match responseHeaders == { 'Content-Type': ['application/json'] }", "match header content-type == 'application/json'", @@ -209,7 +209,7 @@ void testCookie() { run( URL_STEP, "cookie foo = 'bar'", - "path '/hello'", + "path 'hello'", "method get" ); matchVar("response", "{ Cookie: ['foo=bar'] }"); @@ -226,7 +226,7 @@ void testCookieWithDateInThePast() { run( URL_STEP, "cookie foo = {value:'bar', expires: '" + pastDate + "'}", - "path '/hello'", + "path 'hello'", "method get" ); matchVar("response", "{ Cookie: ['foo=bar'] }"); @@ -243,7 +243,7 @@ void testCookieWithDateInTheFuture() { run( URL_STEP, "cookie foo = { value: 'bar', expires: '" + futureDate + "' }", - "path '/hello'", + "path 'hello'", "method get" ); matchVar("response", "{ Cookie: ['foo=bar'] }"); @@ -257,7 +257,7 @@ void testCookieWithMaxAgeZero() { run( URL_STEP, "cookie foo = { value: 'bar', max-age: '0' }", - "path '/hello'", + "path 'hello'", "method get" ); matchVar("response", "{ Cookie: ['#string'] }"); @@ -271,7 +271,7 @@ void testFormFieldGet() { run( URL_STEP, "form field foo = 'bar'", - "path '/hello'", + "path 'hello'", "method get" ); matchVar("response", "{ foo: ['bar'] }"); @@ -285,7 +285,7 @@ void testFormFieldPost() { run( URL_STEP, "form field foo = 'bar'", - "path '/hello'", + "path 'hello'", "method post" ); matchVar("response", "foo=bar"); @@ -299,7 +299,7 @@ void testMultiPartField() { run( URL_STEP, "multipart field foo = 'bar'", - "path '/hello'", + "path 'hello'", "method post" ); matchVar("response", "{ foo: ['bar'] }"); @@ -313,7 +313,7 @@ void testMultiPartFile() { run( URL_STEP, "multipart file foo = { filename: 'foo.txt', value: 'hello' }", - "path '/hello'", + "path 'hello'", "method post" ); matchVar("response", "{ foo: [{ name: 'foo', value: '#notnull', contentType: 'text/plain', charset: 'UTF-8', filename: 'foo.txt', transferEncoding: '7bit' }] }"); @@ -327,7 +327,7 @@ void testConfigureResponseHeaders() { "def response = ''"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "method get" ); matchVar("responseHeaders", "{ 'Content-Type': ['text/html'] }"); @@ -342,7 +342,7 @@ void testConfigureLowerCaseResponseHeaders() { run( "configure lowerCaseResponseHeaders = true", URL_STEP, - "path '/hello'", + "path 'hello'", "method get" ); matchVar("responseHeaders", "{ 'content-type': ['text/html'] }"); @@ -356,7 +356,7 @@ void testResponseContentTypeForXml() { "def response = 'world'"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "method get", "match header content-type == 'application/xml'", "match responseType == 'xml'", @@ -371,7 +371,7 @@ void testResponseAutoConversionForXmlAsPlainText() { "def response = 'world'"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "method get", "match header content-type == 'text/plain'", "match responseType == 'xml'", @@ -386,7 +386,7 @@ void testResponseAutoConversionForJsonAsPlainText() { "def response = '{ \"foo\": \"bar\"}'"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "method get", "match header content-type == 'text/plain'", "match responseType == 'json'", @@ -401,7 +401,7 @@ void testResponseAutoConversionForTextWithTags() { "def response = ' a .'"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "method get", "match header content-type == 'text/plain'", "match responseType == 'string'", @@ -416,7 +416,7 @@ void testResponseContentTypeForNonXmlWithTags() { "def response = ' a .'"); run( URL_STEP, - "path '/hello'", + "path 'hello'", "method get", "match header content-type == 'text/turtle'", "match responseType == 'string'", @@ -431,7 +431,7 @@ void testWildcardLikePathMatch() { "def response = requestUri"); run( URL_STEP, - "path '/hello/foo/bar'", + "path 'hello', 'foo', 'bar'", "method get", "match response == 'hello/foo/bar'" ); diff --git a/karate-core/src/test/java/com/intuit/karate/http/RequestHandlerTest.java b/karate-core/src/test/java/com/intuit/karate/http/RequestHandlerTest.java index 8d62720bf..5e9ecfbb5 100644 --- a/karate-core/src/test/java/com/intuit/karate/http/RequestHandlerTest.java +++ b/karate-core/src/test/java/com/intuit/karate/http/RequestHandlerTest.java @@ -52,7 +52,7 @@ private void matchHeaderContains(String name, String expected) { @Test void testIndexAndAjaxPost() { - request.path("/index"); + request.path("index"); handle(); matchHeaderContains("Set-Cookie", "karate.sid"); matchHeaderEquals("Content-Type", "text/html"); @@ -61,7 +61,7 @@ void testIndexAndAjaxPost() { assertTrue(body.contains("Apple")); assertTrue(body.contains("Orange")); assertTrue(body.contains("Billie")); - request.path("/person") + request.path("person") .contentType("application/x-www-form-urlencoded") .header("HX-Request", "true") .body("firstName=John&lastName=Smith&email=john%40smith.com") diff --git a/karate-demo/src/test/java/demo/DemoTestParallel.java b/karate-demo/src/test/java/demo/DemoTestParallel.java index 8b79706cb..ceda5c8c4 100644 --- a/karate-demo/src/test/java/demo/DemoTestParallel.java +++ b/karate-demo/src/test/java/demo/DemoTestParallel.java @@ -37,7 +37,7 @@ public void testParallel() { public static void generateReport(String karateOutputPath) { Collection jsonFiles = FileUtils.listFiles(new File(karateOutputPath), new String[] {"json"}, true); - List jsonPaths = new ArrayList(jsonFiles.size()); + List jsonPaths = new ArrayList<>(jsonFiles.size()); jsonFiles.forEach(file -> jsonPaths.add(file.getAbsolutePath())); Configuration config = new Configuration(new File("target"), "demo"); ReportBuilder reportBuilder = new ReportBuilder(jsonPaths, config); diff --git a/karate-demo/src/test/java/demo/encoding/encoding.feature b/karate-demo/src/test/java/demo/encoding/encoding.feature index 0ff2b92f8..2f97c70fe 100644 --- a/karate-demo/src/test/java/demo/encoding/encoding.feature +++ b/karate-demo/src/test/java/demo/encoding/encoding.feature @@ -35,13 +35,6 @@ Scenario: path escapes special characters Then status 200 And match response == '"<>#{}|\^[]`' -Scenario: leading / in first path is tolerated, but will issue a warning - Given url demoBaseUrl + '/' - And path '/encoding', 'hello' - When method get - Then status 200 - And match response == 'hello' - Scenario: leading / in path is not required Given url demoBaseUrl And path 'encoding', 'hello' @@ -50,7 +43,7 @@ Scenario: leading / in path is not required And match response == 'hello' Scenario: manually decode before passing to karate - * def encoded = '%2Ffoo%2Bbar' + * def encoded = 'foo%2Bbar' * def decoded = java.net.URLDecoder.decode(encoded, 'UTF-8') Given url demoBaseUrl + '/encoding' And path decoded diff --git a/karate-demo/src/test/java/demo/error/no-url.feature b/karate-demo/src/test/java/demo/error/no-url.feature index 408586e65..e787adaec 100644 --- a/karate-demo/src/test/java/demo/error/no-url.feature +++ b/karate-demo/src/test/java/demo/error/no-url.feature @@ -1,11 +1,11 @@ -Feature: No URLfound proper error response +Feature: No URL found proper error response Background: * url demoBaseUrl * configure lowerCaseResponseHeaders = true Scenario: Invalid URL response - Given path '/hello' + Given path 'hello' When method get Then status 404 And match header content-type contains 'application/json'