Skip to content

Commit

Permalink
Merge pull request Azure#212 from daschult/URLEncoding
Browse files Browse the repository at this point in the history 
Change UrlBuilder to not encode on toString()
  • Loading branch information
Dan Schulte authored Sep 8, 2017
2 parents 05d7a41 + 14d5a09 commit 8b19798
Show file tree
Hide file tree
Showing 6 changed files with 247 additions and 69 deletions.
46 changes: 12 additions & 34 deletions client-runtime/src/main/java/com/microsoft/rest/v2/SwaggerMethodParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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];
}

Expand All @@ -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];
}

Expand All @@ -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());
}

/**
Expand All @@ -179,7 +179,7 @@ public String path(Object[] methodArguments) {
* @return An Iterable with the encoded query parameters.
*/
public Iterable<EncodedParameter> encodedQueryParameters(Object[] swaggerMethodArguments) {
return getEncodedParameters(querySubstitutions, swaggerMethodArguments);
return getEncodedParameters(querySubstitutions, swaggerMethodArguments, UrlEscapers.urlFormParameterEscaper());
}

/**
Expand All @@ -190,7 +190,7 @@ public Iterable<EncodedParameter> encodedQueryParameters(Object[] swaggerMethodA
public Iterable<HttpHeader> headers(Object[] swaggerMethodArguments) {
final HttpHeaders result = new HttpHeaders(headers);

final Iterable<EncodedParameter> substitutedHeaders = getEncodedParameters(headerSubstitutions, swaggerMethodArguments);
final Iterable<EncodedParameter> substitutedHeaders = getEncodedParameters(headerSubstitutions, swaggerMethodArguments, null);
for (final EncodedParameter substitutedHeader : substitutedHeaders) {
result.add(substitutedHeader.name(), substitutedHeader.encodedValue());
}
Expand Down Expand Up @@ -235,7 +235,7 @@ private void setHttpMethodAndRelativePath(String httpMethod, String relativePath
this.relativePath = relativePath;
}

private static String applySubstitutions(String originalValue, Iterable<Substitution> substitutions, Object[] methodArguments) {
private static String applySubstitutions(String originalValue, Iterable<Substitution> substitutions, Object[] methodArguments, Escaper escaper) {
String result = originalValue;

if (methodArguments != null) {
Expand All @@ -246,7 +246,7 @@ private static String applySubstitutions(String originalValue, Iterable<Substitu

String substitutionValue = String.valueOf(methodArgument);
if (substitution.shouldEncode()) {
substitutionValue = encode(substitutionValue);
substitutionValue = escaper.escape(substitutionValue);
}

result = result.replace("{" + substitution.urlParameterName() + "}", substitutionValue);
Expand All @@ -257,7 +257,7 @@ private static String applySubstitutions(String originalValue, Iterable<Substitu
return result;
}

private static Iterable<EncodedParameter> getEncodedParameters(Iterable<Substitution> substitutions, Object[] methodArguments) {
private static Iterable<EncodedParameter> getEncodedParameters(Iterable<Substitution> substitutions, Object[] methodArguments, Escaper escaper) {
final List<EncodedParameter> result = new ArrayList<>();

if (substitutions != null) {
Expand All @@ -267,8 +267,8 @@ private static Iterable<EncodedParameter> getEncodedParameters(Iterable<Substitu
final Object methodArgument = methodArguments[substitution.methodParameterIndex()];

String parameterValue = String.valueOf(methodArgument);
if (substitution.shouldEncode()) {
parameterValue = encode(parameterValue);
if (substitution.shouldEncode() && escaper != null) {
parameterValue = escaper.escape(parameterValue);
}

result.add(new EncodedParameter(substitution.urlParameterName(), parameterValue));
Expand All @@ -278,26 +278,4 @@ private static Iterable<EncodedParameter> getEncodedParameters(Iterable<Substitu

return result;
}

/**
* URL encode the provided value using the default (UTF-8) encoding.
* @param segment The value to URL encode.
* @return The encoded value.
*/
protected static String encode(String segment) {
return encode(segment, "UTF-8");
}

/**
* URL encode the provided value using the provided encoding.
* @param segment The value to URL encode.
* @return The encoded value.
*/
protected static String encode(String segment, String encoding) {
try {
return URLEncoder.encode(segment, encoding);
} catch (UnsupportedEncodingException e) {
return segment;
}
}
}
35 changes: 25 additions & 10 deletions client-runtime/src/main/java/com/microsoft/rest/v2/http/UrlBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@

package com.microsoft.rest.v2.http;

import java.net.URI;
import java.net.URISyntaxException;

/**
* A builder class that is used to create URLs.
*/
Expand All @@ -34,6 +31,9 @@ public UrlBuilder withScheme(String scheme) {
* @return This UrlBuilder so that multiple setters can be chained together.
*/
public UrlBuilder withHost(String host) {
if (host != null && host.endsWith("/")) {
host = host.substring(0, host.length() - 1);
}
this.host = host;
return this;
}
Expand All @@ -60,7 +60,7 @@ public UrlBuilder withPath(String path) {
*/
public UrlBuilder withQueryParameter(String queryParameterName, String queryParameterEncodedValue) {
if (query == null) {
query = "";
query = "?";
}
else {
query += "&";
Expand All @@ -74,13 +74,28 @@ public UrlBuilder withQueryParameter(String queryParameterName, String queryPara
* @return The string representation of the URL that is being built.
*/
public String toString() {
URI uri;
try {
uri = new URI(scheme, null, host, -1, path, query, null);
final StringBuilder result = new StringBuilder();

if (scheme != null) {
result.append(scheme);

if (!scheme.endsWith("://")) {
result.append("://");
}
}

if (host != null) {
result.append(host);
}
catch (URISyntaxException e) {
uri = null;

if (path != null) {
result.append(path);
}
return uri == null ? null : uri.toString();

if (query != null) {
result.append(query);
}

return result.toString();
}
}
95 changes: 90 additions & 5 deletions client-runtime/src/test/java/com/microsoft/rest/v2/RestProxyTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.microsoft.rest.v2.http.HttpClient;
import com.microsoft.rest.v2.http.HttpHeaders;
import org.junit.Test;
import retrofit2.http.Path;
import rx.Completable;
import rx.Single;

Expand Down Expand Up @@ -140,20 +141,85 @@ private interface Service5 {
@GET("anything")
HttpBinJSON getAnything();

@GET("anything/with+plus")
HttpBinJSON getAnythingWithPlus();

@GET("anything/{path}")
HttpBinJSON getAnythingWithPathParam(@PathParam("path") String pathParam);

@GET("anything/{path}")
HttpBinJSON getAnythingWithEncodedPathParam(@PathParam(value="path", encoded=true) String pathParam);

@GET("anything")
Single<HttpBinJSON> getAnythingAsync();
}

@Test
public void SyncGetRequestWithAnythingReturn() {
public void SyncGetRequestWithAnything() {
final HttpBinJSON json = createService(Service5.class)
.getAnything();
assertNotNull(json);
assertEquals("http://httpbin.org/anything", json.url);
}

@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();
Expand All @@ -166,20 +232,39 @@ 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<HttpBinJSON> 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);
assertEquals("http://httpbin.org/anything?a=A&b=15", json.url);
}

@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();
Expand Down Expand Up @@ -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();
Expand Down
10 changes: 0 additions & 10 deletions client-runtime/src/test/java/com/microsoft/rest/v2/SwaggerMethodParserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(" "));
}
}
Loading

0 comments on commit 8b19798

Please sign in to comment.