You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2022/05/24 01:55:39 UTC
[james-project] branch master updated: JAMES-3431 Stricter validation for DSN ENVID (#1002)
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
The following commit(s) were added to refs/heads/master by this push:
new 5ad5cb6a4e JAMES-3431 Stricter validation for DSN ENVID (#1002)
5ad5cb6a4e is described below
commit 5ad5cb6a4e07432502350f4ee1dee9a50fae9d01
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Tue May 24 08:55:35 2022 +0700
JAMES-3431 Stricter validation for DSN ENVID (#1002)
Setting an invalid EndId leads to emails not being
transmissible to third party systems. Stricter validation
fixes this.
We need to match the XText syntax. Provides a utility to
check, encode and decode XText.
See https://datatracker.ietf.org/doc/html/rfc3461#section-4
---
.../main/java/org/apache/mailet/DsnParameters.java | 93 ++++++++++++++++++++++
.../java/org/apache/mailet/DsnParametersTest.java | 68 +++++++++++++---
.../transport/matchers/DSNDelayRequestedTest.java | 2 -
3 files changed, 152 insertions(+), 11 deletions(-)
diff --git a/mailet/api/src/main/java/org/apache/mailet/DsnParameters.java b/mailet/api/src/main/java/org/apache/mailet/DsnParameters.java
index 5d48583c48..11a761fed3 100644
--- a/mailet/api/src/main/java/org/apache/mailet/DsnParameters.java
+++ b/mailet/api/src/main/java/org/apache/mailet/DsnParameters.java
@@ -32,6 +32,7 @@ import org.apache.commons.lang3.tuple.Pair;
import org.apache.james.core.MailAddress;
import com.github.fge.lambdas.Throwing;
+import com.google.common.base.CharMatcher;
import com.google.common.base.Joiner;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
@@ -52,6 +53,96 @@ public class DsnParameters {
public static final String ENVID_PARAMETER = "ENVID";
public static final String RET_PARAMETER = "RET";
+ public static class XText {
+ private static final CharMatcher ESCAPE_CHAR = CharMatcher.isNot('+');
+ // https://datatracker.ietf.org/doc/html/rfc3461#section-4
+ private static CharMatcher XCHAR_VALIDATOR = CharMatcher.inRange('!', '~')
+ .and(CharMatcher.isNot('='))
+ .and(ESCAPE_CHAR);
+ private static CharMatcher HEX_VALIDATOR = CharMatcher.inRange('0', '9')
+ .or(CharMatcher.inRange('A', 'F'));
+
+ static String encode(String text) {
+ if (isOnlyXChar(text)) {
+ return text;
+ }
+ StringBuilder result = new StringBuilder();
+ text.chars()
+ .forEach(i -> encode(i, result));
+ return result.toString();
+ }
+
+ private static StringBuilder encode(int i, StringBuilder result) {
+ if (i < 33 || i > 126 || i == 43 || i == 61) {
+ return result.append('+')
+ .append(String.format("%02X", i));
+ }
+ return result.append((char) i);
+ }
+
+ static String decode(String text) {
+ Preconditions.checkArgument(isValid(text), "Decoding invalid xtext '%s'", text);
+ if (XCHAR_VALIDATOR.matchesAllOf(text)) {
+ return text;
+ }
+ char[] chars = text.toCharArray();
+ StringBuilder result = new StringBuilder();
+ int i = 0;
+ while (i < chars.length) {
+ char c = chars[i];
+ if (c == 43) {
+ // + escape sequence
+ char hex1 = chars[i + 1];
+ char hex2 = chars[i + 2];
+ char hexChar = (char) Integer.parseInt("" + hex1 + hex2, 16);
+ result.append(hexChar);
+ i += 3;
+ } else {
+ result.append(c);
+ i++;
+ }
+ }
+ return result.toString();
+ }
+
+ static boolean isValid(String text) {
+ if (isOnlyXChar(text)) {
+ return true;
+ }
+ char[] chars = text.toCharArray();
+ int i = 0;
+ while (i < chars.length) {
+ char c = chars[i];
+ if (!XCHAR_VALIDATOR.matches(c)) {
+ if (c == 43) {
+ // + escape sequence
+ if (!isValidEscapeSequence(chars, i)) {
+ return false;
+ }
+ i += 3;
+ } else {
+ return false;
+ }
+ } else {
+ i++;
+ }
+ }
+ return true;
+ }
+
+ private static boolean isValidEscapeSequence(char[] chars, int i) {
+ if (i + 3 > chars.length) {
+ return false;
+ }
+ return HEX_VALIDATOR.matches(chars[i + 1])
+ && HEX_VALIDATOR.matches(chars[i + 2]);
+ }
+
+ private static boolean isOnlyXChar(String text) {
+ return XCHAR_VALIDATOR.matchesAllOf(text);
+ }
+ }
+
/**
* RET parameter allow the sender to control which part of the bounced message should be returned to the sender.
*
@@ -104,6 +195,8 @@ public class DsnParameters {
public static EnvId of(String value) {
Preconditions.checkNotNull(value);
+ Preconditions.checkArgument(XText.isValid(value), "According to RFC-3461 EnvId should be a valid xtext" +
+ ", thus composed of CHARs between \"!\" (33) and \"~\" (126) inclusive, except for \"+\" and \"=\" or follow the hexadecimal escape sequence.");
return new EnvId(value);
}
diff --git a/mailet/api/src/test/java/org/apache/mailet/DsnParametersTest.java b/mailet/api/src/test/java/org/apache/mailet/DsnParametersTest.java
index ff0e6c3ee6..ba0c0c8b93 100644
--- a/mailet/api/src/test/java/org/apache/mailet/DsnParametersTest.java
+++ b/mailet/api/src/test/java/org/apache/mailet/DsnParametersTest.java
@@ -29,8 +29,11 @@ import org.apache.mailet.DsnParameters.EnvId;
import org.apache.mailet.DsnParameters.Notify;
import org.apache.mailet.DsnParameters.RecipientDsnParameters;
import org.apache.mailet.DsnParameters.Ret;
+import org.apache.mailet.DsnParameters.XText;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
import com.google.common.collect.ImmutableMap;
@@ -113,19 +116,23 @@ class DsnParametersTest {
@Nested
class EnvIdTest {
@Test
- void parseShouldReturnEmptyOnUnknownValue() {
- assertThat(Ret.parse("unknown")).isEmpty();
+ void ofShouldThrowOnNullValue() {
+ assertThatThrownBy(() -> EnvId.of(null))
+ .isInstanceOf(NullPointerException.class);
}
- @Test
- void parseShouldReturnEmptyOnEmptyValue() {
- assertThat(EnvId.of("").asString()).isEqualTo("");
+ @ParameterizedTest
+ @ValueSource(strings = {"bad envid", "bad=envid", "Bad+envId", "Bad+"})
+ void ofShouldThrowOnBadValue(String badValue) {
+ assertThatThrownBy(() -> EnvId.of(badValue))
+ .isInstanceOf(IllegalArgumentException.class);
}
- @Test
- void ofShouldThrowOnNullValue() {
- assertThatThrownBy(() -> EnvId.of(null))
- .isInstanceOf(NullPointerException.class);
+ @ParameterizedTest
+ @ValueSource(strings = {"good", "000023", "good+32space", "+32", "", "+61", "+43"})
+ void ofShouldAcceptGoodValue(String value) {
+ assertThat(EnvId.of(value).asString())
+ .isEqualTo(value);
}
@Test
@@ -147,6 +154,49 @@ class DsnParametersTest {
}
}
+ @Nested
+ class XTextTest {
+ @ParameterizedTest
+ @ValueSource(strings = {"bad envid", "bad=envid", "Bad+envId", "Bad+", "Bad+2"})
+ void isValidShouldReturnFalseOnBadValue(String badValue) {
+ assertThat(XText.isValid(badValue))
+ .isFalse();
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"bad envid", "bad=envid", "Bad+envId", "Bad+", "Bad+0o", "Bad+2"})
+ void decodeShouldThrowWhenInvalid(String badValue) {
+ assertThatThrownBy(() -> XText.decode(badValue))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"good", "000023", "", "good+32space", "+32", "+61", "+43"})
+ void isValidShouldReturnTrueOnGoodValue(String value) {
+ assertThat(XText.isValid(value))
+ .isTrue();
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"abc", "a+32bc", "", " &*^%$@#%^&#%%kefbvvule89623r757'\"\\/?<>=+-_", "0123"})
+ void encodeDecodeShouldNoop(String value) {
+ assertThat(XText.decode(XText.encode(value)))
+ .isEqualTo(value);
+ }
+
+ @Test
+ void encodeShouldReturnTheGoodValue() {
+ assertThat(XText.encode("abc def+0123=gdef"))
+ .isEqualTo("abc+20def+2B0123+3Dgdef");
+ }
+
+ @Test
+ void decodeShouldReturnTheGoodValue() {
+ assertThat(XText.decode("abc+20def+2B0123+3Dgdef"))
+ .isEqualTo("abc def+0123=gdef");
+ }
+ }
+
@Nested
class NotifyTest {
@Test
diff --git a/server/mailet/mailets/src/test/java/org/apache/james/transport/matchers/DSNDelayRequestedTest.java b/server/mailet/mailets/src/test/java/org/apache/james/transport/matchers/DSNDelayRequestedTest.java
index 7a6f714729..62fe867e18 100644
--- a/server/mailet/mailets/src/test/java/org/apache/james/transport/matchers/DSNDelayRequestedTest.java
+++ b/server/mailet/mailets/src/test/java/org/apache/james/transport/matchers/DSNDelayRequestedTest.java
@@ -27,13 +27,11 @@ import static org.apache.mailet.base.MailAddressFixture.RECIPIENT1;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.junit.jupiter.api.Assertions.*;
import java.util.EnumSet;
import org.apache.james.server.core.MailImpl;
import org.apache.mailet.DsnParameters;
-import org.apache.mailet.base.MailAddressFixture;
import org.apache.mailet.base.test.FakeMatcherConfig;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org