Skip to content

Commit 6fa9742

Browse files
author
Stephane Landelle
committed
Don't replace empty path with a / in Request, close AsyncHttpClient#606 
1 parent bd8bdbb commit 6fa9742

File tree

15 files changed

+61
-94
lines changed

15 files changed

+61
-94
lines changed

‎api/src/main/java/org/asynchttpclient/Request.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,6 @@ public interface Request {
4646
*/
4747
String getMethod();
4848

49-
/**
50-
* Return the decoded url
51-
*
52-
* @return the decoded url
53-
*/
54-
String getUrl();
55-
5649
UriComponents getURI();
5750

5851
/**

‎api/src/main/java/org/asynchttpclient/RequestBuilderBase.java

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -114,20 +114,6 @@ public InetAddress getLocalAddress() {
114114
return localAddress;
115115
}
116116

117-
private String removeTrailingSlash(UriComponents uri) {
118-
String uriString = uri.toUrl();
119-
if (uriString.endsWith("/")) {
120-
return uriString.substring(0, uriString.length() - 1);
121-
} else {
122-
return uriString;
123-
}
124-
}
125-
126-
@Override
127-
public String getUrl() {
128-
return removeTrailingSlash(getURI());
129-
}
130-
131117
@Override
132118
public UriComponents getURI() {
133119
return uri;
@@ -302,8 +288,6 @@ public T setUrl(String url) {
302288
}
303289

304290
public T setURI(UriComponents uri) {
305-
if (uri.getPath() == null)
306-
throw new NullPointerException("uri.path");
307291
request.uri = uri;
308292
return derived.cast(this);
309293
}
@@ -602,22 +586,20 @@ private void computeRequestLength() {
602586
private void computeFinalUri() {
603587

604588
if (request.uri == null) {
605-
logger.debug("setUrl hasn't been invoked. Using http://localhost");
589+
logger.debug("setUrl hasn't been invoked. Using {}", DEFAULT_REQUEST_URL);
606590
request.uri = DEFAULT_REQUEST_URL;
607591
}
608592

609593
AsyncHttpProviderUtils.validateSupportedScheme(request.uri);
610594

611-
// FIXME is that right?
612-
String newPath = isNonEmpty(request.uri.getPath()) ? request.uri.getPath() : "/";
613595
String newQuery = queryComputer.computeFullQueryString(request.uri.getQuery(), queryParams);
614596

615597
request.uri = new UriComponents(//
616598
request.uri.getScheme(),//
617599
request.uri.getUserInfo(),//
618600
request.uri.getHost(),//
619601
request.uri.getPort(),//
620-
newPath,//
602+
request.uri.getPath(),//
621603
newQuery);
622604
}
623605

‎api/src/main/java/org/asynchttpclient/resumable/ResumableAsyncHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ public AsyncHandler.STATE onHeadersReceived(HttpResponseHeaders headers) throws
197197
*/
198198
public Request adjustRequestRange(Request request) {
199199

200-
Long ri = resumableIndex.get(request.getUrl());
200+
Long ri = resumableIndex.get(request.getURI().toUrl());
201201
if (ri != null) {
202202
byteTransferred.set(ri);
203203
}

‎api/src/main/java/org/asynchttpclient/uri/UriComponents.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
*/
1313
package org.asynchttpclient.uri;
1414

15+
import static org.asynchttpclient.util.MiscUtils.isNonEmpty;
16+
1517
import java.net.URI;
1618
import java.net.URISyntaxException;
1719

@@ -64,6 +66,15 @@ public String getQuery() {
6466
return query;
6567
}
6668

69+
/**
70+
* Convenient for HTTP layer when targeting server root
71+
*
72+
* @return the raw path or "/" if it's null
73+
*/
74+
public String getNonEmptyPath() {
75+
return isNonEmpty(path) ? path : "/";
76+
}
77+
6778
public String getPath() {
6879
return path;
6980
}

‎api/src/main/java/org/asynchttpclient/util/AsyncHttpProviderUtils.java

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
*/
1313
package org.asynchttpclient.util;
1414

15-
import static org.asynchttpclient.util.MiscUtils.isNonEmpty;
16-
1715
import org.asynchttpclient.AsyncHttpClientConfig;
1816
import org.asynchttpclient.AsyncHttpProvider;
1917
import org.asynchttpclient.HttpResponseBodyPart;
@@ -49,22 +47,6 @@ public static final void validateSupportedScheme(UriComponents uri) {
4947
}
5048
}
5149

52-
public final static UriComponents createNonEmptyPathURI(String u) {
53-
UriComponents uri = UriComponents.create(u);
54-
validateSupportedScheme(uri);
55-
56-
String path = uri.getPath();
57-
if (path == null) {
58-
throw new IllegalArgumentException("The URI path, of the URI " + uri + ", must be non-null");
59-
} else if (isNonEmpty(path) && path.charAt(0) != '/') {
60-
throw new IllegalArgumentException("The URI path, of the URI " + uri + ". must start with a '/'");
61-
} else if (!isNonEmpty(path)) {
62-
return UriComponents.create(u + "/");
63-
}
64-
65-
return uri;
66-
}
67-
6850
/**
6951
* @param bodyParts NON EMPTY body part
7052
* @param maxLen

‎api/src/test/java/org/asynchttpclient/async/AsyncProvidersBasicTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,16 @@
6464

6565
public abstract class AsyncProvidersBasicTest extends AbstractBasicTest {
6666

67+
protected abstract String acceptEncodingHeader();
68+
69+
protected abstract AsyncHttpProviderConfig<?, ?> getProviderConfig();
70+
6771
@Test(groups = { "standalone", "default_provider", "async" })
6872
public void asyncProviderEncodingTest() throws Exception {
6973
AsyncHttpClient client = getAsyncHttpClient(null);
7074
try {
7175
Request request = new RequestBuilder("GET").setUrl(getTargetUrl() + "?q=+%20x").build();
72-
assertEquals(request.getUrl(), getTargetUrl() + "?q=%20%20x");
76+
assertEquals(request.getURI().toUrl(), getTargetUrl() + "?q=%20%20x");
7377

7478
String url = client.executeRequest(request, new AsyncCompletionHandler<String>() {
7579
@Override
@@ -680,8 +684,6 @@ public Response onCompleted(Response response) throws Exception {
680684
client.close();
681685
}
682686
}
683-
684-
protected abstract String acceptEncodingHeader();
685687

686688
@Test(groups = { "standalone", "default_provider", "async" })
687689
public void asyncDoPostBasicGZIPTest() throws Exception {
@@ -1595,6 +1597,4 @@ public void mirrorByteTest() throws Exception {
15951597
client.close();
15961598
}
15971599
}
1598-
1599-
protected abstract AsyncHttpProviderConfig<?, ?> getProviderConfig();
16001600
}

‎api/src/test/java/org/asynchttpclient/async/RequestBuilderTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public void testEncodesQueryParameters() throws UnsupportedEncodingException {
7070
}
7171
String expValue = sb.toString();
7272
Request request = builder.build();
73-
assertEquals(request.getUrl(), "http://example.com/?name=" + expValue);
73+
assertEquals(request.getURI().toUrl(), "http://example.com/?name=" + expValue);
7474
}
7575
}
7676

@@ -83,7 +83,7 @@ public void testChaining() throws IOException, ExecutionException, InterruptedEx
8383

8484
Request request2 = new RequestBuilder(request).build();
8585

86-
assertEquals(request2.getUrl(), request.getUrl());
86+
assertEquals(request2.getURI(), request.getURI());
8787
}
8888

8989
@Test(groups = {"standalone", "default_provider"})
@@ -93,7 +93,7 @@ public void testParsesQueryParams() throws IOException, ExecutionException, Inte
9393
.addQueryParam("param2", "value2")
9494
.build();
9595

96-
assertEquals(request.getUrl(), "http://foo.com/?param1=value1&param2=value2");
96+
assertEquals(request.getURI().toUrl(), "http://foo.com/?param1=value1&param2=value2");
9797
List<Param> params = request.getQueryParams();
9898
assertEquals(params.size(), 2);
9999
assertEquals(params.get(0), new Param("param1", "value1"));
@@ -104,14 +104,14 @@ public void testParsesQueryParams() throws IOException, ExecutionException, Inte
104104
public void testUserProvidedRequestMethod() {
105105
Request req = new RequestBuilder("ABC").setUrl("http://foo.com").build();
106106
assertEquals(req.getMethod(), "ABC");
107-
assertEquals(req.getUrl(), "http://foo.com/");
107+
assertEquals(req.getURI().toUrl(), "http://foo.com/");
108108
}
109109

110110
@Test(groups = {"standalone", "default_provider"})
111111
public void testPercentageEncodedUserInfo() {
112112
final Request req = new RequestBuilder("GET").setUrl("http://hello:wor%[email protected]").build();
113113
assertEquals(req.getMethod(), "GET");
114-
assertEquals(req.getUrl(), "http://hello:wor%[email protected]");
114+
assertEquals(req.getURI().toUrl(), "http://hello:wor%[email protected]");
115115
}
116116

117117
@Test(groups = {"standalone", "default_provider"})

‎api/src/test/java/org/asynchttpclient/resumable/ResumableAsyncHandlerTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,13 @@ public void testAdjustRange() {
3131
ResumableAsyncHandler h = new ResumableAsyncHandler(proc);
3232
Request request = new RequestBuilder("GET").setUrl("http://test/url").build();
3333
Request newRequest = h.adjustRequestRange(request);
34-
assertEquals(newRequest.getUrl(), request.getUrl());
34+
assertEquals(newRequest.getURI(), request.getURI());
3535
String rangeHeader = newRequest.getHeaders().getFirstValue("Range");
3636
assertNull(rangeHeader);
3737

3838
proc.put("http://test/url", 5000);
3939
newRequest = h.adjustRequestRange(request);
40-
assertEquals(newRequest.getUrl(), request.getUrl());
40+
assertEquals(newRequest.getURI(), request.getURI());
4141
rangeHeader = newRequest.getHeaders().getFirstValue("Range");
4242
assertEquals(rangeHeader, "bytes=5000-");
4343
}

‎providers/grizzly/src/main/java/org/asynchttpclient/providers/grizzly/HttpTxContext.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.asynchttpclient.providers.grizzly.bodyhandler.BodyHandler;
1919
import org.asynchttpclient.providers.grizzly.statushandler.StatusHandler;
2020
import org.asynchttpclient.providers.grizzly.statushandler.StatusHandler.InvocationStatus;
21+
import org.asynchttpclient.uri.UriComponents;
2122
import org.asynchttpclient.util.AsyncHttpProviderUtils;
2223
import org.asynchttpclient.websocket.WebSocket;
2324
import org.glassfish.grizzly.CloseListener;
@@ -33,6 +34,7 @@
3334
import java.io.IOException;
3435
import java.util.concurrent.atomic.AtomicInteger;
3536
import java.util.concurrent.atomic.AtomicLong;
37+
3638
import org.asynchttpclient.providers.grizzly.filters.events.GracefulCloseEvent;
3739
import org.glassfish.grizzly.Connection;
3840
import org.glassfish.grizzly.filterchain.FilterChain;
@@ -50,7 +52,7 @@ public final class HttpTxContext {
5052
private final GrizzlyAsyncHttpProvider provider;
5153

5254
private Request request;
53-
private String requestUrl;
55+
private UriComponents requestUri;
5456
private final AsyncHandler handler;
5557
private BodyHandler bodyHandler;
5658
private StatusHandler statusHandler;
@@ -61,7 +63,7 @@ public final class HttpTxContext {
6163
private final AtomicLong totalBodyWritten = new AtomicLong();
6264
private AsyncHandler.STATE currentState;
6365

64-
private String wsRequestURI;
66+
private UriComponents wsRequestURI;
6567
private boolean isWSRequest;
6668
private HandShake handshake;
6769
private ProtocolHandler protocolHandler;
@@ -97,7 +99,7 @@ private HttpTxContext(final GrizzlyAsyncHttpProvider provider, final GrizzlyResp
9799
this.handler = handler;
98100
redirectsAllowed = this.provider.getClientConfig().isFollowRedirect();
99101
maxRedirectCount = this.provider.getClientConfig().getMaxRedirects();
100-
this.requestUrl = request.getUrl();
102+
this.requestUri = request.getURI();
101103
}
102104

103105
// ---------------------------------------------------------- Public Methods
@@ -160,12 +162,12 @@ public void setRequest(Request request) {
160162
this.request = request;
161163
}
162164

163-
public String getRequestUrl() {
164-
return requestUrl;
165+
public UriComponents getRequestUri() {
166+
return requestUri;
165167
}
166168

167-
public void setRequestUrl(String requestUrl) {
168-
this.requestUrl = requestUrl;
169+
public void setRequestUri(UriComponents requestUri) {
170+
this.requestUri = requestUri;
169171
}
170172

171173
public AsyncHandler getHandler() {
@@ -232,11 +234,11 @@ public void setCurrentState(AsyncHandler.STATE currentState) {
232234
this.currentState = currentState;
233235
}
234236

235-
public String getWsRequestURI() {
237+
public UriComponents getWsRequestURI() {
236238
return wsRequestURI;
237239
}
238240

239-
public void setWsRequestURI(String wsRequestURI) {
241+
public void setWsRequestURI(UriComponents wsRequestURI) {
240242
this.wsRequestURI = wsRequestURI;
241243
}
242244

‎providers/grizzly/src/main/java/org/asynchttpclient/providers/grizzly/filters/AsyncHttpClientFilter.java

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ private boolean sendAsGrizzlyRequest(
218218
return true;
219219
}
220220

221-
if (isUpgradeRequest(httpTxContext.getHandler()) && isWSRequest(httpTxContext.getRequestUrl())) {
221+
if (isUpgradeRequest(httpTxContext.getHandler()) && isWSRequest(httpTxContext.getRequestUri())) {
222222
httpTxContext.setWSRequest(true);
223223
convertToUpgradeRequest(httpTxContext);
224224
}
@@ -238,7 +238,7 @@ private boolean sendAsGrizzlyRequest(
238238
final int port = uri.getPort();
239239
requestPacket.setRequestURI(uri.getHost() + ':' + (port == -1 ? 443 : port));
240240
} else {
241-
requestPacket.setRequestURI(uri.getPath());
241+
requestPacket.setRequestURI(uri.getNonEmptyPath());
242242
}
243243

244244
final BodyHandler bodyHandler = isPayloadAllowed(method) ?
@@ -257,7 +257,7 @@ private boolean sendAsGrizzlyRequest(
257257

258258
if (httpTxContext.isWSRequest()) {
259259
try {
260-
final URI wsURI = new URI(httpTxContext.getWsRequestURI());
260+
final URI wsURI = httpTxContext.getWsRequestURI().toURI();
261261
httpTxContext.setProtocolHandler(Version.RFC6455.createHandler(true));
262262
httpTxContext.setHandshake(httpTxContext.getProtocolHandler().createHandShake(wsURI));
263263
requestPacket = (HttpRequestPacket) httpTxContext.getHandshake().composeHeaders().getHttpHeader();
@@ -462,21 +462,18 @@ private static boolean isUpgradeRequest(final AsyncHandler handler) {
462462
return (handler instanceof UpgradeHandler);
463463
}
464464

465-
private static boolean isWSRequest(final String requestUri) {
466-
return (requestUri.charAt(0) == 'w' && requestUri.charAt(1) == 's');
465+
private static boolean isWSRequest(final UriComponents requestUri) {
466+
return requestUri.getScheme().startsWith("ws");
467467
}
468468

469469
private static void convertToUpgradeRequest(final HttpTxContext ctx) {
470-
final int colonIdx = ctx.getRequestUrl().indexOf(':');
471-
472-
if (colonIdx < 2 || colonIdx > 3) {
473-
throw new IllegalArgumentException("Invalid websocket URL: " + ctx.getRequestUrl());
474-
}
475-
476-
final StringBuilder sb = new StringBuilder(ctx.getRequestUrl());
477-
sb.replace(0, colonIdx, ((colonIdx == 2) ? "http" : "https"));
478-
ctx.setWsRequestURI(ctx.getRequestUrl());
479-
ctx.setRequestUrl(sb.toString());
470+
471+
UriComponents originalUri = ctx.getRequestUri();
472+
String newScheme = originalUri.getScheme().equalsIgnoreCase("https")? "wss" : "ws";
473+
ctx.setRequestUri(originalUri.withNewScheme(newScheme));
474+
475+
ctx.setWsRequestURI(ctx.getRequestUri());
476+
ctx.setRequestUri(originalUri.withNewScheme(newScheme));
480477
}
481478

482479
private void addGeneralHeaders(final Request request, final HttpRequestPacket requestPacket) {

‎providers/grizzly/src/main/java/org/asynchttpclient/providers/grizzly/filters/ProxyFilter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public NextAction handleWrite(FilterChainContext ctx) throws IOException {
6262
assert (context != null);
6363
Request req = context.getRequest();
6464
if (!secure) {
65-
request.setRequestURI(req.getURI().toString());
65+
request.setRequestURI(req.getURI().toUrl());
6666
}
6767
addProxyHeaders(getRealm(req), request);
6868
return ctx.getInvokeAction();

‎providers/grizzly/src/main/java/org/asynchttpclient/providers/grizzly/statushandler/AuthorizationHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public boolean handleStatus(final HttpResponsePacket responsePacket, final HttpT
6767
responsePacket.setSkipRemainder(true); // ignore the remainder of the response
6868

6969
final Request req = httpTransactionContext.getRequest();
70-
realm = new Realm.RealmBuilder().clone(realm).setScheme(realm.getAuthScheme()).setUri(req.getURI().getPath())
70+
realm = new Realm.RealmBuilder().clone(realm).setScheme(realm.getAuthScheme()).setUri(req.getURI().getNonEmptyPath())
7171
.setMethodName(req.getMethod()).setUsePreemptiveAuth(true).parseWWWAuthenticateHeader(auth).build();
7272
String lowerCaseAuth = auth.toLowerCase(Locale.ENGLISH);
7373
if (lowerCaseAuth.startsWith("basic")) {

‎providers/grizzly/src/main/java/org/asynchttpclient/providers/grizzly/statushandler/RedirectHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public boolean handleStatus(final HttpResponsePacket responsePacket, final HttpT
7575
httpTransactionContext.setFuture(null);
7676
newContext.setInvocationStatus(CONTINUE);
7777
newContext.setRequest(requestToSend);
78-
newContext.setRequestUrl(requestToSend.getUrl());
78+
newContext.setRequestUri(requestToSend.getURI());
7979
HttpTxContext.set(ctx, newContext);
8080
httpTransactionContext.getProvider().execute(c, requestToSend, newContext.getHandler(), newContext.getFuture(), newContext);
8181
return false;

0 commit comments

Comments
 (0)