Migrate URIBuilder to httpcore5, Fixes AB#2909378

changelog.txt

@@ -1,3 +1,7 @@
+vNext
+----------
+- [MINOR] Migrate URIbuilder to httpcore5 (#2522)
+
 Version 18.1.0
 ----------
 (common4j 15.1.0)

common/build.gradle

@@ -155,6 +155,7 @@ dependencies {
 
     implementation(group: 'com.microsoft.device.display', name: 'display-mask', version: '0.3.0')
 
+    implementation 'org.apache.httpcomponents.core5:httpcore5:5.3'
     implementation "cz.msebera.android:httpclient:$rootProject.ext.mseberaApacheHttpClientVersion"
     implementation "com.nimbusds:nimbus-jose-jwt:$rootProject.ext.nimbusVersion"
     implementation "androidx.appcompat:appcompat:$rootProject.ext.appCompatVersion"

common/src/main/java/com/microsoft/identity/common/internal/cache/BaseActiveBrokerCache.kt

@@ -27,7 +27,7 @@ import com.microsoft.identity.common.java.interfaces.INameValueStorage
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
-import net.jcip.annotations.ThreadSafe
+import javax.annotation.concurrent.ThreadSafe
 
 /**
  * A cache for storing the active broker as known by the caller.

common/src/main/java/com/microsoft/identity/common/internal/cache/IActiveBrokerCache.kt

@@ -23,7 +23,7 @@
 package com.microsoft.identity.common.internal.cache
 
 import com.microsoft.identity.common.internal.broker.BrokerData
-import net.jcip.annotations.ThreadSafe
+import javax.annotation.concurrent.ThreadSafe
 
 
 /**

common4j/build.gradle

@@ -219,6 +219,7 @@ dependencies {
     implementation "org.json:json:$rootProject.ext.jsonVersion"
     implementation "com.github.stephenc.jcip:jcip-annotations:$rootProject.ext.jcipAnnotationVersion"
     implementation "cz.msebera.android:httpclient:$rootProject.ext.mseberaApacheHttpClientVersion"
+    implementation 'org.apache.httpcomponents.core5:httpcore5:5.3'
 
     testCompileOnly "com.github.spotbugs:spotbugs-annotations:$rootProject.ext.spotBugsAnnotationVersion"
     testCompileOnly "org.projectlombok:lombok:$rootProject.ext.lombokVersion"

common4j/src/main/com/microsoft/identity/common/java/nativeauth/providers/NativeAuthOAuth2Configuration.kt

@@ -220,7 +220,7 @@ class NativeAuthOAuth2Configuration(
     private fun getEndpointUrlFromRootAndTenantAndSuffix(root: URL, endpointSuffix: String): URL {
         return try {
             if (BuildValues.getDC().isNotEmpty()) {
-                UrlUtil.appendPathAndQueryToURL(root, endpointSuffix, "dc=${BuildValues.getDC()}")
+                UrlUtil.appendPathAndQueryToURL(root, endpointSuffix, mapOf("dc" to BuildValues.getDC()))
             } else {
                 UrlUtil.appendPathToURL(root, endpointSuffix)
             }

common4j/src/main/com/microsoft/identity/common/java/util/CommonURIBuilder.java

@@ -22,24 +22,26 @@
 // THE SOFTWARE.
 package com.microsoft.identity.common.java.util;
 
+import org.apache.hc.core5.http.NameValuePair;
+import org.apache.hc.core5.net.URIBuilder;
+
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.List;
 import java.util.Map;
 
-import cz.msebera.android.httpclient.NameValuePair;
 import edu.umd.cs.findbugs.annotations.Nullable;
 import lombok.NonNull;
 
 /**
  * Our URIBuilder.
  * We want to make sure we never send duplicated parameters to the server.
  * This is done by
- * 1. disabling {@link cz.msebera.android.httpclient.client.utils.URIBuilder#addParameter(String, String)} and
- * {@link cz.msebera.android.httpclient.client.utils.URIBuilder#addParameters(List)}
+ * 1. disabling {@link org.apache.hc.core5.net.URIBuilder#addParameter(String, String)} and
+ * {@link org.apache.hc.core5.net.URIBuilder#addParameters(List)}
  * 2. adding {@link CommonURIBuilder#addParametersIfAbsent}
  */
-public class CommonURIBuilder extends cz.msebera.android.httpclient.client.utils.URIBuilder {
+public class CommonURIBuilder extends URIBuilder {
 
     public CommonURIBuilder() {
         super();

common4j/src/main/com/microsoft/identity/common/java/util/UrlUtil.java

@@ -66,7 +66,7 @@ public static URL appendPathToURL(@NonNull final URL urlToAppend,
      */
     public static URL appendPathAndQueryToURL(@NonNull final URL urlToAppend,
                                       @Nullable final String pathString,
-                                      @Nullable final String queryParam)
+                                      @Nullable final Map<String, String> queryParam)
             throws URISyntaxException, MalformedURLException {
 
         if (StringUtil.isNullOrEmpty(pathString)) {
@@ -99,9 +99,8 @@ public static URL appendPathAndQueryToURL(@NonNull final URL urlToAppend,
         }
 
         builder.setPathSegments(pathSegments);
-        if (queryParam != null && !queryParam.isEmpty()) {
-            builder.setQuery(queryParam);
-        }
+        builder.addParametersIfAbsent(queryParam);
+
         return builder.build().toURL();
     }
 

common4j/src/test/com/microsoft/identity/common/java/providers/Constants.java

@@ -41,7 +41,7 @@ public class Constants {
     public static final String MOCK_STATE = "MOCK_STATE";
     public static final String MOCK_STATE_ENCODED = "TU9DS19TVEFURQ";
     public static final String MOCK_SCOPE = "MOCK_SCOPE MOCK_SCOPE2";
-    public static final String MOCK_SCOPE_ENCODED = "MOCK_SCOPE+MOCK_SCOPE2";
+    public static final String MOCK_SCOPE_ENCODED = "MOCK_SCOPE%20MOCK_SCOPE2";
     public static final String MOCK_QUERY_1 = "MOCK_QUERY_1";
     public static final String MOCK_QUERY_2 = "MOCK_QUERY_2";
     public static final String MOCK_VALUE_1 = "MOCK_VALUE_1";

common4j/src/test/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/MicrosoftStsAuthorizationRequestTests.java

@@ -56,7 +56,7 @@ public class MicrosoftStsAuthorizationRequestTests {
     private static final String DEFAULT_TEST_REDIRECT_URI_ENCODED = "some%3A%2F%2Fredirect.uri";
     private static final String DEFAULT_TEST_LOGIN_HINT = "someLoginHint";
     private static final String DEFAULT_TEST_SCOPE = "scope1 scope2";
-    private static final String DEFAULT_TEST_SCOPE_ENCODED = "scope1+scope2";
+    private static final String DEFAULT_TEST_SCOPE_ENCODED = "scope1%20scope2";
     private static final String DEFAULT_TEST_AUTHORIZATION_ENDPOINT = "https://login.microsoftonline.com/common/oAuth2/v2.0/authorize";
     private static final UUID DEFAULT_TEST_CORRELATION_ID = UUID.randomUUID();
     private static final List<Map.Entry<String, String>> DEFAULT_TEST_EXTRA_QP = new ArrayList<Map.Entry<String, String>>() {{

common4j/src/test/com/microsoft/identity/common/java/util/CommonURIBuilderTest.java

@@ -23,6 +23,7 @@
 
 package com.microsoft.identity.common.java.util;
 
+import org.apache.hc.core5.http.NameValuePair;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -34,8 +35,6 @@
 import java.util.List;
 import java.util.Map;
 
-import cz.msebera.android.httpclient.NameValuePair;
-
 @RunWith(JUnit4.class)
 public class CommonURIBuilderTest {
 
@@ -80,6 +79,29 @@ public void testCallingAddParametersIfAbsent_ValueAlreadyExists(){
         Assert.assertEquals("Value1", builder.getQueryParams().get(0).getValue());
     }
 
+    @Test
+    public void testCallingAddParametersIfAbsent_WithNullMap(){
+        final CommonURIBuilder builder = new CommonURIBuilder()
+                .setParameter("Test1", "Value1")
+                .addParametersIfAbsent((Map<String, ?>) null);
+
+        Assert.assertEquals(1, builder.getQueryParams().size());
+        Assert.assertEquals("Test1", builder.getQueryParams().get(0).getName());
+        Assert.assertEquals("Value1", builder.getQueryParams().get(0).getValue());
+    }
+
+    @Test
+    public void testCallingAddParametersIfAbsent_WithEmptyMap(){
+        final Map<String, String> map = new HashMap<>();
+        final CommonURIBuilder builder = new CommonURIBuilder()
+                .setParameter("Test1", "Value1")
+                .addParametersIfAbsent(map);
+
+        Assert.assertEquals(1, builder.getQueryParams().size());
+        Assert.assertEquals("Test1", builder.getQueryParams().get(0).getName());
+        Assert.assertEquals("Value1", builder.getQueryParams().get(0).getValue());
+    }
+
     @Test
     public void testCallingAddParametersIfAbsent_WithMap(){
         final Map<String, String> map = new HashMap<>();