Skip to content

Commit e897c61

Browse files
mbknormbknor
committed
[playframework#853] new fix for urls, encoding, AHC and WS
This approach does not rely on WS.parameters, but it passes the QueryString-part of the url to AHC in a encoding-suitable way..
1 parent f3b7337 commit e897c61

File tree

4 files changed

+115
-81
lines changed

4 files changed

+115
-81
lines changed

‎framework/src/play/libs/WS.java

-69
Original file line numberDiff line numberDiff line change
@@ -246,75 +246,6 @@ public WSRequest() {
246246
}
247247

248248
public WSRequest(String url, String encoding) {
249-
250-
// WSAsync is using raw-urls in AHC => does not encode the url or the parameters.
251-
// so we have to do it before sending them to AHC.
252-
// Params added with setParameter() are encoded before sent to AHC.
253-
// We need to pile of any params directly added to the url, and add them as parameters.
254-
// This will make all params correct encoded.. (This is the same as happens inside AHC when
255-
// passing AHC an url with params). This approach should work fine with all impls of WSImpl.
256-
257-
// does url contain query_string?
258-
int i = url.indexOf('?');
259-
if ( i > 0) {
260-
261-
try {
262-
// extract query-string-part
263-
String queryPart = url.substring(i+1);
264-
// remove query-string from url
265-
url = url.substring(0,i);
266-
267-
// parse queryPart - and decode it... (it is going to be re-encoded later)
268-
for( String param : queryPart.split("&")) {
269-
String[] paramParts = param.split("=");
270-
String name;
271-
String value = null;
272-
if ( paramParts.length == 1) {
273-
// only a flag
274-
name = URLDecoder.decode(paramParts[0], encoding);
275-
276-
// if param ends with "=", then we have an empty value
277-
// if not ending with "=", we have a key without a value (a flag)
278-
if (param.endsWith("=")) {
279-
value = "";
280-
}
281-
} else {
282-
name = URLDecoder.decode(paramParts[0], encoding);
283-
value = URLDecoder.decode(paramParts[1], encoding);
284-
}
285-
286-
// create or add param/value
287-
Object existingParamValue = parameters.get(name);
288-
if ( existingParamValue != null) {
289-
if ( value != null ) {
290-
// must add it.
291-
292-
if ( existingParamValue instanceof String[]) {
293-
String[] oldArray = (String[])existingParamValue;
294-
// allready got array with values.. must add one more..
295-
String[] newArray = new String[oldArray.length+1];
296-
int n=0;
297-
for( String oldValue : oldArray) {
298-
newArray[n++] = oldValue;
299-
}
300-
newArray[n++] = value;
301-
} else {
302-
// existingParamValue is a string, we must replace it with array
303-
// containing both old and new value
304-
parameters.put( name, new String[]{(String)existingParamValue, value});
305-
}
306-
}
307-
} else {
308-
// a new param - add it
309-
parameters.put(name, value);
310-
}
311-
}
312-
} catch (UnsupportedEncodingException e) {
313-
throw new RuntimeException("Error parsing query-part of url",e);
314-
}
315-
}
316-
317-
318249
this.url = url;
319250
this.encoding = encoding;
320251
setDefaultContentType();

‎framework/src/play/libs/ws/WSAsync.java

+107-12
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import java.io.IOException;
55
import java.io.InputStream;
66
import java.io.UnsupportedEncodingException;
7+
import java.net.URLDecoder;
8+
import java.net.URLEncoder;
79
import java.util.ArrayList;
810
import java.util.Arrays;
911
import java.util.Collection;
@@ -111,13 +113,105 @@ protected WSAsyncRequest(String url, String encoding) {
111113
super(url, encoding);
112114
}
113115

116+
/**
117+
* Returns the url but removed the queryString-part of it
118+
* The QueryString-info is later added with addQueryString()
119+
*/
120+
protected String getUrlWithoutQueryString() {
121+
int i = url.indexOf('?');
122+
if ( i > 0) {
123+
return url.substring(0,i);
124+
} else {
125+
return url;
126+
}
127+
}
128+
129+
/**
130+
* Adds the queryString-part of the url to the BoundRequestBuilder
131+
* @return the same BoundRequestBuilder as passed in
132+
*/
133+
protected BoundRequestBuilder addQueryString(BoundRequestBuilder requestBuilder) {
134+
135+
// AsyncHttpClient is by default encoding everything in utf-8 so for us to be able to use
136+
// different encoding we have configured AHC to use raw urls. When using raw urls,
137+
// AHC does not encode url and QueryParam with utf-8 - but there is another problem:
138+
// If we send raw (none-encoded) url (with queryString) to AHC, it does not url-encode it,
139+
// but transform all illegal chars to '?'.
140+
// If we pre-encoded the url with QueryString before sending it to AHC, ahc will decode it, and then
141+
// later break it with '?'.
142+
143+
// This method basically does the same as RequestBuilderBase.buildUrl() except from destroying the
144+
// pre-encoding
145+
146+
// does url contain query_string?
147+
int i = url.indexOf('?');
148+
if ( i > 0) {
149+
150+
try {
151+
// extract query-string-part
152+
String queryPart = url.substring(i+1);
153+
154+
// parse queryPart - and decode it... (it is going to be re-encoded later)
155+
for( String param : queryPart.split("&")) {
156+
157+
i = param.indexOf('=');
158+
String name;
159+
String value = null;
160+
if ( i<=0) {
161+
// only a flag
162+
name = URLDecoder.decode(param, encoding);
163+
} else {
164+
name = URLDecoder.decode(param.substring(0,i), encoding);
165+
value = URLDecoder.decode(param.substring(i+1), encoding);
166+
}
167+
168+
if (value == null) {
169+
requestBuilder.addQueryParameter(URLEncoder.encode(name, encoding), null);
170+
} else {
171+
requestBuilder.addQueryParameter(URLEncoder.encode(name, encoding), URLEncoder.encode(value, encoding));
172+
}
173+
174+
}
175+
} catch (UnsupportedEncodingException e) {
176+
throw new RuntimeException("Error parsing query-part of url",e);
177+
}
178+
}
179+
180+
return requestBuilder;
181+
}
182+
183+
184+
public BoundRequestBuilder prepareGet() {
185+
return addQueryString(httpClient.prepareGet(getUrlWithoutQueryString()));
186+
}
187+
188+
public BoundRequestBuilder prepareOptions() {
189+
return addQueryString(httpClient.prepareOptions(getUrlWithoutQueryString()));
190+
}
191+
192+
public BoundRequestBuilder prepareHead() {
193+
return addQueryString(httpClient.prepareHead(getUrlWithoutQueryString()));
194+
}
195+
196+
public BoundRequestBuilder preparePost() {
197+
return addQueryString(httpClient.preparePost(getUrlWithoutQueryString()));
198+
}
199+
200+
public BoundRequestBuilder preparePut() {
201+
return addQueryString(httpClient.preparePut(getUrlWithoutQueryString()));
202+
}
203+
204+
public BoundRequestBuilder prepareDelete() {
205+
return addQueryString(httpClient.prepareDelete(getUrlWithoutQueryString()));
206+
}
207+
114208
/** Execute a GET request synchronously. */
115209
@Override
116210
public HttpResponse get() {
117211
this.type = "GET";
118212
sign();
119213
try {
120-
return new HttpAsyncResponse(prepare(httpClient.prepareGet(url)).execute().get());
214+
return new HttpAsyncResponse(prepare(prepareGet()).execute().get());
121215
} catch (Exception e) {
122216
Logger.error(e.toString());
123217
throw new RuntimeException(e);
@@ -129,16 +223,17 @@ public HttpResponse get() {
129223
public Promise<HttpResponse> getAsync() {
130224
this.type = "GET";
131225
sign();
132-
return execute(httpClient.prepareGet(url));
226+
return execute(prepareGet());
133227
}
134228

229+
135230
/** Execute a POST request.*/
136231
@Override
137232
public HttpResponse post() {
138233
this.type = "POST";
139234
sign();
140235
try {
141-
return new HttpAsyncResponse(prepare(httpClient.preparePost(url)).execute().get());
236+
return new HttpAsyncResponse(prepare(preparePost()).execute().get());
142237
} catch (Exception e) {
143238
throw new RuntimeException(e);
144239
}
@@ -149,15 +244,15 @@ public HttpResponse post() {
149244
public Promise<HttpResponse> postAsync() {
150245
this.type = "POST";
151246
sign();
152-
return execute(httpClient.preparePost(url));
247+
return execute(preparePost());
153248
}
154249

155250
/** Execute a PUT request.*/
156251
@Override
157252
public HttpResponse put() {
158253
this.type = "PUT";
159254
try {
160-
return new HttpAsyncResponse(prepare(httpClient.preparePut(url)).execute().get());
255+
return new HttpAsyncResponse(prepare(preparePut()).execute().get());
161256
} catch (Exception e) {
162257
throw new RuntimeException(e);
163258
}
@@ -167,15 +262,15 @@ public HttpResponse put() {
167262
@Override
168263
public Promise<HttpResponse> putAsync() {
169264
this.type = "PUT";
170-
return execute(httpClient.preparePut(url));
265+
return execute(preparePut());
171266
}
172267

173268
/** Execute a DELETE request.*/
174269
@Override
175270
public HttpResponse delete() {
176271
this.type = "DELETE";
177272
try {
178-
return new HttpAsyncResponse(prepare(httpClient.prepareDelete(url)).execute().get());
273+
return new HttpAsyncResponse(prepare(prepareDelete()).execute().get());
179274
} catch (Exception e) {
180275
throw new RuntimeException(e);
181276
}
@@ -185,15 +280,15 @@ public HttpResponse delete() {
185280
@Override
186281
public Promise<HttpResponse> deleteAsync() {
187282
this.type = "DELETE";
188-
return execute(httpClient.prepareDelete(url));
283+
return execute(prepareDelete());
189284
}
190285

191286
/** Execute a OPTIONS request.*/
192287
@Override
193288
public HttpResponse options() {
194289
this.type = "OPTIONS";
195290
try {
196-
return new HttpAsyncResponse(prepare(httpClient.prepareOptions(url)).execute().get());
291+
return new HttpAsyncResponse(prepare(prepareOptions()).execute().get());
197292
} catch (Exception e) {
198293
throw new RuntimeException(e);
199294
}
@@ -203,15 +298,15 @@ public HttpResponse options() {
203298
@Override
204299
public Promise<HttpResponse> optionsAsync() {
205300
this.type = "OPTIONS";
206-
return execute(httpClient.prepareOptions(url));
301+
return execute(prepareOptions());
207302
}
208303

209304
/** Execute a HEAD request.*/
210305
@Override
211306
public HttpResponse head() {
212307
this.type = "HEAD";
213308
try {
214-
return new HttpAsyncResponse(prepare(httpClient.prepareHead(url)).execute().get());
309+
return new HttpAsyncResponse(prepare(prepareHead()).execute().get());
215310
} catch (Exception e) {
216311
throw new RuntimeException(e);
217312
}
@@ -221,7 +316,7 @@ public HttpResponse head() {
221316
@Override
222317
public Promise<HttpResponse> headAsync() {
223318
this.type = "HEAD";
224-
return execute(httpClient.prepareHead(url));
319+
return execute(prepareHead());
225320
}
226321

227322
/** Execute a TRACE request.*/

‎samples-and-tests/just-test-cases/app/controllers/Rest.java

+3
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ public static void echo(String id) {
109109
for ( String key : params.all().keySet()) {
110110
String[] values = params.all().get(key);
111111
for( String v : values) {
112+
if ( v == null) {
113+
v = "flag";
114+
}
112115
r += "|" + key + "|" + v;
113116
}
114117
}

‎samples-and-tests/just-test-cases/test/RestTest.java

+5
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ public void testEncodingEcho() {
134134
assertEquals("abc|id|abc|body||b|æøå|a|æøå|a|x", WS.url("http://localhost:9003/encoding/echo/abc?a=æøå&a=x&b=æøå").get().getString());
135135
assertEquals("æøå|id|æøå|body||b|æøå|a|æøå|a|x", WS.url("http://localhost:9003/encoding/echo/%s?a=æøå&a=x&b=æøå", "æøå").get().getString());
136136
assertEquals("æøå|id|æøå|body||b|æøå|a|æøå|a|x", WS.url("http://localhost:9003/encoding/echo/%s?", "æøå").setParameter("a",new String[]{"æøå","x"}).setParameter("b","æøå").get().getString());
137+
// test with value including '='
138+
assertEquals("abc|id|abc|body||b|æøå=|a|æøå|a|x", WS.url("http://localhost:9003/encoding/echo/abc?a=æøå&a=x&b=æøå=").get().getString());
139+
//test with 'flag'
140+
assertEquals("abc|id|abc|body||b|flag|a|flag", WS.url("http://localhost:9003/encoding/echo/abc?a&b=").get().getString());
141+
137142
// verify url ending with only ? or none
138143
assertEquals("abc|id|abc|body|", WS.url("http://localhost:9003/encoding/echo/abc?").get().getString());
139144
assertEquals("abc|id|abc|body|", WS.url("http://localhost:9003/encoding/echo/abc").get().getString());

0 commit comments

Comments
 (0)