diff --git a/client-runtime/src/main/java/com/microsoft/rest/v2/SwaggerMethodParser.java b/client-runtime/src/main/java/com/microsoft/rest/v2/SwaggerMethodParser.java index 6fd010bb4d57..5691958f65ed 100644 --- a/client-runtime/src/main/java/com/microsoft/rest/v2/SwaggerMethodParser.java +++ b/client-runtime/src/main/java/com/microsoft/rest/v2/SwaggerMethodParser.java @@ -6,6 +6,8 @@ package com.microsoft.rest.v2; +import com.google.common.escape.Escaper; +import com.google.common.net.UrlEscapers; import com.microsoft.rest.v2.annotations.BodyParam; import com.microsoft.rest.v2.annotations.DELETE; import com.microsoft.rest.v2.annotations.GET; @@ -21,11 +23,9 @@ import com.microsoft.rest.v2.http.HttpHeader; import com.microsoft.rest.v2.http.HttpHeaders; -import java.io.UnsupportedEncodingException; import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.lang.reflect.Type; -import java.net.URLEncoder; import java.util.ArrayList; import java.util.List; @@ -148,7 +148,7 @@ public String httpMethod() { * @return The final host to use for HTTP requests for this Swagger method. */ public String scheme(Object[] swaggerMethodArguments) { - final String substitutedHost = applySubstitutions(rawHost, hostSubstitutions, swaggerMethodArguments); + final String substitutedHost = applySubstitutions(rawHost, hostSubstitutions, swaggerMethodArguments, UrlEscapers.urlPathSegmentEscaper()); return substitutedHost.split("://")[0]; } @@ -158,7 +158,7 @@ public String scheme(Object[] swaggerMethodArguments) { * @return The final host to use for HTTP requests for this Swagger method. */ public String host(Object[] swaggerMethodArguments) { - final String substitutedHost = applySubstitutions(rawHost, hostSubstitutions, swaggerMethodArguments); + final String substitutedHost = applySubstitutions(rawHost, hostSubstitutions, swaggerMethodArguments, UrlEscapers.urlPathSegmentEscaper()); return substitutedHost.split("://")[1]; } @@ -168,7 +168,7 @@ public String host(Object[] swaggerMethodArguments) { * @return The path value with its placeholders replaced by the matching substitutions. */ public String path(Object[] methodArguments) { - return applySubstitutions(relativePath, pathSubstitutions, methodArguments); + return applySubstitutions(relativePath, pathSubstitutions, methodArguments, UrlEscapers.urlPathSegmentEscaper()); } /** @@ -179,7 +179,7 @@ public String path(Object[] methodArguments) { * @return An Iterable with the encoded query parameters. */ public Iterable encodedQueryParameters(Object[] swaggerMethodArguments) { - return getEncodedParameters(querySubstitutions, swaggerMethodArguments); + return getEncodedParameters(querySubstitutions, swaggerMethodArguments, UrlEscapers.urlFormParameterEscaper()); } /** @@ -190,7 +190,7 @@ public Iterable encodedQueryParameters(Object[] swaggerMethodA public Iterable headers(Object[] swaggerMethodArguments) { final HttpHeaders result = new HttpHeaders(headers); - final Iterable substitutedHeaders = getEncodedParameters(headerSubstitutions, swaggerMethodArguments); + final Iterable substitutedHeaders = getEncodedParameters(headerSubstitutions, swaggerMethodArguments, null); for (final EncodedParameter substitutedHeader : substitutedHeaders) { result.add(substitutedHeader.name(), substitutedHeader.encodedValue()); } @@ -235,7 +235,7 @@ private void setHttpMethodAndRelativePath(String httpMethod, String relativePath this.relativePath = relativePath; } - private static String applySubstitutions(String originalValue, Iterable substitutions, Object[] methodArguments) { + private static String applySubstitutions(String originalValue, Iterable substitutions, Object[] methodArguments, Escaper escaper) { String result = originalValue; if (methodArguments != null) { @@ -246,7 +246,7 @@ private static String applySubstitutions(String originalValue, Iterable getEncodedParameters(Iterable substitutions, Object[] methodArguments) { + private static Iterable getEncodedParameters(Iterable substitutions, Object[] methodArguments, Escaper escaper) { final List result = new ArrayList<>(); if (substitutions != null) { @@ -267,8 +267,8 @@ private static Iterable getEncodedParameters(Iterable getEncodedParameters(Iterable getAnythingAsync(); } @Test - public void SyncGetRequestWithAnythingReturn() { + public void SyncGetRequestWithAnything() { final HttpBinJSON json = createService(Service5.class) .getAnything(); assertNotNull(json); @@ -153,7 +163,63 @@ public void SyncGetRequestWithAnythingReturn() { } @Test - public void AsyncGetRequestWithAnythingReturn() { + public void SyncGetRequestWithAnythingWithPlus() { + final HttpBinJSON json = createService(Service5.class) + .getAnythingWithPlus(); + assertNotNull(json); + assertEquals("http://httpbin.org/anything/with+plus", json.url); + } + + @Test + public void SyncGetRequestWithAnythingWithPathParam() { + final HttpBinJSON json = createService(Service5.class) + .getAnythingWithPathParam("withpathparam"); + assertNotNull(json); + assertEquals("http://httpbin.org/anything/withpathparam", json.url); + } + + @Test + public void SyncGetRequestWithAnythingWithPathParamWithSpace() { + final HttpBinJSON json = createService(Service5.class) + .getAnythingWithPathParam("with path param"); + assertNotNull(json); + assertEquals("http://httpbin.org/anything/with path param", json.url); + } + + @Test + public void SyncGetRequestWithAnythingWithPathParamWithPlus() { + final HttpBinJSON json = createService(Service5.class) + .getAnythingWithPathParam("with+path+param"); + assertNotNull(json); + assertEquals("http://httpbin.org/anything/with+path+param", json.url); + } + + @Test + public void SyncGetRequestWithAnythingWithEncodedPathParam() { + final HttpBinJSON json = createService(Service5.class) + .getAnythingWithEncodedPathParam("withpathparam"); + assertNotNull(json); + assertEquals("http://httpbin.org/anything/withpathparam", json.url); + } + + @Test + public void SyncGetRequestWithAnythingWithEncodedPathParamWithPercent20() { + final HttpBinJSON json = createService(Service5.class) + .getAnythingWithEncodedPathParam("with%20path%20param"); + assertNotNull(json); + assertEquals("http://httpbin.org/anything/with path param", json.url); + } + + @Test + public void SyncGetRequestWithAnythingWithEncodedPathParamWithPlus() { + final HttpBinJSON json = createService(Service5.class) + .getAnythingWithEncodedPathParam("with+path+param"); + assertNotNull(json); + assertEquals("http://httpbin.org/anything/with+path+param", json.url); + } + + @Test + public void AsyncGetRequestWithAnything() { final HttpBinJSON json = createService(Service5.class) .getAnythingAsync() .toBlocking().value(); @@ -166,12 +232,15 @@ private interface Service6 { @GET("anything") HttpBinJSON getAnything(@QueryParam("a") String a, @QueryParam("b") int b); + @GET("anything") + HttpBinJSON getAnythingWithEncoded(@QueryParam(value="a", encoded=true) String a, @QueryParam("b") int b); + @GET("anything") Single getAnythingAsync(@QueryParam("a") String a, @QueryParam("b") int b); } @Test - public void SyncGetRequestWithQueryParametersAndAnythingReturn() { + public void SyncGetRequestWithQueryParametersAndAnything() { final HttpBinJSON json = createService(Service6.class) .getAnything("A", 15); assertNotNull(json); @@ -179,7 +248,23 @@ public void SyncGetRequestWithQueryParametersAndAnythingReturn() { } @Test - public void AsyncGetRequestWithQueryParametersAndAnythingReturn() { + public void SyncGetRequestWithQueryParametersAndAnythingWithPercent20() { + final HttpBinJSON json = createService(Service6.class) + .getAnything("A%20Z", 15); + assertNotNull(json); + assertEquals("http://httpbin.org/anything?a=A%2520Z&b=15", json.url); + } + + @Test + public void SyncGetRequestWithQueryParametersAndAnythingWithEncodedWithPercent20() { + final HttpBinJSON json = createService(Service6.class) + .getAnythingWithEncoded("x%20y", 15); + assertNotNull(json); + assertEquals("http://httpbin.org/anything?a=x y&b=15", json.url); + } + + @Test + public void AsyncGetRequestWithQueryParametersAndAnything() { final HttpBinJSON json = createService(Service6.class) .getAnythingAsync("A", 15) .toBlocking().value(); @@ -211,7 +296,7 @@ public void SyncGetRequestWithHeaderParametersAndAnythingReturn() { } @Test - public void AsyncGetRequestWithHeaderParametersAndAnythingReturn() { + public void AsyncGetRequestWithHeaderParametersAndAnything() { final HttpBinJSON json = createService(Service7.class) .getAnythingAsync("A", 15) .toBlocking().value(); diff --git a/client-runtime/src/test/java/com/microsoft/rest/v2/SwaggerMethodParserTests.java b/client-runtime/src/test/java/com/microsoft/rest/v2/SwaggerMethodParserTests.java index 898ff9b858ac..b2e5d3adc304 100644 --- a/client-runtime/src/test/java/com/microsoft/rest/v2/SwaggerMethodParserTests.java +++ b/client-runtime/src/test/java/com/microsoft/rest/v2/SwaggerMethodParserTests.java @@ -26,14 +26,4 @@ public void withNoAnnotations() { assertEquals("https", methodParser.scheme(null)); assertEquals("raw.host.com", methodParser.host(null)); } - - @Test - public void encodeWithBadEncoding() { - assertEquals(" ", SwaggerMethodParser.encode(" ", "myBadEncoding")); - } - - @Test - public void encodeWithDefaultEncoding() { - assertEquals("+", SwaggerMethodParser.encode(" ")); - } } diff --git a/client-runtime/src/test/java/com/microsoft/rest/v2/http/MockHttpClient.java b/client-runtime/src/test/java/com/microsoft/rest/v2/http/MockHttpClient.java index 6eeae9b9fbd5..512fe576ab4c 100644 --- a/client-runtime/src/test/java/com/microsoft/rest/v2/http/MockHttpClient.java +++ b/client-runtime/src/test/java/com/microsoft/rest/v2/http/MockHttpClient.java @@ -30,39 +30,42 @@ public Single sendRequestAsync(HttpRequest request) { final String requestHost = requestUrl.getHost(); if (requestHost.equalsIgnoreCase("httpbin.org")) { final String requestPath = requestUrl.getPath(); - if (requestPath.equalsIgnoreCase("/anything")) { + final String requestPathLower = requestPath.toLowerCase(); + if (requestPathLower.equals("/anything") || requestPathLower.startsWith("/anything/")) { final HttpBinJSON json = new HttpBinJSON(); - json.url = request.url(); + json.url = request.url() + // This is just to mimic the behavior we've seen with httpbin.org. + .replace("%20", " "); json.headers = toMap(request.headers()); response = new MockHttpResponse(json); } - else if (requestPath.startsWith("/bytes/")) { + else if (requestPathLower.startsWith("/bytes/")) { final String byteCountString = requestPath.substring("/bytes/".length()); final int byteCount = Integer.parseInt(byteCountString); response = new MockHttpResponse(new byte[byteCount]); } - else if (requestPath.equalsIgnoreCase("/delete")) { + else if (requestPathLower.equals("/delete")) { final HttpBinJSON json = new HttpBinJSON(); json.data = bodyToString(request); response = new MockHttpResponse(json); } - else if (requestPath.equalsIgnoreCase("/get")) { + else if (requestPathLower.equals("/get")) { final HttpBinJSON json = new HttpBinJSON(); json.url = request.url(); json.headers = toMap(request.headers()); response = new MockHttpResponse(json); } - else if (requestPath.equalsIgnoreCase("/patch")) { + else if (requestPathLower.equals("/patch")) { final HttpBinJSON json = new HttpBinJSON(); json.data = bodyToString(request); response = new MockHttpResponse(json); } - else if (requestPath.equalsIgnoreCase("/post")) { + else if (requestPathLower.equals("/post")) { final HttpBinJSON json = new HttpBinJSON(); json.data = bodyToString(request); response = new MockHttpResponse(json); } - else if (requestPath.equalsIgnoreCase("/put")) { + else if (requestPathLower.equals("/put")) { final HttpBinJSON json = new HttpBinJSON(); json.data = bodyToString(request); response = new MockHttpResponse(json); diff --git a/client-runtime/src/test/java/com/microsoft/rest/v2/http/UrlBuilderTests.java b/client-runtime/src/test/java/com/microsoft/rest/v2/http/UrlBuilderTests.java index 591c55f277a8..2edcfed0e3e0 100644 --- a/client-runtime/src/test/java/com/microsoft/rest/v2/http/UrlBuilderTests.java +++ b/client-runtime/src/test/java/com/microsoft/rest/v2/http/UrlBuilderTests.java @@ -9,7 +9,7 @@ public class UrlBuilderTests { public void withScheme() { final UrlBuilder builder = new UrlBuilder() .withScheme("http"); - assertEquals(null, builder.toString()); + assertEquals("http://", builder.toString()); } @Test @@ -20,11 +20,82 @@ public void withSchemeAndHost() { assertEquals("http://www.example.com", builder.toString()); } + @Test + public void withSchemeAndHostWhenHostHasWhitespace() { + final UrlBuilder builder = new UrlBuilder() + .withScheme("http") + .withHost("www.exa mple.com"); + assertEquals("http://www.exa mple.com", builder.toString()); + } + @Test public void withHost() { final UrlBuilder builder = new UrlBuilder() .withHost("www.example.com"); - assertEquals("//www.example.com", builder.toString()); + assertEquals("www.example.com", builder.toString()); + } + + @Test + public void withHostWhenHostHasWhitespace() { + final UrlBuilder builder = new UrlBuilder() + .withHost("www.exampl e.com"); + assertEquals("www.exampl e.com", builder.toString()); + } + + @Test + public void withHostAndPath() { + final UrlBuilder builder = new UrlBuilder() + .withHost("www.example.com") + .withPath("my/path"); + assertEquals("www.example.com/my/path", builder.toString()); + } + + @Test + public void withHostAndPathWithSlashAfterHost() { + final UrlBuilder builder = new UrlBuilder() + .withHost("www.example.com/") + .withPath("my/path"); + assertEquals("www.example.com/my/path", builder.toString()); + } + + @Test + public void withHostAndPathWithSlashBeforePath() { + final UrlBuilder builder = new UrlBuilder() + .withHost("www.example.com") + .withPath("/my/path"); + assertEquals("www.example.com/my/path", builder.toString()); + } + + @Test + public void withHostAndPathWithSlashAfterHostAndBeforePath() { + final UrlBuilder builder = new UrlBuilder() + .withHost("www.example.com/") + .withPath("/my/path"); + assertEquals("www.example.com/my/path", builder.toString()); + } + + @Test + public void withHostAndPathWithWhitespaceInPath() { + final UrlBuilder builder = new UrlBuilder() + .withHost("www.example.com") + .withPath("my path"); + assertEquals("www.example.com/my path", builder.toString()); + } + + @Test + public void withHostAndPathWithPlusInPath() { + final UrlBuilder builder = new UrlBuilder() + .withHost("www.example.com") + .withPath("my+path"); + assertEquals("www.example.com/my+path", builder.toString()); + } + + @Test + public void withHostAndPathWithPercent20InPath() { + final UrlBuilder builder = new UrlBuilder() + .withHost("www.example.com") + .withPath("my%20path"); + assertEquals("www.example.com/my%20path", builder.toString()); } @Test @@ -36,6 +107,42 @@ public void withSchemeAndHostAndOneQueryParameter() { assertEquals("http://www.example.com?A=B", builder.toString()); } + @Test + public void withSchemeAndHostAndOneQueryParameterWhenQueryParameterNameHasWhitespace() { + final UrlBuilder builder = new UrlBuilder() + .withScheme("http") + .withHost("www.example.com") + .withQueryParameter("App les", "B"); + assertEquals("http://www.example.com?App les=B", builder.toString()); + } + + @Test + public void withSchemeAndHostAndOneQueryParameterWhenQueryParameterNameHasPercent20() { + final UrlBuilder builder = new UrlBuilder() + .withScheme("http") + .withHost("www.example.com") + .withQueryParameter("App%20les", "B"); + assertEquals("http://www.example.com?App%20les=B", builder.toString()); + } + + @Test + public void withSchemeAndHostAndOneQueryParameterWhenQueryParameterValueHasWhitespace() { + final UrlBuilder builder = new UrlBuilder() + .withScheme("http") + .withHost("www.example.com") + .withQueryParameter("Apples", "Go od"); + assertEquals("http://www.example.com?Apples=Go od", builder.toString()); + } + + @Test + public void withSchemeAndHostAndOneQueryParameterWhenQueryParameterValueHasPercent20() { + final UrlBuilder builder = new UrlBuilder() + .withScheme("http") + .withHost("www.example.com") + .withQueryParameter("Apples", "Go%20od"); + assertEquals("http://www.example.com?Apples=Go%20od", builder.toString()); + } + @Test public void withSchemeAndHostAndTwoQueryParameters() { final UrlBuilder builder = new UrlBuilder()