Repository: ant-ivy
Updated Branches:
  refs/heads/master 6607bdcb3 -> cdc548b80


IVY-1563 Move to 4.x version of HttpComponents HttpClient library


Project: http://git-wip-us.apache.org/repos/asf/ant-ivy/repo
Commit: http://git-wip-us.apache.org/repos/asf/ant-ivy/commit/cdc548b8
Tree: http://git-wip-us.apache.org/repos/asf/ant-ivy/tree/cdc548b8
Diff: http://git-wip-us.apache.org/repos/asf/ant-ivy/diff/cdc548b8

Branch: refs/heads/master
Commit: cdc548b80ef6cf8e52b74b0b073acc8fd3b4996a
Parents: 6607bdc
Author: Jaikiran Pai <jaiki...@apache.org>
Authored: Mon Jul 24 16:46:52 2017 +0530
Committer: Jaikiran Pai <jaiki...@apache.org>
Committed: Mon Jul 24 18:15:53 2017 +0530

----------------------------------------------------------------------
 asciidoc/use/settings.adoc                      |   2 +-
 ivy.xml                                         |   6 +-
 .../apache/ivy/util/url/AbstractURLHandler.java |   6 +-
 .../apache/ivy/util/url/HttpClientHandler.java  | 536 ++++++++-----------
 .../apache/ivy/util/url/URLHandlerRegistry.java |  34 +-
 .../ivy/util/url/HttpClientURLHandlerTest.java  | 110 ++++
 .../ivy/util/url/HttpclientURLHandlerTest.java  |  28 +-
 7 files changed, 376 insertions(+), 346 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ant-ivy/blob/cdc548b8/asciidoc/use/settings.adoc
----------------------------------------------------------------------
diff --git a/asciidoc/use/settings.adoc b/asciidoc/use/settings.adoc
index 4c98e07..319abe1 100644
--- a/asciidoc/use/settings.adoc
+++ b/asciidoc/use/settings.adoc
@@ -55,7 +55,7 @@ my.variable.yourid=your.value
 
 == HTTP Authentication
 
-__Note__: HTTP Authentication can be used only if 
link:http://jakarta.apache.org/commons/httpclient/[commons-httpclient.jar] is 
in your classpath
+__Note__: HTTP Authentication can be used only if 
link:https://hc.apache.org/httpcomponents-client-ga/index.html[HttpComponents 
HttpClient library] (minimum of 4.5.3 version) and its 
linke:https://hc.apache.org/httpcomponents-client-4.5.x/dependency-management.html[dependencies]
 are in your classpath.
 
 If any of the url you use in Ivy (especially in dependency resolvers) need 
HTTP authentication, then you have to provide the `host`, `realm`, `username` 
and `passwd` attributes of the configure task. These settings will then be used 
in any further call to ivy tasks.
 

http://git-wip-us.apache.org/repos/asf/ant-ivy/blob/cdc548b8/ivy.xml
----------------------------------------------------------------------
diff --git a/ivy.xml b/ivy.xml
index d7417c8..f38c1e5 100644
--- a/ivy.xml
+++ b/ivy.xml
@@ -44,7 +44,7 @@
     </publications>
     <dependencies>
         <dependency org="org.apache.ant" name="ant" rev="1.9.9" 
conf="default,ant"/>
-        <dependency org="commons-httpclient" name="commons-httpclient" 
rev="3.1" conf="default,httpclient->runtime,master"/>
+        <dependency org="org.apache.httpcomponents" name="httpclient" 
rev="4.5.3" conf="default,httpclient->runtime,master"/>
         <dependency org="oro" name="oro" rev="2.0.8" conf="default,oro"/>
         <dependency org="commons-vfs" name="commons-vfs" rev="1.0" 
conf="default,vfs"/>
         <dependency org="com.jcraft" name="jsch" rev="0.1.54" 
conf="default,sftp"/>
@@ -62,8 +62,10 @@
         <dependency org="ant-contrib" name="ant-contrib" rev="1.0b3" 
conf="test" transitive="false"/>
         <dependency org="xmlunit" name="xmlunit" rev="1.6" conf="test" 
transitive="false"/>
 
-        <!-- Global exclude for junit and hamcrest -->
+        <!-- Global excludes -->
         <exclude org="junit" module="junit" 
conf="core,default,httpclient,oro,vfs,sftp,standalone,ant"/>
         <exclude org="org.hamcrest" module="hamcrest-core" 
conf="core,default,httpclient,oro,vfs,sftp,standalone,ant"/>
+        <!-- Exclude the whole outdated commons-httpclient org -->
+        <exclude org="commons-httpclient" conf="*"/>
     </dependencies>
 </ivy-module>

http://git-wip-us.apache.org/repos/asf/ant-ivy/blob/cdc548b8/src/java/org/apache/ivy/util/url/AbstractURLHandler.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/ivy/util/url/AbstractURLHandler.java 
b/src/java/org/apache/ivy/util/url/AbstractURLHandler.java
index a1184d2..c951ff1 100644
--- a/src/java/org/apache/ivy/util/url/AbstractURLHandler.java
+++ b/src/java/org/apache/ivy/util/url/AbstractURLHandler.java
@@ -86,11 +86,7 @@ public abstract class AbstractURLHandler implements 
URLHandler {
     }
 
     protected String getUserAgent() {
-        String userAgent = System.getProperty("http.agent");
-        if (userAgent == null) {
-            userAgent = "Apache Ivy/" + Ivy.getIvyVersion();
-        }
-        return userAgent;
+        return System.getProperty("http.agent", "Apache Ivy/" + 
Ivy.getIvyVersion());
     }
 
     protected void validatePutStatusCode(URL dest, int statusCode, String 
statusMessage)

http://git-wip-us.apache.org/repos/asf/ant-ivy/blob/cdc548b8/src/java/org/apache/ivy/util/url/HttpClientHandler.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/ivy/util/url/HttpClientHandler.java 
b/src/java/org/apache/ivy/util/url/HttpClientHandler.java
index 469ad0c..04d0008 100644
--- a/src/java/org/apache/ivy/util/url/HttpClientHandler.java
+++ b/src/java/org/apache/ivy/util/url/HttpClientHandler.java
@@ -17,24 +17,34 @@
  */
 package org.apache.ivy.util.url;
 
-import org.apache.commons.httpclient.Credentials;
-import org.apache.commons.httpclient.Header;
-import org.apache.commons.httpclient.HttpClient;
-import org.apache.commons.httpclient.HttpException;
-import org.apache.commons.httpclient.HttpMethodBase;
-import org.apache.commons.httpclient.HttpStatus;
-import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager;
-import org.apache.commons.httpclient.NTCredentials;
-import org.apache.commons.httpclient.auth.AuthPolicy;
-import org.apache.commons.httpclient.auth.AuthScheme;
-import org.apache.commons.httpclient.auth.AuthScope;
-import org.apache.commons.httpclient.auth.CredentialsNotAvailableException;
-import org.apache.commons.httpclient.auth.CredentialsProvider;
-import org.apache.commons.httpclient.methods.GetMethod;
-import org.apache.commons.httpclient.methods.HeadMethod;
-import org.apache.commons.httpclient.methods.PutMethod;
-import org.apache.commons.httpclient.methods.RequestEntity;
-import org.apache.commons.httpclient.params.HttpMethodParams;
+import org.apache.http.Header;
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
+import org.apache.http.auth.AuthSchemeProvider;
+import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.Credentials;
+import org.apache.http.auth.NTCredentials;
+import org.apache.http.client.CredentialsProvider;
+import org.apache.http.client.config.AuthSchemes;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpHead;
+import org.apache.http.client.methods.HttpPut;
+import org.apache.http.config.Lookup;
+import org.apache.http.config.RegistryBuilder;
+import org.apache.http.conn.HttpClientConnectionManager;
+import org.apache.http.conn.routing.HttpRoutePlanner;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.FileEntity;
+import org.apache.http.impl.auth.BasicSchemeFactory;
+import org.apache.http.impl.auth.DigestSchemeFactory;
+import org.apache.http.impl.auth.NTLMSchemeFactory;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClients;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.http.impl.conn.SystemDefaultRoutePlanner;
 import org.apache.ivy.core.settings.TimeoutConstraint;
 import org.apache.ivy.util.CopyProgressListener;
 import org.apache.ivy.util.FileUtil;
@@ -42,56 +52,81 @@ import org.apache.ivy.util.HostUtil;
 import org.apache.ivy.util.Message;
 
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.OutputStream;
+import java.net.ProxySelector;
 import java.net.URL;
-import java.net.UnknownHostException;
+import java.nio.charset.Charset;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
-import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Locale;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  *
  */
-public class HttpClientHandler extends AbstractURLHandler {
+class HttpClientHandler extends AbstractURLHandler implements AutoCloseable {
     private static final SimpleDateFormat LAST_MODIFIED_FORMAT = new 
SimpleDateFormat(
             "EEE, d MMM yyyy HH:mm:ss z", Locale.US);
 
-    // proxy configuration: obtain from system properties
-    private int proxyPort;
+    // A instance of the HttpClientHandler which gets registered to be closed
+    // when the JVM exits
+    static final HttpClientHandler DELETE_ON_EXIT_INSTANCE;
 
-    private String proxyHost = null;
+    static {
+        DELETE_ON_EXIT_INSTANCE = new HttpClientHandler();
+        final Thread shutdownHook = new Thread(new Runnable() {
+            @Override
+            public void run() {
+                try {
+                    DELETE_ON_EXIT_INSTANCE.close();
+                } catch (Exception e) {
+                    // ignore since this is anyway happening during shutdown 
of the JVM
+                }
+            }
+        });
+        shutdownHook.setName("ivy-httpclient-shutdown-handler");
+        shutdownHook.setDaemon(true);
+        Runtime.getRuntime().addShutdownHook(shutdownHook);
+    }
 
-    private String proxyUserName = null;
+    private final CloseableHttpClient httpClient;
 
-    private String proxyPasswd = null;
+    HttpClientHandler() {
+        this.httpClient = buildUnderlyingClient();
+    }
 
-    private HttpClientHelper httpClientHelper;
+    private CloseableHttpClient buildUnderlyingClient() {
+        return HttpClients.custom()
+                .setConnectionManager(createConnectionManager())
+                .setRoutePlanner(createProxyRoutePlanner())
+                .setUserAgent(this.getUserAgent())
+                .setDefaultAuthSchemeRegistry(createAuthSchemeRegistry())
+                .setDefaultCredentialsProvider(new IvyCredentialsProvider())
+                .build();
+    }
 
-    private static HttpClient httpClient;
+    private static HttpRoutePlanner createProxyRoutePlanner() {
+        // use the standard JRE ProxySelector to get proxy information
+        Message.verbose("Using JRE standard ProxySelector for configuring HTTP 
proxy");
+        return new SystemDefaultRoutePlanner(ProxySelector.getDefault());
+    }
 
-    public HttpClientHandler() {
-        configureProxy();
+    private static Lookup<AuthSchemeProvider> createAuthSchemeRegistry() {
+        return 
RegistryBuilder.<AuthSchemeProvider>create().register(AuthSchemes.DIGEST, new 
DigestSchemeFactory())
+                .register(AuthSchemes.BASIC, new BasicSchemeFactory())
+                .register(AuthSchemes.NTLM, new NTLMSchemeFactory())
+                .build();
     }
 
-    private void configureProxy() {
-        proxyHost = System.getProperty("http.proxyHost");
-        // TODO constant is better ...
-        if (useProxy()) {
-            proxyPort = Integer.parseInt(System.getProperty("http.proxyPort", 
"80"));
-            proxyUserName = System.getProperty("http.proxyUser");
-            proxyPasswd = System.getProperty("http.proxyPassword");
-            // It seems there is no equivalent in HttpClient for
-            // 'http.nonProxyHosts' property
-            Message.verbose("proxy configured: host=" + proxyHost + " port=" + 
proxyPort + " user="
-                    + proxyUserName);
-        } else {
-            Message.verbose("no proxy configured");
-        }
+    private static HttpClientConnectionManager createConnectionManager() {
+        return new PoolingHttpClientConnectionManager();
+    }
+
+    private static List<String> getAuthSchemePreferredOrder() {
+        return Arrays.asList(AuthSchemes.DIGEST, AuthSchemes.BASIC, 
AuthSchemes.NTLM);
     }
 
     @Override
@@ -103,16 +138,10 @@ public class HttpClientHandler extends AbstractURLHandler 
{
     public InputStream openStream(final URL url, final TimeoutConstraint 
timeoutConstraint) throws IOException {
         final int connectionTimeout = (timeoutConstraint == null || 
timeoutConstraint.getConnectionTimeout() < 0) ? 0 : 
timeoutConstraint.getConnectionTimeout();
         final int readTimeout = (timeoutConstraint == null || 
timeoutConstraint.getReadTimeout() < 0) ? 0 : 
timeoutConstraint.getReadTimeout();
-        final GetMethod get = doGet(url, connectionTimeout, readTimeout);
-        if (!checkStatusCode(url, get)) {
-            get.releaseConnection();
-            throw new IOException("The HTTP response code for " + url
-                    + " did not indicate a success." + " See log for more 
detail.");
-        }
-
-        Header encoding = get.getResponseHeader("Content-Encoding");
-        return getDecodingInputStream(encoding == null ? null : 
encoding.getValue(),
-                get.getResponseBodyAsStream());
+        final CloseableHttpResponse response = doGet(url, connectionTimeout, 
readTimeout);
+        this.requireSuccessStatus(HttpGet.METHOD_NAME, url, response);
+        final Header encoding = this.getContentEncoding(response);
+        return getDecodingInputStream(encoding == null ? null : 
encoding.getValue(), response.getEntity().getContent());
     }
 
     @Override
@@ -126,21 +155,15 @@ public class HttpClientHandler extends AbstractURLHandler 
{
 
         final int connectionTimeout = (timeoutConstraint == null || 
timeoutConstraint.getConnectionTimeout() < 0) ? 0 : 
timeoutConstraint.getConnectionTimeout();
         final int readTimeout = (timeoutConstraint == null || 
timeoutConstraint.getReadTimeout() < 0) ? 0 : 
timeoutConstraint.getReadTimeout();
-        final GetMethod get = doGet(src, connectionTimeout, readTimeout);
-        try {
+        try (final CloseableHttpResponse response = doGet(src, 
connectionTimeout, readTimeout)) {
             // We can only figure the content we got is want we want if the 
status is success.
-            if (!checkStatusCode(src, get)) {
-                throw new IOException("The HTTP response code for " + src
-                        + " did not indicate a success." + " See log for more 
detail.");
+            this.requireSuccessStatus(HttpGet.METHOD_NAME, src, response);
+            final Header encoding = this.getContentEncoding(response);
+            try (final InputStream is = getDecodingInputStream(encoding == 
null ? null : encoding.getValue(),
+                    response.getEntity().getContent())) {
+                FileUtil.copy(is, dest, listener);
             }
-
-            Header encoding = get.getResponseHeader("Content-Encoding");
-            InputStream is = getDecodingInputStream(encoding == null ? null : 
encoding.getValue(),
-                    get.getResponseBodyAsStream());
-            FileUtil.copy(is, dest, listener);
-            dest.setLastModified(getLastModified(get));
-        } finally {
-            get.releaseConnection();
+            dest.setLastModified(getLastModified(response));
         }
     }
 
@@ -152,22 +175,20 @@ public class HttpClientHandler extends AbstractURLHandler 
{
     @Override
     public void upload(final File src, final URL dest, final 
CopyProgressListener listener, final TimeoutConstraint timeoutConstraint) 
throws IOException {
         final int connectionTimeout = (timeoutConstraint == null || 
timeoutConstraint.getConnectionTimeout() < 0) ? 0 : 
timeoutConstraint.getConnectionTimeout();
-        final HttpClient client = getClient();
-        // TODO: Use the newer way of settings the connection timeout via 
HttpConnectionManagerParams
-        // once we stop support for HttpClient 2.x version
-        client.setConnectionTimeout(connectionTimeout);
-
-        final PutMethod put = new PutMethod(normalizeToString(dest));
-        put.setDoAuthentication(useAuthentication(dest) || 
useProxyAuthentication());
-        put.getParams().setBooleanParameter("http.protocol.expect-continue", 
true);
-        try {
-            put.setRequestEntity(new FileRequestEntity(src));
-            int statusCode = client.executeMethod(put);
-            validatePutStatusCode(dest, statusCode, null);
-        } finally {
-            put.releaseConnection();
+        final int readTimeout = (timeoutConstraint == null || 
timeoutConstraint.getReadTimeout() < 0) ? 0 : 
timeoutConstraint.getReadTimeout();
+        final RequestConfig requestConfig = 
RequestConfig.custom().setSocketTimeout(readTimeout)
+                .setConnectTimeout(connectionTimeout)
+                .setAuthenticationEnabled(hasCredentialsConfigured(dest))
+                .setTargetPreferredAuthSchemes(getAuthSchemePreferredOrder())
+                .setProxyPreferredAuthSchemes(getAuthSchemePreferredOrder())
+                .setExpectContinueEnabled(true)
+                .build();
+        final HttpPut put = new HttpPut(normalizeToString(dest));
+        put.setConfig(requestConfig);
+        put.setEntity(new FileEntity(src));
+        try (final CloseableHttpResponse response = 
this.httpClient.execute(put)) {
+            validatePutStatusCode(dest, 
response.getStatusLine().getStatusCode(), 
response.getStatusLine().getReasonPhrase());
         }
-
     }
 
     @Override
@@ -184,289 +205,182 @@ public class HttpClientHandler extends 
AbstractURLHandler {
     public URLInfo getURLInfo(final URL url, final TimeoutConstraint 
timeoutConstraint) {
         final int connectionTimeout = (timeoutConstraint == null || 
timeoutConstraint.getConnectionTimeout() < 0) ? 0 : 
timeoutConstraint.getConnectionTimeout();
         final int readTimeout = (timeoutConstraint == null || 
timeoutConstraint.getReadTimeout() < 0) ? 0 : 
timeoutConstraint.getReadTimeout();
-        HttpMethodBase method = null;
+        CloseableHttpResponse response = null;
         try {
+            final String httpMethod;
             if (getRequestMethod() == URLHandler.REQUEST_METHOD_HEAD) {
-                method = doHead(url, connectionTimeout, readTimeout);
+                httpMethod = HttpHead.METHOD_NAME;
+                response = doHead(url, connectionTimeout, readTimeout);
             } else {
-                method = doGet(url, connectionTimeout, readTimeout);
+                httpMethod = HttpGet.METHOD_NAME;
+                response = doGet(url, connectionTimeout, readTimeout);
             }
-            if (checkStatusCode(url, method)) {
-                return new URLInfo(true, getResponseContentLength(method), 
getLastModified(method),
-                        method.getRequestCharSet());
+            if (checkStatusCode(httpMethod, url, response)) {
+                final HttpEntity responseEntity = response.getEntity();
+                final Charset charSet = 
ContentType.getOrDefault(responseEntity).getCharset();
+                return new URLInfo(true, responseEntity == null ? 0 : 
responseEntity.getContentLength(),
+                        getLastModified(response), charSet.name());
             }
-        } catch (HttpException e) {
-            Message.error("HttpClientHandler: " + e.getMessage() + ":" + 
e.getReasonCode() + "="
-                    + e.getReason() + " url=" + url);
-        } catch (UnknownHostException e) {
-            Message.warn("Host " + e.getMessage() + " not found. url=" + url);
-            Message.info("You probably access the destination server through "
-                    + "a proxy server that is not well configured.");
-        } catch (IOException e) {
+        } catch (IOException | IllegalArgumentException e) {
+            // IllegalArgumentException is thrown by HttpClient library to 
indicate the URL is not valid,
+            // this happens for instance when trying to download a dynamic 
version (cfr IVY-390)
             Message.error("HttpClientHandler: " + e.getMessage() + " url=" + 
url);
-        } catch (IllegalArgumentException e) {
-            // thrown by HttpClient to indicate the URL is not valid, this 
happens for instance
-            // when trying to download a dynamic version (cfr IVY-390)
         } finally {
-            if (method != null) {
-                method.releaseConnection();
+            if (response != null) {
+                try {
+                    response.close();
+                } catch (IOException e) {
+                    // ignore
+                }
             }
         }
         return UNAVAILABLE;
     }
 
-    private boolean checkStatusCode(URL url, HttpMethodBase method) {
-        int status = method.getStatusCode();
+    private boolean checkStatusCode(final String httpMethod, final URL 
sourceURL, final HttpResponse response) {
+        final int status = response.getStatusLine().getStatusCode();
         if (status == HttpStatus.SC_OK) {
             return true;
         }
-
         // IVY-1328: some servers return a 204 on a HEAD request
-        if ("HEAD".equals(method.getName()) && (status == 204)) {
+        if (HttpHead.METHOD_NAME.equals(httpMethod) && (status == 204)) {
             return true;
         }
 
-        Message.debug("HTTP response status: " + status + " url=" + url);
+        Message.debug("HTTP response status: " + status + " url=" + sourceURL);
         if (status == HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED) {
             Message.warn("Your proxy requires authentication.");
         } else if (String.valueOf(status).startsWith("4")) {
-            Message.verbose("CLIENT ERROR: " + method.getStatusText() + " 
url=" + url);
+            Message.verbose("CLIENT ERROR: " + 
response.getStatusLine().getReasonPhrase() + " url=" + sourceURL);
         } else if (String.valueOf(status).startsWith("5")) {
-            Message.error("SERVER ERROR: " + method.getStatusText() + " url=" 
+ url);
+            Message.error("SERVER ERROR: " + 
response.getStatusLine().getReasonPhrase() + " url=" + sourceURL);
         }
-
         return false;
     }
 
-    private long getLastModified(HttpMethodBase method) {
-        Header header = method.getResponseHeader("last-modified");
-        if (header != null) {
-            String lastModified = header.getValue();
-            try {
-                return LAST_MODIFIED_FORMAT.parse(lastModified).getTime();
-            } catch (ParseException e) {
-                // ignored
-            }
-            return System.currentTimeMillis();
-        } else {
-            return System.currentTimeMillis();
+    /**
+     * Checks the status code of the response and if it's considered as 
successful response, then this method just returns back.
+     * Else it {@link CloseableHttpResponse#close() closes the response} and 
throws an {@link IOException} for the unsuccessful response.
+     *
+     * @param httpMethod The HTTP method that was used for the source request
+     * @param sourceURL  The URL of the source request
+     * @param response   The response to the source request
+     * @throws IOException Thrown if the response was considered unsuccessful
+     */
+    private void requireSuccessStatus(final String httpMethod, final URL 
sourceURL, final CloseableHttpResponse response) throws IOException {
+        if (this.checkStatusCode(httpMethod, sourceURL, response)) {
+            return;
         }
-    }
-
-    private long getResponseContentLength(HttpMethodBase head) {
-        return getHttpClientHelper().getResponseContentLength(head);
-    }
-
-    private HttpClientHelper getHttpClientHelper() {
-        if (httpClientHelper == null) {
-            // use commons httpclient 3.0 if available
-            try {
-                HttpMethodBase.class.getMethod("getResponseContentLength");
-                httpClientHelper = new HttpClientHelper3x();
-                Message.verbose("using commons httpclient 3.x helper");
-            } catch (SecurityException e) {
-                Message.verbose("unable to get access to 
getResponseContentLength of "
-                        + "commons-httpclient HeadMethod. Please use 
commons-httpclient 3.0 or "
-                        + "use ivy with sufficient security permissions.");
-                Message.verbose("exception: " + e.getMessage());
-                httpClientHelper = new HttpClientHelper2x();
-                Message.verbose("using commons httpclient 2.x helper");
-            } catch (NoSuchMethodException e) {
-                httpClientHelper = new HttpClientHelper2x();
-                Message.verbose("using commons httpclient 2.x helper");
-            }
+        // this is now considered an unsuccessful response, so close the 
response and throw an exception
+        try {
+            response.close();
+        } catch (Exception e) {
+            // log and move on
+            Message.debug("Could not close the HTTP response for url=" + 
sourceURL, e);
         }
-        return httpClientHelper;
+        throw new IOException("Response to request '" + httpMethod + " " + 
sourceURL + "' did not indicate a success (see debug log for details)");
     }
 
-    public int getHttpClientMajorVersion() {
-        HttpClientHelper helper = getHttpClientHelper();
-        return helper.getHttpClientMajorVersion();
+    private Header getContentEncoding(final HttpResponse response) {
+        return response.getFirstHeader("Content-Encoding");
     }
 
-    private GetMethod doGet(final URL url, final int connectionTimeout, final 
int readTimeout) throws IOException {
-        final HttpClient client = getClient();
-        // TODO: Use the newer way of settings the connection and read timeout 
via HttpConnectionManagerParams
-        // once we stop support for HttpClient 2.x version
-        client.setConnectionTimeout(connectionTimeout);
-        client.setTimeout(readTimeout);
-
-        GetMethod get = new GetMethod(normalizeToString(url));
-        get.setDoAuthentication(useAuthentication(url) || 
useProxyAuthentication());
-        get.setRequestHeader("Accept-Encoding", "gzip,deflate");
-        client.executeMethod(get);
-        return get;
-    }
-
-    private HeadMethod doHead(final URL url, final int connectionTimeout, 
final int readTimeout) throws IOException {
-        final HttpClient client = getClient();
-        // TODO: Use the newer way of settings the connection and read timeout 
via HttpConnectionManagerParams
-        // once we stop support for HttpClient 2.x version
-        client.setConnectionTimeout(connectionTimeout);
-        client.setTimeout(readTimeout);
-
-        HeadMethod head = new HeadMethod(normalizeToString(url));
-        head.setDoAuthentication(useAuthentication(url) || 
useProxyAuthentication());
-        client.executeMethod(head);
-        return head;
-    }
-
-    private HttpClient getClient() {
-        if (httpClient == null) {
-            final MultiThreadedHttpConnectionManager connManager = new 
MultiThreadedHttpConnectionManager();
-            httpClient = new HttpClient(connManager);
-
-            Runtime.getRuntime().addShutdownHook(new Thread(new Runnable() {
-                public void run() {
-                    connManager.shutdown();
-                }
-            }));
-
-            List<String> authPrefs = new ArrayList<>(3);
-            authPrefs.add(AuthPolicy.DIGEST);
-            authPrefs.add(AuthPolicy.BASIC);
-            authPrefs.add(AuthPolicy.NTLM); // put it at the end to give less 
priority (IVY-213)
-            
httpClient.getParams().setParameter(AuthPolicy.AUTH_SCHEME_PRIORITY, authPrefs);
-
-            if (useProxy()) {
-                httpClient.getHostConfiguration().setProxy(proxyHost, 
proxyPort);
-                if (useProxyAuthentication()) {
-                    httpClient.getState().setProxyCredentials(
-                            new AuthScope(proxyHost, proxyPort, 
AuthScope.ANY_REALM),
-                            createCredentials(proxyUserName, proxyPasswd));
-                }
-            }
-
-            // user-agent
-            httpClient.getParams().setParameter(HttpMethodParams.USER_AGENT,
-                    getUserAgent());
-
-            // authentication
-            httpClient.getParams().setParameter(CredentialsProvider.PROVIDER,
-                    new IvyCredentialsProvider());
+    private long getLastModified(final HttpResponse response) {
+        final Header header = response.getFirstHeader("last-modified");
+        if (header == null) {
+            return System.currentTimeMillis();
         }
-
-        return httpClient;
+        final String lastModified = header.getValue();
+        try {
+            return LAST_MODIFIED_FORMAT.parse(lastModified).getTime();
+        } catch (ParseException e) {
+            // ignored
+        }
+        return System.currentTimeMillis();
     }
 
-    private boolean useProxy() {
-        return proxyHost != null && proxyHost.trim().length() > 0;
+    private CloseableHttpResponse doGet(final URL url, final int 
connectionTimeout, final int readTimeout) throws IOException {
+        final RequestConfig requestConfig = 
RequestConfig.custom().setSocketTimeout(readTimeout)
+                .setConnectTimeout(connectionTimeout)
+                .setAuthenticationEnabled(hasCredentialsConfigured(url))
+                .setTargetPreferredAuthSchemes(getAuthSchemePreferredOrder())
+                .setProxyPreferredAuthSchemes(getAuthSchemePreferredOrder())
+                .build();
+        final HttpGet httpGet = new HttpGet(normalizeToString(url));
+        httpGet.setConfig(requestConfig);
+        httpGet.addHeader("Accept-Encoding", "gzip,deflate");
+        return this.httpClient.execute(httpGet);
     }
 
-    private boolean useAuthentication(URL url) {
-        return CredentialsStore.INSTANCE.hasCredentials(url.getHost());
+    private CloseableHttpResponse doHead(final URL url, final int 
connectionTimeout, final int readTimeout) throws IOException {
+        final RequestConfig requestConfig = 
RequestConfig.custom().setSocketTimeout(readTimeout)
+                .setConnectTimeout(connectionTimeout)
+                .setAuthenticationEnabled(hasCredentialsConfigured(url))
+                .setTargetPreferredAuthSchemes(getAuthSchemePreferredOrder())
+                .setProxyPreferredAuthSchemes(getAuthSchemePreferredOrder())
+                .build();
+        final HttpHead httpHead = new HttpHead(normalizeToString(url));
+        httpHead.setConfig(requestConfig);
+        return this.httpClient.execute(httpHead);
     }
 
-    private boolean useProxyAuthentication() {
-        return (proxyUserName != null && proxyUserName.trim().length() > 0);
-    }
-
-    private static final class HttpClientHelper3x implements HttpClientHelper {
-        private static final int VERSION = 3;
-
-        private HttpClientHelper3x() {
-        }
-
-        public long getResponseContentLength(HttpMethodBase method) {
-            return method.getResponseContentLength();
-        }
-
-        /**
-         * {@inheritDoc}
-         */
-        public int getHttpClientMajorVersion() {
-            return VERSION;
-        }
+    private boolean hasCredentialsConfigured(final URL url) {
+        return CredentialsStore.INSTANCE.hasCredentials(url.getHost());
     }
 
-    private static final class HttpClientHelper2x implements HttpClientHelper {
-        private static final int VERSION = 2;
-
-        private HttpClientHelper2x() {
-        }
-
-        public long getResponseContentLength(HttpMethodBase method) {
-            Header header = method.getResponseHeader("Content-Length");
-            if (header != null) {
-                try {
-                    return Integer.parseInt(header.getValue());
-                } catch (NumberFormatException e) {
-                    Message.verbose("Invalid content-length value: " + 
e.getMessage());
-                }
-            }
-            return 0;
-        }
-
-        /**
-         * {@inheritDoc}
-         */
-        public int getHttpClientMajorVersion() {
-            return VERSION;
+    @Override
+    public void close() throws Exception {
+        if (this.httpClient != null) {
+            this.httpClient.close();
         }
     }
 
-    public interface HttpClientHelper {
-        long getResponseContentLength(HttpMethodBase method);
-
-        int getHttpClientMajorVersion();
-    }
-
     private static class IvyCredentialsProvider implements CredentialsProvider 
{
 
-        public Credentials getCredentials(AuthScheme scheme, String host, int 
port, boolean proxy)
-                throws CredentialsNotAvailableException {
-            String realm = scheme.getRealm();
+        private final ConcurrentHashMap<AuthScope, Credentials> cachedCreds = 
new ConcurrentHashMap<>();
 
-            org.apache.ivy.util.Credentials c = 
CredentialsStore.INSTANCE.getCredentials(realm,
-                    host);
-            if (c != null) {
-                return createCredentials(c.getUserName(), c.getPasswd());
+        @Override
+        public void setCredentials(final AuthScope authscope, final 
Credentials credentials) {
+            if (authscope == null) {
+                throw new IllegalArgumentException("AuthScope cannot be null");
             }
-
-            return null;
+            this.cachedCreds.put(authscope, credentials);
         }
-    }
-
-    private static Credentials createCredentials(String username, String 
password) {
-        String user;
-        String domain;
-
-        int backslashIndex = username.indexOf('\\');
-        if (backslashIndex >= 0) {
-            user = username.substring(backslashIndex + 1);
-            domain = username.substring(0, backslashIndex);
-        } else {
-            user = username;
-            domain = System.getProperty("http.auth.ntlm.domain", "");
-        }
-
-        return new NTCredentials(user, password, HostUtil.getLocalHostName(), 
domain);
-    }
 
-    private static class FileRequestEntity implements RequestEntity {
-        private File file;
-
-        public FileRequestEntity(File file) {
-            this.file = file;
-        }
-
-        public long getContentLength() {
-            return file.length();
-        }
-
-        public String getContentType() {
-            return null;
+        @Override
+        public Credentials getCredentials(final AuthScope authscope) {
+            if (authscope == null) {
+                return null;
+            }
+            // TODO: check if the credentials are requested for a proxy, in 
which case, we use the
+            // system configured proxy (system) properties to return the creds
+
+            final String realm = authscope.getRealm();
+            final String host = authscope.getHost();
+            final org.apache.ivy.util.Credentials ivyConfiguredCred = 
CredentialsStore.INSTANCE.getCredentials(realm, host);
+            if (ivyConfiguredCred == null) {
+                return null;
+            }
+            return createCredentials(ivyConfiguredCred.getUserName(), 
ivyConfiguredCred.getPasswd());
         }
 
-        public boolean isRepeatable() {
-            return true;
+        @Override
+        public void clear() {
+            this.cachedCreds.clear();
         }
 
-        public void writeRequest(OutputStream out) throws IOException {
-            try (InputStream instream = new FileInputStream(file)) {
-                FileUtil.copy(instream, out, null, false);
+        private static Credentials createCredentials(final String username, 
final String password) {
+            final String user;
+            final String domain;
+            int backslashIndex = username.indexOf('\\');
+            if (backslashIndex >= 0) {
+                user = username.substring(backslashIndex + 1);
+                domain = username.substring(0, backslashIndex);
+            } else {
+                user = username;
+                domain = System.getProperty("http.auth.ntlm.domain", "");
             }
+            return new NTCredentials(user, password, 
HostUtil.getLocalHostName(), domain);
         }
     }
-
 }

http://git-wip-us.apache.org/repos/asf/ant-ivy/blob/cdc548b8/src/java/org/apache/ivy/util/url/URLHandlerRegistry.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/ivy/util/url/URLHandlerRegistry.java 
b/src/java/org/apache/ivy/util/url/URLHandlerRegistry.java
index 8cf62de..629fd33 100644
--- a/src/java/org/apache/ivy/util/url/URLHandlerRegistry.java
+++ b/src/java/org/apache/ivy/util/url/URLHandlerRegistry.java
@@ -19,6 +19,8 @@ package org.apache.ivy.util.url;
 
 import org.apache.ivy.util.Message;
 
+import java.lang.reflect.Field;
+
 /**
  *
  */
@@ -32,7 +34,7 @@ public final class URLHandlerRegistry {
         return defaultHandler;
     }
 
-    public static void setDefault(URLHandler def) {
+    public static void setDefault(final URLHandler def) {
         defaultHandler = def;
     }
 
@@ -44,25 +46,17 @@ public final class URLHandlerRegistry {
      */
     public static URLHandler getHttp() {
         try {
-            Class.forName("org.apache.commons.httpclient.HttpClient");
-
-            // temporary fix for IVY-880: only use HttpClientHandler when
-            // http-client-3.x is available
-            
Class.forName("org.apache.commons.httpclient.params.HttpClientParams");
-
-            Class<?> handler = 
Class.forName("org.apache.ivy.util.url.HttpClientHandler");
-            Message.verbose("jakarta commons httpclient detected: using it for 
http downloading");
-            return (URLHandler) handler.newInstance();
-        } catch (ClassNotFoundException e) {
-            Message.verbose("jakarta commons httpclient not found: using jdk 
url handling");
-            return new BasicURLHandler();
-        } catch (NoClassDefFoundError e) {
-            Message.verbose("error occurred while loading jakarta commons 
httpclient: "
-                    + e.getMessage());
-            Message.verbose("Using jdk url handling instead.");
-            return new BasicURLHandler();
-        } catch (InstantiationException | IllegalAccessException e) {
-            Message.verbose("couldn't instantiate HttpClientHandler: using jdk 
url handling");
+            // check for the presence of HttpComponents HttpClient
+            Class.forName("org.apache.http.client.HttpClient");
+            // load our (wrapper) http handler
+            final Class<?> handler = 
Class.forName("org.apache.ivy.util.url.HttpClientHandler");
+            // we always use just one instance which is internally registered 
to be closed
+            // when the JVM exits
+            final Field instance = 
handler.getDeclaredField("DELETE_ON_EXIT_INSTANCE");
+            return (URLHandler) instance.get(null);
+        } catch (ClassNotFoundException | NoSuchFieldException | 
IllegalAccessException e) {
+            Message.verbose("Using JDK backed URL handler for HTTP interaction 
since the " +
+                    "Apache HttpComponents HttpClient backed handler couldn't 
be created due to: " + e.getMessage());
             return new BasicURLHandler();
         }
     }

http://git-wip-us.apache.org/repos/asf/ant-ivy/blob/cdc548b8/test/java/org/apache/ivy/util/url/HttpClientURLHandlerTest.java
----------------------------------------------------------------------
diff --git a/test/java/org/apache/ivy/util/url/HttpClientURLHandlerTest.java 
b/test/java/org/apache/ivy/util/url/HttpClientURLHandlerTest.java
new file mode 100644
index 0000000..0690c3e
--- /dev/null
+++ b/test/java/org/apache/ivy/util/url/HttpClientURLHandlerTest.java
@@ -0,0 +1,110 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+package org.apache.ivy.util.url;
+
+import java.io.File;
+import java.net.URL;
+
+import org.apache.ivy.core.settings.NamedTimeoutConstraint;
+import org.apache.ivy.core.settings.TimeoutConstraint;
+import org.apache.ivy.util.FileUtil;
+import org.apache.ivy.util.url.URLHandler.URLInfo;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test {@link HttpClientHandler}
+ */
+public class HttpclientURLHandlerTest {
+    // remote.test
+    private File testDir;
+
+    private HttpClientHandler handler;
+    private final TimeoutConstraint defaultTimeoutConstraint;
+
+    {
+        defaultTimeoutConstraint = new 
NamedTimeoutConstraint("default-http-client-handler-timeout");
+        ((NamedTimeoutConstraint) 
defaultTimeoutConstraint).setConnectionTimeout(5000);
+    }
+
+    @Before
+    public void setUp() {
+        testDir = new File("build/HttpclientURLHandlerTest");
+        testDir.mkdirs();
+
+        handler = new HttpClientHandler();
+    }
+
+    @After
+    public void tearDown() {
+        try {
+            handler.close();
+        } catch (Exception e) {
+            // ignore
+        }
+        FileUtil.forceDelete(testDir);
+    }
+
+    @Test
+    public void testIsReachable() throws Exception {
+        assertTrue("URL resource was expected to be reachable", 
handler.isReachable(new URL("http://www.google.fr/";), 
defaultTimeoutConstraint));
+        assertFalse("URL resource was expected to be unreachable", 
handler.isReachable(new URL("http://www.google.fr/unknownpage.html";), 
defaultTimeoutConstraint));
+    }
+
+    @Test
+    public void testGetURLInfo() throws Exception {
+        // IVY-390
+        URLHandler handler = new HttpClientHandler();
+        URLInfo info = handler
+                .getURLInfo(new URL(
+                        
"https://repo1.maven.org/maven2/commons-lang/commons-lang/[1.0,3.0[/commons-lang-[1.0,3.0[.pom";),
 defaultTimeoutConstraint);
+
+        assertEquals(URLHandler.UNAVAILABLE, info);
+    }
+
+    @Test
+    public void testContentEncoding() throws Exception {
+        assertDownloadOK(new 
URL("http://carsten.codimi.de/gzip.yaws/daniels.html";), new File(
+                testDir, "gzip.txt"));
+        assertDownloadOK(new URL(
+                
"http://carsten.codimi.de/gzip.yaws/daniels.html?deflate=on&zlib=on";), new File(
+                testDir, "deflate-zlib.txt"));
+        assertDownloadOK(new 
URL("http://carsten.codimi.de/gzip.yaws/daniels.html?deflate=on";),
+            new File(testDir, "deflate.txt"));
+        assertDownloadOK(new URL("http://carsten.codimi.de/gzip.yaws/a5.ps";), 
new File(testDir,
+                "a5-gzip.ps"));
+        assertDownloadOK(new 
URL("http://carsten.codimi.de/gzip.yaws/a5.ps?deflate=on";), new File(
+                testDir, "a5-deflate.ps"));
+        assertDownloadOK(new 
URL("http://carsten.codimi.de/gzip.yaws/nh80.pdf";), new File(testDir,
+                "nh80-gzip.pdf"));
+        assertDownloadOK(new 
URL("http://carsten.codimi.de/gzip.yaws/nh80.pdf?deflate=on";),
+            new File(testDir, "nh80-deflate.pdf"));
+    }
+
+    private void assertDownloadOK(final URL url, final File file) throws 
Exception {
+        handler.download(url, file, null, defaultTimeoutConstraint);
+        assertTrue("Content from " + url + " wasn't downloaded to " + file, 
file.exists());
+        assertTrue("Unexpected content at " + file + " for resource that was 
downloaded from " + url, file.isFile() && file.length() > 0);
+    }
+}

http://git-wip-us.apache.org/repos/asf/ant-ivy/blob/cdc548b8/test/java/org/apache/ivy/util/url/HttpclientURLHandlerTest.java
----------------------------------------------------------------------
diff --git a/test/java/org/apache/ivy/util/url/HttpclientURLHandlerTest.java 
b/test/java/org/apache/ivy/util/url/HttpclientURLHandlerTest.java
index 80cdd1a..0690c3e 100644
--- a/test/java/org/apache/ivy/util/url/HttpclientURLHandlerTest.java
+++ b/test/java/org/apache/ivy/util/url/HttpclientURLHandlerTest.java
@@ -20,6 +20,8 @@ package org.apache.ivy.util.url;
 import java.io.File;
 import java.net.URL;
 
+import org.apache.ivy.core.settings.NamedTimeoutConstraint;
+import org.apache.ivy.core.settings.TimeoutConstraint;
 import org.apache.ivy.util.FileUtil;
 import org.apache.ivy.util.url.URLHandler.URLInfo;
 
@@ -32,13 +34,19 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 /**
- * Test HttpClientHandler
+ * Test {@link HttpClientHandler}
  */
 public class HttpclientURLHandlerTest {
     // remote.test
     private File testDir;
 
     private HttpClientHandler handler;
+    private final TimeoutConstraint defaultTimeoutConstraint;
+
+    {
+        defaultTimeoutConstraint = new 
NamedTimeoutConstraint("default-http-client-handler-timeout");
+        ((NamedTimeoutConstraint) 
defaultTimeoutConstraint).setConnectionTimeout(5000);
+    }
 
     @Before
     public void setUp() {
@@ -50,13 +58,18 @@ public class HttpclientURLHandlerTest {
 
     @After
     public void tearDown() {
+        try {
+            handler.close();
+        } catch (Exception e) {
+            // ignore
+        }
         FileUtil.forceDelete(testDir);
     }
 
     @Test
     public void testIsReachable() throws Exception {
-        assertTrue(handler.isReachable(new URL("http://www.google.fr/";)));
-        assertFalse(handler.isReachable(new 
URL("http://www.google.fr/unknownpage.html";)));
+        assertTrue("URL resource was expected to be reachable", 
handler.isReachable(new URL("http://www.google.fr/";), 
defaultTimeoutConstraint));
+        assertFalse("URL resource was expected to be unreachable", 
handler.isReachable(new URL("http://www.google.fr/unknownpage.html";), 
defaultTimeoutConstraint));
     }
 
     @Test
@@ -65,7 +78,7 @@ public class HttpclientURLHandlerTest {
         URLHandler handler = new HttpClientHandler();
         URLInfo info = handler
                 .getURLInfo(new URL(
-                        
"https://repo1.maven.org/maven2/commons-lang/commons-lang/[1.0,3.0[/commons-lang-[1.0,3.0[.pom";));
+                        
"https://repo1.maven.org/maven2/commons-lang/commons-lang/[1.0,3.0[/commons-lang-[1.0,3.0[.pom";),
 defaultTimeoutConstraint);
 
         assertEquals(URLHandler.UNAVAILABLE, info);
     }
@@ -89,8 +102,9 @@ public class HttpclientURLHandlerTest {
             new File(testDir, "nh80-deflate.pdf"));
     }
 
-    private void assertDownloadOK(URL url, File file) throws Exception {
-        handler.download(url, file, null);
-        assertTrue(file.exists());
+    private void assertDownloadOK(final URL url, final File file) throws 
Exception {
+        handler.download(url, file, null, defaultTimeoutConstraint);
+        assertTrue("Content from " + url + " wasn't downloaded to " + file, 
file.exists());
+        assertTrue("Unexpected content at " + file + " for resource that was 
downloaded from " + url, file.isFile() && file.length() > 0);
     }
 }

Reply via email to