From ef8d01c891ddfe947da57b55a20cb25fde9e19f6 Mon Sep 17 00:00:00 2001
From: Michael Dombrowski <mdombro@amazon.com>
Date: Wed, 31 May 2023 17:44:47 -0400
Subject: [PATCH] fix: use URI builder for proxy, test for @ in username
 (#1482)

---
 .../com/aws/greengrass/util/ProxyUtils.java   | 25 +++++++++++++------
 .../aws/greengrass/util/ProxyUtilsTest.java   | 16 ++++++++----
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/src/main/java/com/aws/greengrass/util/ProxyUtils.java b/src/main/java/com/aws/greengrass/util/ProxyUtils.java
index 31e78815d9..e379814212 100644
--- a/src/main/java/com/aws/greengrass/util/ProxyUtils.java
+++ b/src/main/java/com/aws/greengrass/util/ProxyUtils.java
@@ -7,7 +7,10 @@
 
 import com.aws.greengrass.config.Topics;
 import com.aws.greengrass.deployment.DeviceConfiguration;
+import com.aws.greengrass.logging.api.Logger;
+import com.aws.greengrass.logging.impl.LogManager;
 import lombok.NonNull;
+import org.apache.http.client.utils.URIBuilder;
 import software.amazon.awssdk.crt.http.HttpProxyOptions;
 import software.amazon.awssdk.crt.io.ClientTlsContext;
 import software.amazon.awssdk.http.SdkHttpClient;
@@ -16,6 +19,8 @@
 
 import java.io.IOException;
 import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.security.GeneralSecurityException;
@@ -38,6 +43,7 @@
 
 public final class ProxyUtils {
 
+    private static final Logger logger = LogManager.getLogger(ProxyUtils.class);
     private static final AtomicReference<DeviceConfiguration> deviceConfiguration = new AtomicReference<>();
 
     private ProxyUtils() {
@@ -342,15 +348,20 @@ private static String removeAuthFromProxyUrl(String proxyUrl) {
 
     private static String addAuthToProxyUrl(String proxyUrl, String username, String password) {
         URI uri = URI.create(proxyUrl);
-        StringBuilder sb = new StringBuilder();
-        sb.append(uri.getScheme()).append("://").append(username).append(':').append(password).append('@')
-                .append(uri.getHost());
-
-        if (uri.getPort() != -1) {
-            sb.append(':').append(uri.getPort());
+        URIBuilder uriBuilder = new URIBuilder(uri, StandardCharsets.UTF_8);
+        if (password == null) {
+            uriBuilder.setUserInfo(username);
+        } else {
+            uriBuilder.setUserInfo(username, password);
         }
 
-        return sb.toString();
+        try {
+            return uriBuilder.build().toString();
+        } catch (URISyntaxException e) {
+            // This shouldn't ever be possible, URI builder will create a valid URI.
+            logger.error("Unable to create proxy URL environment variable", e);
+            return uri.toString();
+        }
     }
 
     /**
diff --git a/src/test/java/com/aws/greengrass/util/ProxyUtilsTest.java b/src/test/java/com/aws/greengrass/util/ProxyUtilsTest.java
index dc5252083c..09e35674b0 100644
--- a/src/test/java/com/aws/greengrass/util/ProxyUtilsTest.java
+++ b/src/test/java/com/aws/greengrass/util/ProxyUtilsTest.java
@@ -61,7 +61,7 @@ void testGetPortFromProxyUrl() {
 
     @Test
     void testGetProxyUsername() {
-        assertEquals("user", ProxyUtils.getProxyUsername("https://user:password@localhost:8080", "test-user"));
+        assertEquals("user@aws", ProxyUtils.getProxyUsername("https://user%40aws:password@localhost:8080", "test-user"));
         assertEquals("usernameOnly", ProxyUtils.getProxyUsername("https://usernameOnly@localhost:8080", "test-user"));
         assertEquals("test-user", ProxyUtils.getProxyUsername("https://myproxy:8080", "test-user"));
         assertNull(ProxyUtils.getProxyUsername("https://myproxy:8080", ""));
@@ -93,8 +93,9 @@ void testGetHttpProxyOptions() {
 
     @Test
     void testGetProxyEnvVarValue_passthroughWithAuth() {
-        when(deviceConfiguration.getProxyUrl()).thenReturn("https://test-user:itsasecret@myproxy:8080");
-        assertEquals("https://test-user:itsasecret@myproxy:8080", ProxyUtils.getProxyEnvVarValue(deviceConfiguration));
+        when(deviceConfiguration.getProxyUrl()).thenReturn("https://test-user%40aws:itsasecret@myproxy:8080");
+        assertEquals("https://test-user%40aws:itsasecret@myproxy:8080",
+                ProxyUtils.getProxyEnvVarValue(deviceConfiguration));
     }
 
     @Test
@@ -106,9 +107,14 @@ void testGetProxyEnvVarValue_passthroughWithoutAuth() {
     @Test
     void testGetProxyEnvVarValue_authInfoAddedToUrl() {
         when(deviceConfiguration.getProxyUrl()).thenReturn("https://myproxy:8080");
-        when(deviceConfiguration.getProxyUsername()).thenReturn("test-user");
+        when(deviceConfiguration.getProxyUsername()).thenReturn("test-user@aws");
         when(deviceConfiguration.getProxyPassword()).thenReturn("itsasecret");
-        assertEquals("https://test-user:itsasecret@myproxy:8080", ProxyUtils.getProxyEnvVarValue(deviceConfiguration));
+        assertEquals("https://test-user%40aws:itsasecret@myproxy:8080",
+                ProxyUtils.getProxyEnvVarValue(deviceConfiguration));
+
+        when(deviceConfiguration.getProxyPassword()).thenReturn(null);
+        assertEquals("https://test-user%40aws@myproxy:8080",
+                ProxyUtils.getProxyEnvVarValue(deviceConfiguration));
     }
 
     @Test