You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2019/11/06 06:18:46 UTC

[james-project] 04/09: JAMES-2947 WebAdmin prevents encoded '/' in a domain name at creation

This is an automated email from the ASF dual-hosted git repository.

btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit c8b5fd3fbd4cdaca7e173a673004a37e7ac29c33
Author: Gautier DI FOLCO <gd...@linagora.com>
AuthorDate: Wed Oct 30 13:45:24 2019 +0100

    JAMES-2947 WebAdmin prevents encoded '/' in a domain name at creation
---
 .../main/java/org/apache/james/core/Domain.java    |  4 +--
 .../java/org/apache/james/core/MailAddress.java    | 10 +++++-
 .../apache/james/domainlist/api/DomainTest.java    |  5 +++
 .../james/webadmin/routes/DomainsRoutes.java       | 21 ++++++++++--
 .../routes/DLPConfigurationRoutesTest.java         |  6 ++--
 .../james/webadmin/routes/DomainsRoutesTest.java   | 40 +++++++++++++++++++++-
 6 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/core/src/main/java/org/apache/james/core/Domain.java b/core/src/main/java/org/apache/james/core/Domain.java
index 3bf831a..2be5e13 100644
--- a/core/src/main/java/org/apache/james/core/Domain.java
+++ b/core/src/main/java/org/apache/james/core/Domain.java
@@ -39,8 +39,8 @@ public class Domain implements Serializable {
 
     public static Domain of(String domain) {
         Preconditions.checkNotNull(domain, "Domain can not be null");
-        Preconditions.checkArgument(!domain.isEmpty() && !domain.contains("@"),
-            "Domain can not be empty nor contain `@`");
+        Preconditions.checkArgument(!domain.isEmpty() && !domain.contains("@") && !domain.contains("/"),
+            "Domain can not be empty nor contain `@` nor `/`");
         Preconditions.checkArgument(domain.length() <= MAXIMUM_DOMAIN_LENGTH,
             "Domain name length should not exceed " + MAXIMUM_DOMAIN_LENGTH + " characters");
         return new Domain(domain);
diff --git a/core/src/main/java/org/apache/james/core/MailAddress.java b/core/src/main/java/org/apache/james/core/MailAddress.java
index 0d4cbc9..beba6bf 100644
--- a/core/src/main/java/org/apache/james/core/MailAddress.java
+++ b/core/src/main/java/org/apache/james/core/MailAddress.java
@@ -232,7 +232,15 @@ public class MailAddress implements java.io.Serializable {
         }
 
         localPart = localPartSB.toString();
-        domain = Domain.of(domainSB.toString());
+        domain = createDomain(domainSB.toString());
+    }
+
+    private Domain createDomain(String domain) throws AddressException {
+        try {
+            return Domain.of(domain);
+        } catch (IllegalArgumentException e) {
+            throw new AddressException(e.getMessage());
+        }
     }
 
     private int parseUnquotedLocalPartOrThrowException(StringBuffer localPartSB, String address, int pos)
diff --git a/core/src/test/java/org/apache/james/domainlist/api/DomainTest.java b/core/src/test/java/org/apache/james/domainlist/api/DomainTest.java
index 1abffc6..3618943 100644
--- a/core/src/test/java/org/apache/james/domainlist/api/DomainTest.java
+++ b/core/src/test/java/org/apache/james/domainlist/api/DomainTest.java
@@ -93,6 +93,11 @@ class DomainTest {
     }
 
     @Test
+    void shouldThrowWhenDomainContainUrlOperatorSymbol() {
+        assertThatThrownBy(() -> Domain.of("Dom/in")).isInstanceOf(IllegalArgumentException.class);
+    }
+
+    @Test
     void shouldThrowWhenDomainIsEmpty() {
         assertThatThrownBy(() -> Domain.of("")).isInstanceOf(IllegalArgumentException.class);
     }
diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DomainsRoutes.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DomainsRoutes.java
index 936b3ea..e0d70fa 100644
--- a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DomainsRoutes.java
+++ b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DomainsRoutes.java
@@ -21,6 +21,9 @@ package org.apache.james.webadmin.routes;
 
 import static org.apache.james.webadmin.Constants.SEPARATOR;
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -265,13 +268,27 @@ public class DomainsRoutes implements Routes {
     }
 
     private Domain checkValidDomain(String domainName) {
+        String urlDecodedDomainName = urlDecodeDomain(domainName);
         try {
-            return Domain.of(domainName);
+            return Domain.of(urlDecodedDomainName);
         } catch (IllegalArgumentException e) {
             throw ErrorResponder.builder()
                 .statusCode(HttpStatus.BAD_REQUEST_400)
                 .type(ErrorType.INVALID_ARGUMENT)
-                .message("Invalid request for domain creation " + domainName)
+                .message("Invalid request for domain creation " + urlDecodedDomainName)
+                .cause(e)
+                .haltError();
+        }
+    }
+
+    private String urlDecodeDomain(String domainName) {
+        try {
+            return URLDecoder.decode(domainName, StandardCharsets.UTF_8.toString());
+        } catch (IllegalArgumentException | UnsupportedEncodingException e) {
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorType.INVALID_ARGUMENT)
+                .message("Invalid request for domain creation " + domainName + " unable to url decode some characters")
                 .cause(e)
                 .haltError();
         }
diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DLPConfigurationRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DLPConfigurationRoutesTest.java
index 10710a8..151f622 100644
--- a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DLPConfigurationRoutesTest.java
+++ b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DLPConfigurationRoutesTest.java
@@ -326,7 +326,7 @@ class DLPConfigurationRoutesTest {
                 .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
                 .body("type", is("InvalidArgument"))
                 .body("message", is("Invalid request for domain: dr@strange.com"))
-                .body("details", is("Domain can not be empty nor contain `@`"));
+                .body("details", is("Domain can not be empty nor contain `@` nor `/`"));
         }
 
         @Test
@@ -643,7 +643,7 @@ class DLPConfigurationRoutesTest {
                 .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
                 .body("type", is("InvalidArgument"))
                 .body("message", is("Invalid request for domain: dr@strange.com"))
-                .body("details", is("Domain can not be empty nor contain `@`"));
+                .body("details", is("Domain can not be empty nor contain `@` nor `/`"));
         }
     }
 
@@ -856,7 +856,7 @@ class DLPConfigurationRoutesTest {
                 .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
                 .body("type", is("InvalidArgument"))
                 .body("message", is("Invalid request for domain: dr@strange.com"))
-                .body("details", is("Domain can not be empty nor contain `@`"));
+                .body("details", is("Domain can not be empty nor contain `@` nor `/`"));
         }
     }
     
diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DomainsRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DomainsRoutesTest.java
index 185cabb..607f166 100644
--- a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DomainsRoutesTest.java
+++ b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DomainsRoutesTest.java
@@ -23,6 +23,7 @@ import static io.restassured.RestAssured.given;
 import static io.restassured.RestAssured.when;
 import static io.restassured.RestAssured.with;
 import static org.apache.james.webadmin.Constants.SEPARATOR;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
@@ -33,6 +34,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.net.InetAddress;
+import java.util.Map;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.james.core.Domain;
@@ -154,7 +156,43 @@ class DomainsRoutesTest {
         }
 
         @Test
-        void putShouldReturnNotFoundWhenDomainNameContainsUrlSeparator() {
+        void putShouldReturnUserErrorWhenNameContainsUrlEncodedUrlOperator() {
+            Map<String, Object> errors = when()
+                .put(DOMAIN + "%2F" + DOMAIN)
+            .then()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .contentType(ContentType.JSON)
+                .extract()
+                .body()
+                .jsonPath()
+                .getMap(".");
+
+            assertThat(errors)
+                .containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
+                .containsEntry("type", "InvalidArgument")
+                .containsEntry("message", "Invalid request for domain creation domain/domain");
+        }
+
+        @Test
+        void putShouldReturnUserErrorWhenNameContainsInvalidUrlEncodedCharacters() {
+            Map<String, Object> errors = when()
+                .put(DOMAIN + "%GG" + DOMAIN)
+            .then()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .contentType(ContentType.JSON)
+                .extract()
+                .body()
+                .jsonPath()
+                .getMap(".");
+
+            assertThat(errors)
+                .containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
+                .containsEntry("type", "InvalidArgument")
+                .containsEntry("message", "Invalid request for domain creation domain%GGdomain unable to url decode some characters");
+        }
+
+        @Test
+        void putShouldReturnUserErrorWhenNameContainsUrlSeparator() {
             when()
                 .put(DOMAIN + "/" + DOMAIN)
             .then()


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org