HTTPCLIENT-1968: Make normalization of URI paths optional

Make it driven by RequestConfig.
This commit is contained in:
Tamas Cservenak 2019-02-16 11:50:11 +01:00
parent 09385e2c93
commit 4093a3015d
11 changed files with 168 additions and 50 deletions

View File

@ -754,7 +754,7 @@ public class CachingExec implements ClientExecChain {
final URI uri = conditionalRequest.getURI();
if (uri != null) {
try {
conditionalRequest.setURI(URIUtils.rewriteURIForRoute(uri, route));
conditionalRequest.setURI(URIUtils.rewriteURIForRoute(uri, route, context.getRequestConfig().isNormalizeUri()));
} catch (final URISyntaxException ex) {
throw new ProtocolException("Invalid URI: " + uri, ex);
}

View File

@ -137,7 +137,7 @@ public class DefaultRedirectHandler implements RedirectHandler {
try {
final URI requestURI = new URI(request.getRequestLine().getUri());
final URI absoluteRequestURI = URIUtils.rewriteURI(requestURI, target, true);
final URI absoluteRequestURI = URIUtils.rewriteURI(requestURI, target, URIUtils.DROP_FRAGMENT_AND_NORMALIZE);
uri = URIUtils.resolve(absoluteRequestURI, uri);
} catch (final URISyntaxException ex) {
throw new ProtocolException(ex.getMessage(), ex);
@ -161,7 +161,7 @@ public class DefaultRedirectHandler implements RedirectHandler {
uri.getHost(),
uri.getPort(),
uri.getScheme());
redirectURI = URIUtils.rewriteURI(uri, target, true);
redirectURI = URIUtils.rewriteURI(uri, target, URIUtils.DROP_FRAGMENT_AND_NORMALIZE);
} catch (final URISyntaxException ex) {
throw new ProtocolException(ex.getMessage(), ex);
}

View File

@ -337,14 +337,14 @@ public class DefaultRequestDirector implements RequestDirector {
// Make sure the request URI is absolute
if (!uri.isAbsolute()) {
final HttpHost target = route.getTargetHost();
uri = URIUtils.rewriteURI(uri, target, true);
uri = URIUtils.rewriteURI(uri, target, URIUtils.DROP_FRAGMENT_AND_NORMALIZE);
} else {
uri = URIUtils.rewriteURI(uri);
}
} else {
// Make sure the request URI is relative
if (uri.isAbsolute()) {
uri = URIUtils.rewriteURI(uri, null, true);
uri = URIUtils.rewriteURI(uri, null, URIUtils.DROP_FRAGMENT_AND_NORMALIZE);
} else {
uri = URIUtils.rewriteURI(uri);
}

View File

@ -60,12 +60,13 @@ public class RequestConfig implements Cloneable {
private final int connectTimeout;
private final int socketTimeout;
private final boolean contentCompressionEnabled;
private final boolean normalizeUri;
/**
* Intended for CDI compatibility
*/
protected RequestConfig() {
this(false, null, null, false, null, false, false, false, 0, false, null, null, 0, 0, 0, true);
this(false, null, null, false, null, false, false, false, 0, false, null, null, 0, 0, 0, true, true);
}
RequestConfig(
@ -84,7 +85,8 @@ public class RequestConfig implements Cloneable {
final int connectionRequestTimeout,
final int connectTimeout,
final int socketTimeout,
final boolean contentCompressionEnabled) {
final boolean contentCompressionEnabled,
final boolean normalizeUri) {
super();
this.expectContinueEnabled = expectContinueEnabled;
this.proxy = proxy;
@ -102,6 +104,7 @@ public class RequestConfig implements Cloneable {
this.connectTimeout = connectTimeout;
this.socketTimeout = socketTimeout;
this.contentCompressionEnabled = contentCompressionEnabled;
this.normalizeUri = normalizeUri;
}
/**
@ -332,6 +335,18 @@ public class RequestConfig implements Cloneable {
return contentCompressionEnabled;
}
/**
* Determines whether client should normalize URIs in requests or not.
* <p>
* Default: {@code true}
* </p>
*
* @since 4.5.8
*/
public boolean isNormalizeUri() {
return normalizeUri;
}
@Override
protected RequestConfig clone() throws CloneNotSupportedException {
return (RequestConfig) super.clone();
@ -356,6 +371,7 @@ public class RequestConfig implements Cloneable {
builder.append(", connectTimeout=").append(connectTimeout);
builder.append(", socketTimeout=").append(socketTimeout);
builder.append(", contentCompressionEnabled=").append(contentCompressionEnabled);
builder.append(", normalizeUri=").append(normalizeUri);
builder.append("]");
return builder.toString();
}
@ -404,6 +420,7 @@ public class RequestConfig implements Cloneable {
private int connectTimeout;
private int socketTimeout;
private boolean contentCompressionEnabled;
private boolean normalizeUri;
Builder() {
super();
@ -416,6 +433,7 @@ public class RequestConfig implements Cloneable {
this.connectTimeout = -1;
this.socketTimeout = -1;
this.contentCompressionEnabled = true;
this.normalizeUri = true;
}
public Builder setExpectContinueEnabled(final boolean expectContinueEnabled) {
@ -513,6 +531,11 @@ public class RequestConfig implements Cloneable {
return this;
}
public Builder setNormalizeUri(final boolean normalizeUri) {
this.normalizeUri = normalizeUri;
return this;
}
public RequestConfig build() {
return new RequestConfig(
expectContinueEnabled,
@ -530,7 +553,8 @@ public class RequestConfig implements Cloneable {
connectionRequestTimeout,
connectTimeout,
socketTimeout,
contentCompressionEnabled);
contentCompressionEnabled,
normalizeUri);
}
}

View File

@ -28,6 +28,7 @@ package org.apache.http.client.utils;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.EnumSet;
import java.util.List;
import java.util.Locale;
import java.util.Stack;
@ -45,6 +46,44 @@ import org.apache.http.util.TextUtils;
*/
public class URIUtils {
/**
* Flags that control how URI is being rewritten.
*
* @since 5.7.8
*/
public enum UriFlag {
DROP_FRAGMENT,
NORMALIZE
}
/**
* Empty set of uri flags.
*
* @since 5.7.8
*/
public static final EnumSet<UriFlag> NO_FLAGS = EnumSet.noneOf(UriFlag.class);
/**
* Set of uri flags containing {@link UriFlag#DROP_FRAGMENT}.
*
* @since 5.7.8
*/
public static final EnumSet<UriFlag> DROP_FRAGMENT = EnumSet.of(UriFlag.DROP_FRAGMENT);
/**
* Set of uri flags containing {@link UriFlag#NORMALIZE}.
*
* @since 5.7.8
*/
public static final EnumSet<UriFlag> NORMALIZE = EnumSet.of(UriFlag.NORMALIZE);
/**
* Set of uri flags containing {@link UriFlag#DROP_FRAGMENT} and {@link UriFlag#NORMALIZE}.
*
* @since 5.7.8
*/
public static final EnumSet<UriFlag> DROP_FRAGMENT_AND_NORMALIZE = EnumSet.of(UriFlag.DROP_FRAGMENT, UriFlag.NORMALIZE);
/**
* Constructs a {@link URI} using all the parameters. This should be
* used instead of
@ -125,12 +164,40 @@ public class URIUtils {
*
* @throws URISyntaxException
* If the resulting URI is invalid.
* @deprecated (5.7.8) Use {@link #rewriteURI(URI, HttpHost, EnumSet)}
*/
@Deprecated
public static URI rewriteURI(
final URI uri,
final HttpHost target,
final boolean dropFragment) throws URISyntaxException
{
return rewriteURI(uri, target, dropFragment ? DROP_FRAGMENT : NO_FLAGS);
}
/**
* A convenience method for creating a new {@link URI} whose scheme, host
* and port are taken from the target host, but whose path, query and
* fragment are taken from the existing URI. What exactly is used and how
* is driven by the passed in flags. The path is set to "/" if not explicitly specified.
*
* @param uri
* Contains the path, query and fragment to use.
* @param target
* Contains the scheme, host and port to use.
* @param flags
* True if the fragment should not be copied.
*
* @throws URISyntaxException
* If the resulting URI is invalid.
* @since 5.7.8
*/
public static URI rewriteURI(
final URI uri,
final HttpHost target,
final boolean dropFragment) throws URISyntaxException {
final EnumSet<UriFlag> flags) throws URISyntaxException {
Args.notNull(uri, "URI");
Args.notNull(flags, "URI flags");
if (uri.isOpaque()) {
return uri;
}
@ -144,36 +211,40 @@ public class URIUtils {
uribuilder.setHost(null);
uribuilder.setPort(-1);
}
if (dropFragment) {
if (flags.contains(UriFlag.DROP_FRAGMENT)) {
uribuilder.setFragment(null);
}
final String path = uribuilder.getPath();
if (TextUtils.isEmpty(path)) {
uribuilder.setPath("/");
} else {
final StringBuilder buf = new StringBuilder(path.length());
boolean foundSlash = false;
for (int i = 0; i < path.length(); i++) {
final char ch = path.charAt(i);
if (ch != '/' || !foundSlash) {
buf.append(ch);
if (flags.contains(UriFlag.NORMALIZE)) {
final StringBuilder buf = new StringBuilder(path.length());
boolean foundSlash = false;
for (int i = 0; i < path.length(); i++) {
final char ch = path.charAt(i);
if (ch != '/' || !foundSlash) {
buf.append(ch);
}
foundSlash = ch == '/';
}
foundSlash = ch == '/';
uribuilder.setPath(buf.toString());
} else {
uribuilder.setPath(path);
}
uribuilder.setPath(buf.toString());
}
return uribuilder.build();
}
/**
* A convenience method for
* {@link URIUtils#rewriteURI(URI, HttpHost, boolean)} that always keeps the
* {@link URIUtils#rewriteURI(URI, HttpHost, EnumSet)} that always keeps the
* fragment.
*/
public static URI rewriteURI(
final URI uri,
final HttpHost target) throws URISyntaxException {
return rewriteURI(uri, target, false);
return rewriteURI(uri, target, NORMALIZE);
}
/**
@ -217,7 +288,7 @@ public class URIUtils {
*
* @since 4.4
*/
public static URI rewriteURIForRoute(final URI uri, final RouteInfo route) throws URISyntaxException {
public static URI rewriteURIForRoute(final URI uri, final RouteInfo route, final boolean normalizeUri) throws URISyntaxException {
if (uri == null) {
return null;
}
@ -225,10 +296,10 @@ public class URIUtils {
// Make sure the request URI is absolute
return uri.isAbsolute()
? rewriteURI(uri)
: rewriteURI(uri, route.getTargetHost(), true);
: rewriteURI(uri, route.getTargetHost(), normalizeUri ? DROP_FRAGMENT_AND_NORMALIZE : DROP_FRAGMENT);
}
// Make sure the request URI is relative
return uri.isAbsolute() ? rewriteURI(uri, null, true) : rewriteURI(uri);
return uri.isAbsolute() ? rewriteURI(uri, null, normalizeUri ? DROP_FRAGMENT_AND_NORMALIZE : DROP_FRAGMENT) : rewriteURI(uri);
}
/**

View File

@ -146,6 +146,9 @@ public class DefaultRedirectStrategy implements RedirectStrategy {
final RequestConfig config = clientContext.getRequestConfig();
URI uri = createLocationURI(location);
if (config.isNormalizeUri()) {
uri = uri.normalize();
}
// rfc2616 demands the location value be a complete URI
// Location = "Location" ":" absoluteURI
@ -159,7 +162,8 @@ public class DefaultRedirectStrategy implements RedirectStrategy {
final HttpHost target = clientContext.getTargetHost();
Asserts.notNull(target, "Target host");
final URI requestURI = new URI(request.getRequestLine().getUri());
final URI absoluteRequestURI = URIUtils.rewriteURI(requestURI, target, false);
final URI absoluteRequestURI = URIUtils.rewriteURI(requestURI, target,
config.isNormalizeUri() ? URIUtils.NORMALIZE : URIUtils.NO_FLAGS);
uri = URIUtils.resolve(absoluteRequestURI, uri);
}
} catch (final URISyntaxException ex) {
@ -186,7 +190,7 @@ public class DefaultRedirectStrategy implements RedirectStrategy {
*/
protected URI createLocationURI(final String location) throws ProtocolException {
try {
final URIBuilder b = new URIBuilder(new URI(location).normalize());
final URIBuilder b = new URIBuilder(new URI(location));
final String host = b.getHost();
if (host != null) {
b.setHost(host.toLowerCase(Locale.ROOT));

View File

@ -69,6 +69,9 @@ import org.apache.http.protocol.RequestUserAgent;
import org.apache.http.util.Args;
import org.apache.http.util.VersionInfo;
import static org.apache.http.client.utils.URIUtils.DROP_FRAGMENT;
import static org.apache.http.client.utils.URIUtils.DROP_FRAGMENT_AND_NORMALIZE;
/**
* Request executor that implements the most fundamental aspects of
* the HTTP specification and the most straight-forward request / response
@ -112,13 +115,14 @@ public class MinimalClientExec implements ClientExecChain {
static void rewriteRequestURI(
final HttpRequestWrapper request,
final HttpRoute route) throws ProtocolException {
final HttpRoute route,
final boolean normalizeUri) throws ProtocolException {
try {
URI uri = request.getURI();
if (uri != null) {
// Make sure the request URI is relative
if (uri.isAbsolute()) {
uri = URIUtils.rewriteURI(uri, null, true);
uri = URIUtils.rewriteURI(uri, null, normalizeUri ? DROP_FRAGMENT_AND_NORMALIZE : DROP_FRAGMENT);
} else {
uri = URIUtils.rewriteURI(uri);
}
@ -139,7 +143,7 @@ public class MinimalClientExec implements ClientExecChain {
Args.notNull(request, "HTTP request");
Args.notNull(context, "HTTP context");
rewriteRequestURI(request, route);
rewriteRequestURI(request, route, context.getRequestConfig().isNormalizeUri());
final ConnectionRequest connRequest = connManager.requestConnection(route, null);
if (execAware != null) {

View File

@ -88,11 +88,12 @@ public class ProtocolExec implements ClientExecChain {
void rewriteRequestURI(
final HttpRequestWrapper request,
final HttpRoute route) throws ProtocolException {
final HttpRoute route,
final boolean normalizeUri) throws ProtocolException {
final URI uri = request.getURI();
if (uri != null) {
try {
request.setURI(URIUtils.rewriteURIForRoute(uri, route));
request.setURI(URIUtils.rewriteURIForRoute(uri, route, normalizeUri));
} catch (final URISyntaxException ex) {
throw new ProtocolException("Invalid URI: " + uri, ex);
}
@ -129,7 +130,7 @@ public class ProtocolExec implements ClientExecChain {
request.setURI(uri);
// Re-write request URI if needed
rewriteRequestURI(request, route);
rewriteRequestURI(request, route, context.getRequestConfig().isNormalizeUri());
final HttpParams params = request.getParams();
HttpHost virtualHost = (HttpHost) params.getParameter(ClientPNames.VIRTUAL_HOST);

View File

@ -34,6 +34,9 @@ import org.apache.http.conn.routing.HttpRoute;
import org.junit.Assert;
import org.junit.Test;
import static org.apache.http.client.utils.URIUtils.DROP_FRAGMENT_AND_NORMALIZE;
import static org.apache.http.client.utils.URIUtils.NORMALIZE;
/**
* This TestCase contains test methods for URI resolving according to RFC 3986.
* The examples are listed in section "5.4 Reference Resolution Examples"
@ -52,15 +55,15 @@ public class TestURIUtils {
Assert.assertEquals("/", URIUtils.rewriteURI(
URI.create("http://thishost//"), null).toString());
Assert.assertEquals("/stuff/morestuff", URIUtils.rewriteURI(
URI.create("http://thishost//stuff/morestuff"), null).toString());
URI.create("http://thishost//stuff///morestuff"), null).toString());
Assert.assertEquals("http://thathost/stuff", URIUtils.rewriteURI(
URI.create("http://thishost/stuff#crap"), target, true).toString());
URI.create("http://thishost/stuff#crap"), target, DROP_FRAGMENT_AND_NORMALIZE).toString());
Assert.assertEquals("http://thathost/stuff#crap", URIUtils.rewriteURI(
URI.create("http://thishost/stuff#crap"), target, false).toString());
URI.create("http://thishost/stuff#crap"), target, NORMALIZE).toString());
Assert.assertEquals("http://thathost/", URIUtils.rewriteURI(
URI.create("http://thishost#crap"), target, true).toString());
URI.create("http://thishost#crap"), target, DROP_FRAGMENT_AND_NORMALIZE).toString());
Assert.assertEquals("http://thathost/#crap", URIUtils.rewriteURI(
URI.create("http://thishost#crap"), target, false).toString());
URI.create("http://thishost#crap"), target, NORMALIZE).toString());
Assert.assertEquals("/stuff/", URIUtils.rewriteURI(
URI.create("http://thishost//////////////stuff/"), null).toString());
Assert.assertEquals("http://thathost/stuff", URIUtils.rewriteURI(
@ -84,21 +87,21 @@ public class TestURIUtils {
public void testRewritePort() throws Exception {
HttpHost target = new HttpHost("thathost", 8080); // port should be copied
Assert.assertEquals("http://thathost:8080/stuff", URIUtils.rewriteURI(
URI.create("http://thishost:80/stuff#crap"), target, true).toString());
URI.create("http://thishost:80/stuff#crap"), target, DROP_FRAGMENT_AND_NORMALIZE).toString());
Assert.assertEquals("http://thathost:8080/stuff#crap", URIUtils.rewriteURI(
URI.create("http://thishost:80/stuff#crap"), target, false).toString());
URI.create("http://thishost:80/stuff#crap"), target, NORMALIZE).toString());
target = new HttpHost("thathost", -1); // input port should be dropped
Assert.assertEquals("http://thathost/stuff", URIUtils.rewriteURI(
URI.create("http://thishost:80/stuff#crap"), target, true).toString());
URI.create("http://thishost:80/stuff#crap"), target, DROP_FRAGMENT_AND_NORMALIZE).toString());
Assert.assertEquals("http://thathost/stuff#crap", URIUtils.rewriteURI(
URI.create("http://thishost:80/stuff#crap"), target, false).toString());
URI.create("http://thishost:80/stuff#crap"), target, NORMALIZE).toString());
}
@Test
public void testRewriteScheme() throws Exception {
final HttpHost target = new HttpHost("thathost", -1, "file"); // scheme should be copied
Assert.assertEquals("file://thathost/stuff", URIUtils.rewriteURI(
URI.create("http://thishost:80/stuff#crap"), target, true).toString());
URI.create("http://thishost:80/stuff#crap"), target, DROP_FRAGMENT_AND_NORMALIZE).toString());
}
@Test
@ -110,23 +113,23 @@ public class TestURIUtils {
// Direct route
Assert.assertEquals(new URI("/test"), URIUtils
.rewriteURIForRoute(new URI("http://foo/test"), new HttpRoute(target1)));
.rewriteURIForRoute(new URI("http://foo/test"), new HttpRoute(target1), true));
// Direct route
Assert.assertEquals(new URI("/"), URIUtils
.rewriteURIForRoute(new URI(""), new HttpRoute(target1)));
.rewriteURIForRoute(new URI(""), new HttpRoute(target1), true));
// Via proxy
Assert.assertEquals(new URI("http://foo/test"), URIUtils
.rewriteURIForRoute(new URI("http://foo/test"), new HttpRoute(target1, proxy)));
.rewriteURIForRoute(new URI("http://foo/test"), new HttpRoute(target1, proxy), true));
// Via proxy
Assert.assertEquals(new URI("http://foo:80/test"), URIUtils
.rewriteURIForRoute(new URI("/test"), new HttpRoute(target1, proxy)));
.rewriteURIForRoute(new URI("/test"), new HttpRoute(target1, proxy), true));
// Via proxy tunnel
Assert.assertEquals(new URI("/test"), URIUtils
.rewriteURIForRoute(new URI("https://foo:443/test"), new HttpRoute(target2, null, proxy, true)));
.rewriteURIForRoute(new URI("https://foo:443/test"), new HttpRoute(target2, null, proxy, true), true));
}
@Test

View File

@ -398,4 +398,15 @@ public class TestDefaultRedirectStrategy {
redirectStrategy.createLocationURI(":::::::");
}
@Test
public void testWithoutNormalize() throws Exception {
final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
final HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_TEMPORARY_REDIRECT, "Temporary Redirect");
response.addHeader("Location", "http://somewhere.com//foo");
final HttpClientContext context1 = new HttpClientContext(new BasicHttpContext());
context1.setRequestConfig(RequestConfig.custom().setNormalizeUri(false).build());
Assert.assertEquals("http://somewhere.com//foo", redirectStrategy.getRedirect(
new HttpGet("http://localhost/stuff"), response, context1).getURI().toASCIIString());
}
}

View File

@ -104,7 +104,7 @@ public class TestProtocolExec {
final HttpRoute route = new HttpRoute(target);
final HttpRequestWrapper request = HttpRequestWrapper.wrap(
new HttpGet("http://foo/test"));
protocolExec.rewriteRequestURI(request, route);
protocolExec.rewriteRequestURI(request, route, true);
Assert.assertEquals(new URI("/test"), request.getURI());
}
@ -113,7 +113,7 @@ public class TestProtocolExec {
final HttpRoute route = new HttpRoute(target);
final HttpRequestWrapper request = HttpRequestWrapper.wrap(
new HttpGet(""));
protocolExec.rewriteRequestURI(request, route);
protocolExec.rewriteRequestURI(request, route, true);
Assert.assertEquals(new URI("/"), request.getURI());
}
@ -122,7 +122,7 @@ public class TestProtocolExec {
final HttpRoute route = new HttpRoute(target, proxy);
final HttpRequestWrapper request = HttpRequestWrapper.wrap(
new HttpGet("http://foo/test"));
protocolExec.rewriteRequestURI(request, route);
protocolExec.rewriteRequestURI(request, route, true);
Assert.assertEquals(new URI("http://foo/test"), request.getURI());
}
@ -131,7 +131,7 @@ public class TestProtocolExec {
final HttpRoute route = new HttpRoute(target, proxy);
final HttpRequestWrapper request = HttpRequestWrapper.wrap(
new HttpGet("/test"));
protocolExec.rewriteRequestURI(request, route);
protocolExec.rewriteRequestURI(request, route, true);
Assert.assertEquals(new URI("http://foo:80/test"), request.getURI());
}