Upgrade Apache HttpClient 4.5.13 -> 5.1.3 (#35)

Author: labkey-adam

labkey-client-api/CHANGELOG.md

@@ -1,9 +1,22 @@
 # The LabKey Remote API Library for Java - Change Log
 
-## version 2.?.?
-*Released*: TBD
+## version 3.0.0
+*Released*: 14 September 2022
+* Migrate internal HTTP handling to use Apache HttpClient 5.1.x
+* Switch `StopImpersonatingCommand` to disable redirects (mimicking previous behavior)
+* Add `Connection.stopImpersonating()` and deprecate `stopImpersonate()`
+* Remove deprecated methods:
+  * ApiVersionException() (use constructor that takes contentType)
+  * CommandException() (use constructor that takes contentType)
+  * Connection.getBaseUrl() (use Connection.getBaseURI())
+  * CredentialsProvider.configureRequest() (use variant that takes a URI)
+  * Filter.NON_BLANK (use Filter.NONBLANK)
+  * Filter.getCaption() (use Filter.getDisplayValue())
+  * Filter.getName() (use Filter.getUrlKey())
+  * Filter.isDataValueRequired() (use Filter.isValueRequired())
 * Remove SAS macros and wrapper classes
-* Add `CreateFolderCommand` and `CreateProjectCommand`
+* Add `CreateFolderCommand`
+* Add `CreateProjectCommand` (earliest compatible LabKey Server version: 22.3.0)
 * Update `LogoutCommand` to use POST
 
 ## version 2.0.0

labkey-client-api/build.gradle

@@ -57,20 +57,19 @@ repositories {
 
 group "org.labkey.api"
 
-version "2.1.0-SNAPSHOT"
+version "3.1.0-SNAPSHOT"
 
 dependencies {
-    implementation "org.apache.httpcomponents:httpmime:${httpmimeVersion}"
     api ("com.googlecode.json-simple:json-simple:${jsonSimpleVersion}")
             {
                 // exclude this because it gets in the way of our own JSON object implementations from server/api
                 exclude group: "org.json", module:"json"
             }
+    api "org.apache.httpcomponents.client5:httpclient5:${httpclient5Version}"
+    api "org.apache.httpcomponents.core5:httpcore5:${httpcore5Version}"
     implementation "net.sf.opencsv:opencsv:${opencsvVersion}"
     implementation "commons-logging:commons-logging:${commonsLoggingVersion}"
-    api "org.apache.httpcomponents:httpclient:${httpclientVersion}"
     implementation "commons-codec:commons-codec:${commonsCodecVersion}"
-    api "org.apache.httpcomponents:httpcore:${httpcoreVersion}"
 }
 
 configurations.all

labkey-client-api/gradle.properties

@@ -14,9 +14,9 @@ commonsCodecVersion=1.15
 commonsLoggingVersion=1.2
 
 hamcrestVersion=1.3
-httpclientVersion=4.5.13
-httpcoreVersion=4.4.14
-httpmimeVersion=4.5.13
+
+httpclient5Version=5.1.3
+httpcore5Version=5.1.4
 
 jsonSimpleVersion=1.1.1
 

labkey-client-api/src/org/labkey/remoteapi/ApiKeyCredentialsProvider.java

@@ -15,9 +15,9 @@
  */
 package org.labkey.remoteapi;
 
-import org.apache.http.auth.AuthenticationException;
-import org.apache.http.client.methods.HttpUriRequest;
-import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.hc.client5.http.auth.AuthenticationException;
+import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
+import org.apache.hc.client5.http.protocol.HttpClientContext;
 
 import java.net.URI;
 

labkey-client-api/src/org/labkey/remoteapi/ApiVersionException.java

@@ -24,14 +24,6 @@
  */
 public class ApiVersionException extends CommandException
 {
-    /**
-     * @deprecated Use {@link #ApiVersionException(String, int, Map, String, String)}
-     */
-    ApiVersionException(String message, int statusCode, Map<String, Object> properties, String responseText)
-    {
-        this(message, statusCode, properties, responseText, null);
-    }
-
     ApiVersionException(String message, int statusCode, Map<String, Object> properties, String responseText, String contentType)
     {
         super(message, statusCode, properties, responseText, contentType);

labkey-client-api/src/org/labkey/remoteapi/BasicAuthCredentialsProvider.java

@@ -15,14 +15,14 @@
  */
 package org.labkey.remoteapi;
 
-import org.apache.http.auth.AuthScope;
-import org.apache.http.auth.AuthenticationException;
-import org.apache.http.auth.Credentials;
-import org.apache.http.auth.UsernamePasswordCredentials;
-import org.apache.http.client.methods.HttpUriRequest;
-import org.apache.http.client.protocol.HttpClientContext;
-import org.apache.http.impl.auth.BasicScheme;
-import org.apache.http.impl.client.BasicCredentialsProvider;
+import org.apache.hc.client5.http.auth.AuthScope;
+import org.apache.hc.client5.http.auth.AuthenticationException;
+import org.apache.hc.client5.http.auth.Credentials;
+import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
+import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
+import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
+import org.apache.hc.client5.http.impl.auth.BasicScheme;
+import org.apache.hc.client5.http.protocol.HttpClientContext;
 
 import java.net.URI;
 
@@ -43,12 +43,10 @@ public BasicAuthCredentialsProvider(String email, String password)
     @Override
     public void configureRequest(URI baseURI, HttpUriRequest request, HttpClientContext httpClientContext) throws AuthenticationException
     {
-        AuthScope scope = new AuthScope(baseURI.getHost(), AuthScope.ANY_PORT, AuthScope.ANY_REALM);
         BasicCredentialsProvider provider = new BasicCredentialsProvider();
-        Credentials credentials = new UsernamePasswordCredentials(_email, _password);
+        AuthScope scope = new AuthScope(baseURI.getHost(), -1);
+        Credentials credentials = new UsernamePasswordCredentials(_email, _password.toCharArray());
         provider.setCredentials(scope, credentials);
-
         httpClientContext.setCredentialsProvider(provider);
-        request.addHeader(new BasicScheme().authenticate(credentials, request, httpClientContext));
     }
 }

labkey-client-api/src/org/labkey/remoteapi/Command.java

@@ -15,15 +15,13 @@
  */
 package org.labkey.remoteapi;
 
-import org.apache.commons.codec.EncoderException;
-import org.apache.commons.codec.net.URLCodec;
 import org.apache.commons.logging.LogFactory;
-import org.apache.http.Header;
-import org.apache.http.auth.AuthenticationException;
-import org.apache.http.client.methods.CloseableHttpResponse;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.methods.HttpUriRequest;
-import org.apache.http.client.utils.URIBuilder;
+import org.apache.hc.client5.http.auth.AuthenticationException;
+import org.apache.hc.client5.http.classic.methods.HttpGet;
+import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
+import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
+import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.net.URIBuilder;
 import org.json.simple.JSONObject;
 import org.json.simple.JSONValue;
 
@@ -256,12 +254,12 @@ private Response(CloseableHttpResponse httpResponse, String contentType, Long co
 
         public String getStatusText()
         {
-            return _httpResponse.getStatusLine().getReasonPhrase();
+            return _httpResponse.getReasonPhrase();
         }
 
         public int getStatusCode()
         {
-            return _httpResponse.getStatusLine().getStatusCode();
+            return _httpResponse.getCode();
         }
 
         public String getContentType()
@@ -328,7 +326,7 @@ protected Response _execute(Connection connection, String folderPath) throws Com
         {
             //construct and initialize the HttpUriRequest
             request = getHttpRequest(connection, folderPath);
-            LogFactory.getLog(Command.class).info("Requesting URL: " + request.getURI().toString());
+            LogFactory.getLog(Command.class).info("Requesting URL: " + request.getRequestUri());
 
             //execute the request
             httpResponse = connection.executeRequest(request, getTimeout());
@@ -448,19 +446,13 @@ protected HttpUriRequest getHttpRequest(Connection connection, String folderPath
 // TODO        (Dave 11/11/14 -- as far as I can tell the default of the AuthSchemes is to automatically handle challenges
 //        method.setDoAuthentication(true);
 
+        // NOTE: Fairly sure that the "unescaped" comment below is obsolete
+        // TODO: Combine getActionUrl() and addParameters() into a single method
         //construct a URI from the results of the getActionUrl method
         //note that this method returns an unescaped URL, so pass
         //false for the second parameter to escape it
         URI uri = getActionUrl(connection, folderPath);
-
-        //the query string values will need to be escaped as they are
-        //put into the query string, and we don't want to re-escape the
-        //entire thing, as the %, & and = characters will be escaped again
-        //so add this separately as a raw value 
-        //(indicating that it has already been escaped)
-        String queryString = getQueryString();
-        if (null != queryString)
-            uri = new URIBuilder(uri).setQuery(queryString).build();
+        uri = addParameters(uri);
 
         return createRequest(uri);
     }
@@ -487,7 +479,7 @@ protected HttpUriRequest createRequest(URI uri)
      * @return The URL
      * @throws URISyntaxException if the uri constructed from the parameters is malformed
      */
-    protected URI getActionUrl(Connection connection, String folderPath) throws URISyntaxException
+    private URI getActionUrl(Connection connection, String folderPath) throws URISyntaxException
     {
         URI uri = connection.getBaseURI();
 
@@ -520,60 +512,55 @@ protected URI getActionUrl(Connection connection, String folderPath) throws URIS
     }
 
     /**
-     * Returns the query string portion of the URL for this command.
+     * Adds all parameters to the passed URI
      * <p>
-     * The parameters in the query string should be encoded to avoid parsing errors on the server.
-     * @return The query string
-     * @throws CommandException Thrown if there is a problem encoding the query string parameter names or values
+     * @return The URI with parameters added
+     * @throws CommandException Thrown if there is a problem building the URI
      */
-    protected String getQueryString() throws CommandException
+    protected URI addParameters(URI uri) throws CommandException, URISyntaxException
     {
         Map<String, Object> params = getParameters();
 
         //add the required version if it's > 0
         if (getRequiredVersion() > 0)
             params.put(CommonParameters.apiVersion.name(), getRequiredVersion());
 
-        StringBuilder qstring = new StringBuilder();
-        URLCodec urlCodec = new URLCodec();
-        try
+        if (params.isEmpty())
+            return uri;
+
+        URIBuilder builder = new URIBuilder(uri);
+
+        for (String name : params.keySet())
         {
-            for (String name : params.keySet())
+            Object value = params.get(name);
+            if (value instanceof Collection<?> col)
             {
-                Object value = params.get(name);
-                if (value instanceof Collection)
+                for (Object o : col)
                 {
-                    for (Object o : ((Collection) value))
-                    {
-                        appendParameter(qstring, urlCodec, name, o);
-                    }
-                }
-                else
-                {
-                    appendParameter(qstring, urlCodec, name, value);
+                    addParameter(builder, name, o);
                 }
             }
+            else
+            {
+                addParameter(builder, name, value);
+            }
+        }
+
+        try
+        {
+            return builder.build();
         }
-        catch(EncoderException e)
+        catch (URISyntaxException e)
         {
             throw new CommandException(e.getMessage());
         }
-
-        return qstring.length() > 0 ? qstring.toString() : null;
     }
 
-    private void appendParameter(StringBuilder qstring, URLCodec urlCodec, String name, Object value)
-            throws EncoderException
+    private void addParameter(URIBuilder builder, String name, Object value)
     {
         String strValue = null == value ? null : getParamValueAsString(value, name);
-        if(null != strValue)
-        {
-            if (qstring.length() > 0)
-                qstring.append('&');
-            qstring.append(urlCodec.encode(name));
-            qstring.append('=');
-            qstring.append(urlCodec.encode(strValue));
-        }
+        if (null != strValue)
+            builder.addParameter(name, strValue);
     }
 
     /**

labkey-client-api/src/org/labkey/remoteapi/CommandException.java

@@ -15,7 +15,7 @@
  */
 package org.labkey.remoteapi;
 
-import org.apache.http.impl.EnglishReasonPhraseCatalog;
+import org.apache.hc.core5.http.impl.EnglishReasonPhraseCatalog;
 
 import java.util.Locale;
 import java.util.Map;
@@ -51,18 +51,6 @@ public CommandException(String message)
         this(message, 0, null, null, null);
     }
 
-    /**
-     * @param message The message text (should not be null).
-     * @param statusCode The HTTP status code.
-     * @param properties The exception property map (may be null)
-     * @param responseText The full response text (may be null)
-     * @deprecated Use {@link #CommandException(String, int, Map, String, String)}
-     */
-    public CommandException(String message, int statusCode, Map<String, Object> properties, String responseText)
-    {
-        this(message, statusCode, properties, responseText, null);
-    }
-
     /**
      * Constructs a new CommandException given a message, HTTP status code,
      * exception property map, responseText, and contentType.