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 <[email protected]> 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: [email protected]")) - .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: [email protected]")) - .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: [email protected]")) - .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: [email protected] For additional commands, e-mail: [email protected]
