URI encoding corner cases

- smtp may not have auth - smtp may not have auth or tls (no query params) - tidy up no username/password/auth encoding test cases added for these issues
thunderbird · Jan 31, 2021 · 86a64c9 · 86a64c9
1 parent 66cd4f8
commit 86a64c9
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 3 deletions.
diff --git a/backend/imap/src/main/java/com/fsck/k9/backend/imap/ImapStoreUriCreator.java b/backend/imap/src/main/java/com/fsck/k9/backend/imap/ImapStoreUriCreator.java
@@ -84,6 +84,7 @@ public static String create(ServerSettings server) {
             } else {
                 path = "/1|";
             }
+            // query is assumed to have at least one entry, and we have to do the encoding ourselves, due to limitations in URI
             return new URI(scheme, userInfo, server.host, server.port, path, null, null).toString() + "?" + query;
         } catch (URISyntaxException e) {
             throw new IllegalArgumentException("Can't create ImapStore URI", e);

diff --git a/mail/common/src/main/java/com/fsck/k9/mail/helper/UrlEncodingHelper.java b/mail/common/src/main/java/com/fsck/k9/mail/helper/UrlEncodingHelper.java
@@ -11,6 +11,7 @@
 public final class UrlEncodingHelper {
     private UrlEncodingHelper() {
     }
+
     public static Map<String, String> splitQuery(String query) {
         Map<String, String> queryParams = new HashMap<>();
         if (query == null) {
@@ -25,6 +26,9 @@ public static Map<String, String> splitQuery(String query) {
     }
 
     public static String buildQuery(Map<String, String> params) {
+        if (params.size() == 0) {
+            return null;
+        }
         StringBuffer query = new StringBuffer("");
         for (String param: params.keySet()) {
             query.append(param);
@@ -37,8 +41,6 @@ public static String buildQuery(Map<String, String> params) {
         return query.toString();
     }
 
-
-
     public static String decodeUtf8(String s) {
         try {
             return URLDecoder.decode(s, "UTF-8");

diff --git a/...protocols/smtp/src/main/java/com/fsck/k9/mail/transport/smtp/SmtpTransportUriCreator.java b/...protocols/smtp/src/main/java/com/fsck/k9/mail/transport/smtp/SmtpTransportUriCreator.java
@@ -39,7 +39,9 @@ public static String createSmtpUri(ServerSettings server) {
         if (! clientCertificateAliasEnc.equals("") ) {
             params.put("tls-cert",clientCertificateAliasEnc);
         }
-        params.put("auth-type",server.authenticationType.name().toLowerCase());
+        if (server.authenticationType != null) {
+            params.put("auth-type", server.authenticationType.name().toLowerCase());
+        }
         String query = buildQuery(params);
 
 
@@ -69,6 +71,11 @@ public static String createSmtpUri(ServerSettings server) {
             }
         } else {
             userInfo = userEnc + ":" + passwordEnc;
+            if (userInfo.equals(":")) {
+                userInfo = null;
+            } else if ((passwordEnc == null) || (passwordEnc.equals(""))) {
+                userInfo = userEnc;
+            }
         }
         try {
             return new URI(scheme, userInfo, server.host, server.port, null, query,

diff --git a/...ocols/smtp/src/test/java/com/fsck/k9/mail/transport/smtp/SmtpTransportUriCreatorTest.java b/...ocols/smtp/src/test/java/com/fsck/k9/mail/transport/smtp/SmtpTransportUriCreatorTest.java
@@ -61,6 +61,31 @@ public void createTransportUri_canEncodeSmtpTlsUri() {
         assertEquals("smtp+tls+://user:password:PLAIN@server:123456?tls-cert=clientCert&auth-type=plain", result);
     }
 
+    @Test
+    public void createTransportUri_noAuthUri() {
+        ServerSettings serverSettings = new ServerSettings(
+                "smtp", "server", 123456,
+                ConnectionSecurity.STARTTLS_REQUIRED, null,
+                null, null, "clientCert");
+
+        String result = SmtpTransportUriCreator.createSmtpUri(serverSettings);
+
+        assertEquals("smtp+tls+://server:123456?tls-cert=clientCert", result);
+    }
+
+
+    @Test
+    public void createTransportUri_noQueryUri() {
+        ServerSettings serverSettings = new ServerSettings(
+                "smtp", "server", 123456,
+                ConnectionSecurity.NONE, null,
+                "user", null, null);
+
+        String result = SmtpTransportUriCreator.createSmtpUri(serverSettings);
+
+        assertEquals("smtp://user@server:123456", result);
+    }
+
     @Test
     public void createTransportUri_canEncodeSmtpUri() {
         ServerSettings serverSettings = new ServerSettings(

diff --git a/...ocols/smtp/src/test/java/com/fsck/k9/mail/transport/smtp/SmtpTransportUriDecoderTest.java b/...ocols/smtp/src/test/java/com/fsck/k9/mail/transport/smtp/SmtpTransportUriDecoderTest.java
@@ -7,6 +7,7 @@
 import org.junit.Test;
 
 import static junit.framework.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 
 
 public class SmtpTransportUriDecoderTest {
@@ -141,6 +142,33 @@ public void decodeTransportUri_canDecodeHybridURI() {
         assertEquals(25, result.port);
     }
 
+    @Test
+    public void decodeTransportUri_decodeCleanishURI() {
+        String storeUri = "smtp+tls+://:@server:25/?auth-type=PLAIN&tls-cert=clientCert";
+
+        ServerSettings result = SmtpTransportUriDecoder.decodeSmtpUri(storeUri);
+
+        assertEquals("clientCert", result.clientCertificateAlias);
+        assertEquals(AuthType.PLAIN, result.authenticationType);
+        assertNull(result.password);
+
+        assertEquals(25, result.port);
+    }
+    @Test
+    public void decodeTransportUri_decodeAlmostEmptyURI() {
+        String storeUri = "smtp+tls+://server";
+
+        ServerSettings result = SmtpTransportUriDecoder.decodeSmtpUri(storeUri);
+
+        assertNull(result.clientCertificateAlias);
+        assertNull(result.authenticationType);
+        assertNull(result.password);
+
+        assertEquals(587, result.port);
+    }
+
+
+
     @Test
     public void decodeTransportUri_canHybridParamsWinURI() {
         String storeUri = "smtp+tls+://user:never:EXTERNAL@server?auth-type=PLAIN&tls-cert=clientCert";