This is an automated email from the ASF dual-hosted git repository. chibenwa pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-jdkim.git
commit ca14ffd1f8a742a52ac67916a1c27cad6fdfe37e Author: Benoit TELLIER <[email protected]> AuthorDate: Fri Oct 30 08:39:48 2026 +0100 [FIX] DKIMQuotedPrintable: follow the RFC 6376 grammar strictly The decoder copied every character it did not handle, control and 8-bit characters and ';' included, and skipped CR and LF one by one, so a bare LF followed by WSP was taken as folding. It now rejects any character outside dkim-safe-char and accepts CR and LF only as the CRLF of an FWS followed by WSP. The default identity, built from d= when i= is absent, is no longer decoded as DKIM-Quoted-Printable. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../james/jdkim/parser/DKIMQuotedPrintable.java | 35 +++++++++++++++++----- .../james/jdkim/tagvalue/SignatureRecordImpl.java | 3 ++ .../jdkim/tagvalue/SignatureRecordTemplate.java | 3 ++ .../jdkim/parser/DKIMQuotedPrintableTest.java | 18 +++++++++++ 4 files changed, 52 insertions(+), 7 deletions(-) diff --git a/main/src/main/java/org/apache/james/jdkim/parser/DKIMQuotedPrintable.java b/main/src/main/java/org/apache/james/jdkim/parser/DKIMQuotedPrintable.java index 7a0de9b..094aec8 100644 --- a/main/src/main/java/org/apache/james/jdkim/parser/DKIMQuotedPrintable.java +++ b/main/src/main/java/org/apache/james/jdkim/parser/DKIMQuotedPrintable.java @@ -25,9 +25,8 @@ public class DKIMQuotedPrintable { public static String dkimQuotedPrintableDecode(CharSequence input) throws IllegalArgumentException { StringBuilder sb = new StringBuilder(input.length()); - // TODO should we fail on WSP that is not part of FWS? - // the specification in 2.6 DKIM-Quoted-Printable is not - // clear + // RFC 6376 section 2.11: + // dkim-quoted-printable = *(FWS / hex-octet / dkim-safe-char) int state = 0; int start = 0; int d = 0; @@ -41,20 +40,33 @@ public class DKIMQuotedPrintable { switch (state) { case 0: switch (input.charAt(i)) { + case '\r': + if (i + 1 >= input.length() || input.charAt(i + 1) != '\n') + throw new IllegalArgumentException( + "Unexpected CR not part of an FWS"); + // fall through case ' ': case '\t': - case '\r': - case '\n': - if ('\n' == input.charAt(i)) - lastWasNL = true; sb.append(input.subSequence(start, i)); start = i + 1; // ignoring whitespace by now. break; + case '\n': + if (i == 0 || input.charAt(i - 1) != '\r') + throw new IllegalArgumentException( + "Unexpected LF not part of an FWS"); + lastWasNL = true; + sb.append(input.subSequence(start, i)); + start = i + 1; + break; case '=': sb.append(input.subSequence(start, i)); state = 1; break; + default: + if (!isDkimSafeChar(input.charAt(i))) + throw new IllegalArgumentException( + "Invalid character at " + i); } break; case 1: @@ -78,6 +90,10 @@ public class DKIMQuotedPrintable { } } } + if (lastWasNL) { + throw new IllegalArgumentException( + "Unexpected LF not part of an FWS"); + } if (state != 0) { throw new IllegalArgumentException( "Invalid quoted printable termination"); @@ -85,4 +101,9 @@ public class DKIMQuotedPrintable { sb.append(input.subSequence(start, input.length())); return sb.toString(); } + + // dkim-safe-char = %x21-3A / %x3C / %x3E-7E + private static boolean isDkimSafeChar(char c) { + return c >= 0x21 && c <= 0x7E && c != ';' && c != '='; + } } diff --git a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java index 2836359..75104e9 100644 --- a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java +++ b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java @@ -190,6 +190,9 @@ public class SignatureRecordImpl extends TagValue implements SignatureRecord { * @see org.apache.james.jdkim.api.SignatureRecord#getIdentity() */ public CharSequence getIdentity() { + // the default identity is built from d=, it is not DKIM-Quoted-Printable + if (!containsTag("i")) + return getValue("i"); return dkimQuotedPrintableDecode(getValue("i")); } diff --git a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java index 32f294a..e7f27ee 100644 --- a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java +++ b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java @@ -174,6 +174,9 @@ public class SignatureRecordTemplate extends TagValue implements SignatureRecord * @see SignatureRecord#getIdentity() */ public CharSequence getIdentity() { + // the default identity is built from d=, it is not DKIM-Quoted-Printable + if (!containsTag("i")) + return getValue("i"); return DKIMQuotedPrintable.dkimQuotedPrintableDecode(getValue("i")); } diff --git a/main/src/test/java/org/apache/james/jdkim/parser/DKIMQuotedPrintableTest.java b/main/src/test/java/org/apache/james/jdkim/parser/DKIMQuotedPrintableTest.java index 617aff5..26e68ba 100644 --- a/main/src/test/java/org/apache/james/jdkim/parser/DKIMQuotedPrintableTest.java +++ b/main/src/test/java/org/apache/james/jdkim/parser/DKIMQuotedPrintableTest.java @@ -100,4 +100,22 @@ public class DKIMQuotedPrintableTest { } // TODO UTF-8 see https://datatracker.ietf.org/doc/html/rfc6376#section-3.2 + + @Test + public void unsafeCharactersAndBareLineBreaksShouldBeRejected() { + assertThatThrownBy(() -> DKIMQuotedPrintable.dkimQuotedPrintableDecode("\u001d")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> DKIMQuotedPrintable.dkimQuotedPrintableDecode("\u00e9")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> DKIMQuotedPrintable.dkimQuotedPrintableDecode("a\n b")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> DKIMQuotedPrintable.dkimQuotedPrintableDecode("\n ")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> DKIMQuotedPrintable.dkimQuotedPrintableDecode("a\r b")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> DKIMQuotedPrintable.dkimQuotedPrintableDecode("a\r\n")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> DKIMQuotedPrintable.dkimQuotedPrintableDecode("a;b")) + .isInstanceOf(IllegalArgumentException.class); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
