Permalink
Browse files

SONAR-2204,SONAR-2259 Fix URL encoding

* For correct URL encoding we must encode parameters on lower level - in Query itself,
  but not in concrete implementation of Connector, because in Query we can distinguish
  concrete parts of URL.

* Moreover in this case any additional encoding routines in Connector are useless,
  so were removed.
  • Loading branch information...
1 parent 7231e77 commit dd4e31cbf0384a1114344602967dfc225b450ddb @Godin committed Mar 5, 2011
Showing with 262 additions and 242 deletions.
  1. +1 −1 sonar-ws-client/pom.xml
  2. +42 −52 sonar-ws-client/src/main/java/org/sonar/wsclient/connectors/HttpClient3Connector.java
  3. +30 −17 sonar-ws-client/src/main/java/org/sonar/wsclient/connectors/HttpClient4Connector.java
  4. +19 −4 sonar-ws-client/src/main/java/org/sonar/wsclient/services/AbstractQuery.java
  5. +7 −15 sonar-ws-client/src/main/java/org/sonar/wsclient/services/DependencyQuery.java
  6. +3 −17 sonar-ws-client/src/main/java/org/sonar/wsclient/services/DependencyTreeQuery.java
  7. +3 −1 sonar-ws-client/src/main/java/org/sonar/wsclient/services/FavouriteCreateQuery.java
  8. +1 −1 sonar-ws-client/src/main/java/org/sonar/wsclient/services/FavouriteDeleteQuery.java
  9. +1 −1 sonar-ws-client/src/main/java/org/sonar/wsclient/services/MetricQuery.java
  10. +0 −1 sonar-ws-client/src/main/java/org/sonar/wsclient/services/Plugin.java
  11. +1 −1 sonar-ws-client/src/main/java/org/sonar/wsclient/services/PropertyCreateQuery.java
  12. +1 −1 sonar-ws-client/src/main/java/org/sonar/wsclient/services/PropertyDeleteQuery.java
  13. +1 −1 sonar-ws-client/src/main/java/org/sonar/wsclient/services/PropertyQuery.java
  14. +1 −1 sonar-ws-client/src/main/java/org/sonar/wsclient/services/PropertyUpdateQuery.java
  15. +4 −5 sonar-ws-client/src/main/java/org/sonar/wsclient/services/SourceQuery.java
  16. +0 −1 sonar-ws-client/src/main/java/org/sonar/wsclient/services/UpdateCenterQuery.java
  17. +1 −1 sonar-ws-client/src/main/java/org/sonar/wsclient/services/UserPropertyDeleteQuery.java
  18. +2 −2 sonar-ws-client/src/main/java/org/sonar/wsclient/services/UserPropertyQuery.java
  19. +0 −1 sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/PluginUnmarshaller.java
  20. +16 −30 sonar-ws-client/src/test/java/org/sonar/wsclient/SonarTest.java
  21. +20 −7 sonar-ws-client/src/test/java/org/sonar/wsclient/services/AbstractQueryTest.java
  22. +3 −3 sonar-ws-client/src/test/java/org/sonar/wsclient/services/DependencyQueryTest.java
  23. +9 −15 sonar-ws-client/src/test/java/org/sonar/wsclient/services/EventQueryTest.java
  24. +3 −3 sonar-ws-client/src/test/java/org/sonar/wsclient/services/MetricQueryTest.java
  25. +4 −4 sonar-ws-client/src/test/java/org/sonar/wsclient/services/PropertyCreateQueryTest.java
  26. +4 −4 sonar-ws-client/src/test/java/org/sonar/wsclient/services/PropertyDeleteQueryTest.java
  27. +4 −4 sonar-ws-client/src/test/java/org/sonar/wsclient/services/PropertyQueryTest.java
  28. +34 −0 sonar-ws-client/src/test/java/org/sonar/wsclient/services/QueryTestCase.java
  29. +5 −5 sonar-ws-client/src/test/java/org/sonar/wsclient/services/ResourceQueryTest.java
  30. +3 −3 sonar-ws-client/src/test/java/org/sonar/wsclient/services/RuleQueryTest.java
  31. +3 −6 sonar-ws-client/src/test/java/org/sonar/wsclient/services/ServerQueryTest.java
  32. +10 −7 sonar-ws-client/src/test/java/org/sonar/wsclient/services/SourceQueryTest.java
  33. +10 −9 sonar-ws-client/src/test/java/org/sonar/wsclient/services/TimeMachineQueryTest.java
  34. +3 −4 sonar-ws-client/src/test/java/org/sonar/wsclient/services/UpdateCenterQueryTest.java
  35. +3 −3 sonar-ws-client/src/test/java/org/sonar/wsclient/services/UserPropertyCreateQueryTest.java
  36. +3 −3 sonar-ws-client/src/test/java/org/sonar/wsclient/services/UserPropertyDeleteQueryTest.java
  37. +7 −5 sonar-ws-client/src/test/java/org/sonar/wsclient/services/ViolationQueryTest.java
  38. +0 −1 sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/PluginUnmarshallerTest.java
  39. +0 −1 sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/UnmarshallerTestCase.java
  40. +0 −1 sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/UnmarshallersTest.java
@@ -87,4 +87,4 @@
<scope>test</scope>
</dependency>
</dependencies>
-</project>
+</project>
@@ -19,27 +19,35 @@
*/
package org.sonar.wsclient.connectors;
-import org.apache.commons.httpclient.*;
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.UnsupportedEncodingException;
+
+import org.apache.commons.httpclient.Credentials;
+import org.apache.commons.httpclient.HttpClient;
+import org.apache.commons.httpclient.HttpException;
+import org.apache.commons.httpclient.HttpMethod;
+import org.apache.commons.httpclient.HttpMethodBase;
+import org.apache.commons.httpclient.HttpStatus;
+import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager;
+import org.apache.commons.httpclient.UsernamePasswordCredentials;
import org.apache.commons.httpclient.auth.AuthScope;
import org.apache.commons.httpclient.methods.DeleteMethod;
+import org.apache.commons.httpclient.methods.EntityEnclosingMethod;
import org.apache.commons.httpclient.methods.GetMethod;
import org.apache.commons.httpclient.methods.PostMethod;
import org.apache.commons.httpclient.methods.PutMethod;
import org.apache.commons.httpclient.methods.StringRequestEntity;
import org.apache.commons.httpclient.params.HttpConnectionManagerParams;
-import org.apache.commons.httpclient.util.URIUtil;
import org.sonar.wsclient.Host;
+import org.sonar.wsclient.services.AbstractQuery;
import org.sonar.wsclient.services.CreateQuery;
import org.sonar.wsclient.services.DeleteQuery;
import org.sonar.wsclient.services.Query;
import org.sonar.wsclient.services.UpdateQuery;
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.io.UnsupportedEncodingException;
-
/**
* @since 2.1
*/
@@ -123,61 +131,43 @@ private String executeRequest(HttpMethodBase method) {
}
private HttpMethodBase newGetRequest(Query<?> query) {
- try {
- String url = server.getHost() + URIUtil.encodeQuery(query.getUrl());
- HttpMethodBase method = new GetMethod(url);
- method.setRequestHeader("Accept", "application/json");
- return method;
+ HttpMethodBase method = new GetMethod(server.getHost() + query.getUrl());
+ setJsonHeader(method);
+ return method;
+ }
- } catch (URIException e) {
- throw new ConnectionException("Query: " + query, e);
- }
+ private HttpMethodBase newDeleteRequest(DeleteQuery<?> query) {
+ HttpMethodBase method = new DeleteMethod(server.getHost() + query.getUrl());
+ setJsonHeader(method);
+ return method;
}
private HttpMethodBase newPostRequest(CreateQuery<?> query) {
- try {
- String url = server.getHost() + URIUtil.encodeQuery(query.getUrl());
- PostMethod method = new PostMethod(url);
- method.setRequestHeader("Accept", "application/json");
- if (query.getBody() != null) {
- method.setRequestEntity(new StringRequestEntity(query.getBody(), "text/plain; charset=UTF-8", "UTF-8"));
- }
- return method;
-
- } catch (URIException e) {
- throw new ConnectionException("Query: " + query, e);
- } catch (UnsupportedEncodingException e) {
- throw new ConnectionException("Unsupported encoding", e);
- }
+ PostMethod method = new PostMethod(server.getHost() + query.getUrl());
+ setJsonHeader(method);
+ setRequestEntity(method, query);
+ return method;
}
private HttpMethodBase newPutRequest(UpdateQuery<?> query) {
- try {
- String url = server.getHost() + URIUtil.encodeQuery(query.getUrl());
- PutMethod method = new PutMethod(url);
- method.setRequestHeader("Accept", "application/json");
- if (query.getBody() != null) {
- method.setRequestEntity(new StringRequestEntity(query.getBody(), "text/plain; charset=UTF-8", "UTF-8"));
- }
- return method;
+ PutMethod method = new PutMethod(server.getHost() + query.getUrl());
+ setJsonHeader(method);
+ setRequestEntity(method, query);
+ return method;
+ }
- } catch (URIException e) {
- throw new ConnectionException("Query: " + query, e);
- } catch (UnsupportedEncodingException e) {
- throw new ConnectionException("Unsupported encoding", e);
+ private void setRequestEntity(EntityEnclosingMethod request, AbstractQuery<?> query) {
+ if (query.getBody() != null) {
+ try {
+ request.setRequestEntity(new StringRequestEntity(query.getBody(), "text/plain; charset=UTF-8", "UTF-8"));
+ } catch (UnsupportedEncodingException e) {
+ throw new ConnectionException("Unsupported encoding", e);
+ }
}
}
- private HttpMethodBase newDeleteRequest(DeleteQuery<?> query) {
- try {
- String url = server.getHost() + URIUtil.encodeQuery(query.getUrl());
- HttpMethodBase method = new DeleteMethod(url);
- method.setRequestHeader("Accept", "application/json");
- return method;
-
- } catch (URIException e) {
- throw new ConnectionException("Query: " + query, e);
- }
+ private void setJsonHeader(HttpMethodBase request) {
+ request.setRequestHeader("Accept", "application/json");
}
private String getResponseBodyAsString(HttpMethod method) {
@@ -19,10 +19,24 @@
*/
package org.sonar.wsclient.connectors;
-import org.apache.http.*;
-import org.apache.http.auth.*;
+import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpException;
+import org.apache.http.HttpHost;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpRequestInterceptor;
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
+import org.apache.http.auth.AuthScheme;
+import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.AuthState;
+import org.apache.http.auth.Credentials;
+import org.apache.http.auth.UsernamePasswordCredentials;
import org.apache.http.client.CredentialsProvider;
import org.apache.http.client.methods.HttpDelete;
+import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpPut;
@@ -38,14 +52,12 @@
import org.apache.http.protocol.HttpContext;
import org.apache.http.util.EntityUtils;
import org.sonar.wsclient.Host;
+import org.sonar.wsclient.services.AbstractQuery;
import org.sonar.wsclient.services.CreateQuery;
import org.sonar.wsclient.services.DeleteQuery;
import org.sonar.wsclient.services.Query;
import org.sonar.wsclient.services.UpdateQuery;
-import java.io.IOException;
-import java.io.UnsupportedEncodingException;
-
/**
* @since 2.1
*/
@@ -85,7 +97,9 @@ private String executeRequest(HttpRequestBase request) {
json = EntityUtils.toString(entity);
} else if (response.getStatusLine().getStatusCode() != HttpStatus.SC_NOT_FOUND) {
- throw new ConnectionException("HTTP error: " + response.getStatusLine().getStatusCode() + ", msg: " + response.getStatusLine().getReasonPhrase() + ", query: " + request.toString());
+ throw new ConnectionException("HTTP error: " + response.getStatusLine().getStatusCode()
+ + ", msg: " + response.getStatusLine().getReasonPhrase()
+ + ", query: " + request.toString());
}
}
@@ -104,7 +118,8 @@ private DefaultHttpClient createClient() {
HttpConnectionParams.setConnectionTimeout(params, TIMEOUT_MS);
HttpConnectionParams.setSoTimeout(params, TIMEOUT_MS);
if (server.getUsername() != null) {
- client.getCredentialsProvider().setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(server.getUsername(), server.getPassword()));
+ client.getCredentialsProvider()
+ .setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(server.getUsername(), server.getPassword()));
}
return client;
}
@@ -138,28 +153,26 @@ private HttpDelete newDeleteMethod(DeleteQuery<?> query) {
private HttpPost newPostMethod(CreateQuery<?> query) {
HttpPost post = new HttpPost(server.getHost() + query.getUrl());
- if (query.getBody() != null) {
- try {
- post.setEntity(new StringEntity(query.getBody(), "UTF-8"));
- } catch (UnsupportedEncodingException e) {
- throw new ConnectionException("Encoding is not supported", e);
- }
- }
setJsonHeader(post);
+ setRequestEntity(post, query);
return post;
}
private HttpPut newPutMethod(UpdateQuery<?> query) {
HttpPut put = new HttpPut(server.getHost() + query.getUrl());
+ setJsonHeader(put);
+ setRequestEntity(put, query);
+ return put;
+ }
+
+ private void setRequestEntity(HttpEntityEnclosingRequestBase request, AbstractQuery<?> query) {
if (query.getBody() != null) {
try {
- put.setEntity(new StringEntity(query.getBody(), "UTF-8"));
+ request.setEntity(new StringEntity(query.getBody(), "UTF-8"));
} catch (UnsupportedEncodingException e) {
throw new ConnectionException("Encoding is not supported", e);
}
}
- setJsonHeader(put);
- return put;
}
private void setJsonHeader(HttpRequestBase request) {
@@ -28,21 +28,36 @@
/**
* Must start with a slash, for example: /api/metrics
+ * <p>
+ * IMPORTANT: In implementations of this method we must use helper methods to construct URL.
+ * </p>
+ *
+ * @see #encode(String)
+ * @see #appendUrlParameter(StringBuilder, String, Object)
+ * @see #appendUrlParameter(StringBuilder, String, Object[])
+ * @see #appendUrlParameter(StringBuilder, String, Date, boolean)
*/
public abstract String getUrl();
/**
- * Request body. By defaut it is empty but it can be overriden.
+ * Request body. By default it is empty but it can be overridden.
*/
public String getBody() {
return null;
}
+ /**
+ * Encodes single parameter value.
+ */
+ protected static String encode(String value) {
+ return WSUtils.getINSTANCE().encodeUrl(value);
+ }
+
protected static void appendUrlParameter(StringBuilder url, String paramKey, Object paramValue) {
if (paramValue != null) {
url.append(paramKey)
.append('=')
- .append(paramValue)
+ .append(encode(paramValue.toString()))
.append('&');
}
}
@@ -55,7 +70,7 @@ protected static void appendUrlParameter(StringBuilder url, String paramKey, Obj
url.append(',');
}
if (paramValues[index] != null) {
- url.append(paramValues[index]);
+ url.append(encode(paramValues[index].toString()));
}
}
url.append('&');
@@ -67,7 +82,7 @@ protected static void appendUrlParameter(StringBuilder url, String paramKey, Dat
String format = (includeTime ? "yyyy-MM-dd'T'HH:mm:ssZ" : "yyyy-MM-dd");
url.append(paramKey)
.append('=')
- .append(WSUtils.getINSTANCE().encodeUrl(WSUtils.getINSTANCE().format(paramValue, format)))
+ .append(encode(WSUtils.getINSTANCE().format(paramValue, format)))
.append('&');
}
}
@@ -59,18 +59,10 @@ public DependencyQuery setDirection(String direction) {
public String getUrl() {
StringBuilder url = new StringBuilder(BASE_URL);
url.append('?');
- if (resourceIdOrKey != null) {
- url.append("resource=").append(resourceIdOrKey).append('&');
- }
- if (direction != null) {
- url.append("dir=").append(direction).append('&');
- }
- if (parentId !=null) {
- url.append("parent=").append(parentId).append('&');
- }
- if (id !=null) {
- url.append("id=").append(id).append('&');
- }
+ appendUrlParameter(url, "resource", resourceIdOrKey);
+ appendUrlParameter(url, "dir", direction);
+ appendUrlParameter(url, "parent", parentId);
+ appendUrlParameter(url, "id", id);
return url.toString();
}
@@ -99,7 +91,7 @@ public DependencyQuery setParentId(String parentId) {
/**
* Resources that depend upon a resource
- *
+ *
* @param resourceIdOrKey the target resource. Can be the primary key (a number) or the logical key (String)
*/
public static DependencyQuery createForIncomingDependencies(String resourceIdOrKey) {
@@ -111,7 +103,7 @@ public static DependencyQuery createForIncomingDependencies(String resourceIdOrK
/**
* Resources that are depended upon a resource = all the resources that a resource depends upon
- *
+ *
* @param resourceIdOrKey the target resource. Can be the primary key (an integer) or the logical key (String)
*/
public static DependencyQuery createForOutgoingDependencies(String resourceIdOrKey) {
@@ -124,7 +116,7 @@ public static DependencyQuery createForOutgoingDependencies(String resourceIdOrK
/**
* Resources that depend upon or are depended upon a resource. It equals the merge of createForIncomingDependencies(resourceIdOrKey)
* and createForOutgoingDependencies(resourceIdOrKey)
- *
+ *
* @param resourceIdOrKey the target resource. Can be the primary key (an integer) or the logical key (String)
*/
public static DependencyQuery createForResource(String resourceIdOrKey) {
@@ -53,23 +53,9 @@ public DependencyTreeQuery setScopes(String... scopes) {
@Override
public String getUrl() {
StringBuilder url = new StringBuilder(BASE_URL);
- url.append("?resource=")
- .append(resourceId)
- .append("&");
- if (scopes != null) {
- url.append("scopes=");
- if (scopes != null) {
- for (int index = 0; index < scopes.length; index++) {
- if (index > 0) {
- url.append(',');
- }
- if (scopes[index] != null) {
- url.append(scopes[index]);
- }
- }
- url.append('&');
- }
- }
+ url.append('?');
+ appendUrlParameter(url, "resource", resourceId);
+ appendUrlParameter(url, "scopes", scopes);
return url.toString();
}
@@ -29,7 +29,9 @@ public FavouriteCreateQuery(String idOrKey) {
@Override
public String getUrl() {
- return new StringBuilder().append(FavouriteQuery.BASE_URL).append("?key=").append(idOrKey).toString();
+ StringBuilder url = new StringBuilder(FavouriteQuery.BASE_URL).append('?');
+ appendUrlParameter(url, "key", idOrKey);
+ return url.toString();
}
@Override
@@ -33,6 +33,6 @@ public String getIdOrKey() {
@Override
public String getUrl() {
- return new StringBuilder().append(FavouriteQuery.BASE_URL).append('/').append(idOrKey).toString();
+ return new StringBuilder().append(FavouriteQuery.BASE_URL).append('/').append(encode(idOrKey)).toString();
}
}
Oops, something went wrong.

0 comments on commit dd4e31c

Please sign in to comment.