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 0527338b10f6f2b9b12b55eb9a9f7b9a5c8f379c Author: Benoit TELLIER <[email protected]> AuthorDate: Fri Oct 30 08:53:48 2026 +0100 [FIX] Body canonicalizers: keep a lone CR in place, whatever the chunking A CR not followed by LF is ordinary body content, but: - SimpleBodyCanonicalizer dropped a trailing lone CR into the final CRLF, and its write(int) path wrote a pending lone CR before the pending CRLFs while write(byte[]) wrote it after them; - RelaxedBodyCanonicalizer dropped the WSP before any CR, not only before a CRLF. Both now hold a single "CR read, LF not yet" state shared by their two write paths and flush a lone CR in order as soon as the next byte is not LF, or on close, so the output no longer depends on how the body is split. Three SimpleBodyCanonicalizer tests asserted the old trailing CR behaviour and are updated to RFC 6376 section 3.4.3. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../jdkim/canon/RelaxedBodyCanonicalizer.java | 84 ++++++++++++---------- .../james/jdkim/canon/SimpleBodyCanonicalizer.java | 84 +++++++++------------- .../jdkim/canon/RelaxedBodyCanonicalizerTest.java | 23 ++++++ .../jdkim/canon/SimpleBodyCanonicalizerTest.java | 22 +++--- 4 files changed, 116 insertions(+), 97 deletions(-) diff --git a/main/src/main/java/org/apache/james/jdkim/canon/RelaxedBodyCanonicalizer.java b/main/src/main/java/org/apache/james/jdkim/canon/RelaxedBodyCanonicalizer.java index cbecbb9..86d8f95 100644 --- a/main/src/main/java/org/apache/james/jdkim/canon/RelaxedBodyCanonicalizer.java +++ b/main/src/main/java/org/apache/james/jdkim/canon/RelaxedBodyCanonicalizer.java @@ -30,6 +30,8 @@ import java.io.OutputStream; public class RelaxedBodyCanonicalizer extends FilterOutputStream { private boolean pendingSpaces; + // a CR was read: WSP before it is dropped only if it ends the line + private boolean pendingCR; public RelaxedBodyCanonicalizer(OutputStream out) { super(new SimpleBodyCanonicalizer(out, true)); @@ -37,50 +39,50 @@ public class RelaxedBodyCanonicalizer extends FilterOutputStream { } public void write(byte[] buffer, int off, int len) throws IOException { - int start = off; - int end = len + off; - for (int k = off; k < end; k++) { - if (pendingSpaces) { - if (buffer[k] != ' ' && buffer[k] != '\t') { - if (buffer[k] != '\r') - out.write(' '); - pendingSpaces = false; - len = len - k + start; - start = k; - } + int end = off + len; + int k = off; + while (k < end) { + if (pendingSpaces || pendingCR) { + write(buffer[k]); + k++; + continue; + } + // nothing is pending: copy everything up to the next WSP or CR as is + int next = k; + while (next < end && buffer[next] != ' ' && buffer[next] != '\t' && buffer[next] != '\r') { + next++; + } + if (next == k) { + write(buffer[k]); + k++; } else { - if (buffer[k] == ' ' || buffer[k] == '\t') { - if (k + 1 < end && buffer[k] == ' ' && buffer[k + 1] != ' ' - && buffer[k + 1] != '\t' && buffer[k + 1] != '\r') { - // optimization: we skip single spaces - // make sure we optimize only when we are on a space. - } else { - // compute everything from start to end; - out.write(buffer, start, k - start); - pendingSpaces = true; - } - } + out.write(buffer, k, next - k); + k = next; } } - if (!pendingSpaces) { - out.write(buffer, start, len); - } } public void write(int b) throws IOException { - if (pendingSpaces) { - if (b != ' ' && b != '\t') { - if (b != '\r') - out.write(' '); + if (pendingCR) { + pendingCR = false; + if (b == '\n') { + // whitespace at the end of a line is ignored pendingSpaces = false; - out.write(b); + out.write('\r'); + out.write('\n'); + return; } + // a lone CR does not end the line + flushSpaces(); + out.write('\r'); + } + if (b == ' ' || b == '\t') { + pendingSpaces = true; + } else if (b == '\r') { + pendingCR = true; } else { - if (b == ' ' || b == '\t') { - pendingSpaces = true; - } else { - out.write(b); - } + flushSpaces(); + out.write(b); } } @@ -93,8 +95,18 @@ public class RelaxedBodyCanonicalizer extends FilterOutputStream { * Called internally to make sure we output the buffered whitespace if any. */ private void complete() throws IOException { - if (pendingSpaces) + flushSpaces(); + if (pendingCR) { + pendingCR = false; + out.write('\r'); + } + } + + private void flushSpaces() throws IOException { + if (pendingSpaces) { out.write(' '); + pendingSpaces = false; + } } } \ No newline at end of file diff --git a/main/src/main/java/org/apache/james/jdkim/canon/SimpleBodyCanonicalizer.java b/main/src/main/java/org/apache/james/jdkim/canon/SimpleBodyCanonicalizer.java index 359cbfe..6e5f8d6 100644 --- a/main/src/main/java/org/apache/james/jdkim/canon/SimpleBodyCanonicalizer.java +++ b/main/src/main/java/org/apache/james/jdkim/canon/SimpleBodyCanonicalizer.java @@ -32,7 +32,9 @@ public class SimpleBodyCanonicalizer extends FilterOutputStream { private static final boolean DEEP_DEBUG = false; private final boolean emptyBodyIsEmpty; + // a CR was read and the next byte tells whether it ends a line private boolean lastWasCR; + // CRLFs read but not written yet: only the trailing ones are dropped private int countCRLF; private boolean hasContent; @@ -51,62 +53,44 @@ public class SimpleBodyCanonicalizer extends FilterOutputStream { } public void write(byte[] b, int off, int len) throws IOException { - if (len <= 0) - return; if (DEEP_DEBUG) System.out.println("I:(" + lastWasCR + "|" + countCRLF + ") [" + new String(b, off, len) + "]"); - if (lastWasCR) { - if (b[off] == '\n') { - countCRLF++; - lastWasCR = false; - off++; - len--; - } else { - // TODO output the lone \r ? (this condition should never happen - // as we expect only CRLF in a compliant 7bit email. - out.write('\r'); - hasContent = true; - lastWasCR = false; + int end = off + len; + int i = off; + while (i < end) { + if (lastWasCR || countCRLF > 0 || b[i] == '\r') { + write(b[i]); + i++; + continue; } - } - int newCountCRLF = 0; - boolean newLastWasCR = false; - if (len >= 1 && b[off + len - 1] == '\r') { - newLastWasCR = true; - len--; - } - while (len >= 2 && b[off + len - 1] == '\n' && b[off + len - 2] == '\r') { - len -= 2; - newCountCRLF++; - } - if (len > 0) { - dumpCRLF(); - out.write(b, off, len); + // nothing is pending: copy everything up to the next CR as is + int next = i; + while (next < end && b[next] != '\r') { + next++; + } + out.write(b, i, next - i); hasContent = true; + i = next; } - countCRLF += newCountCRLF; - lastWasCR = newLastWasCR; } public void write(int b) throws IOException { if (DEEP_DEBUG) System.out.println("B:(" + lastWasCR + "|" + countCRLF + ") [" + (char) b + "]"); - if (lastWasCR && '\n' == b) { + if (lastWasCR) { lastWasCR = false; - countCRLF++; - } else { - if (!lastWasCR && '\r' == b) { - lastWasCR = true; - } else { - dumpCRLF(); - if ('\r' == b) { - lastWasCR = true; - } else { - out.write(b); - hasContent = true; - } + if ('\n' == b) { + countCRLF++; + return; } + // a lone CR is ordinary content (RFC 6376 section 3.4.3) + writeContent('\r'); + } + if ('\r' == b) { + lastWasCR = true; + } else { + writeContent(b); } } @@ -119,24 +103,24 @@ public class SimpleBodyCanonicalizer extends FilterOutputStream { if (DEEP_DEBUG) System.out.println("C:(" + lastWasCR + "|" + countCRLF + ")"); if (lastWasCR) { - // if the last char was a CR we'll let dumpCRLF - // to output the missing \n lastWasCR = false; + writeContent('\r'); } if (emptyBodyIsEmpty && !hasContent) { return; } - countCRLF = 1; + out.write("\r\n".getBytes()); + } + + private void writeContent(int b) throws IOException { dumpCRLF(); + out.write(b); + hasContent = true; } private void dumpCRLF() throws IOException { if (DEEP_DEBUG) System.out.println("D:(" + lastWasCR + "|" + countCRLF + ")"); - if (lastWasCR) { - out.write('\r'); - lastWasCR = false; - } while (countCRLF > 0) { out.write("\r\n".getBytes()); countCRLF--; diff --git a/main/src/test/java/org/apache/james/jdkim/canon/RelaxedBodyCanonicalizerTest.java b/main/src/test/java/org/apache/james/jdkim/canon/RelaxedBodyCanonicalizerTest.java index d571747..4fb7658 100644 --- a/main/src/test/java/org/apache/james/jdkim/canon/RelaxedBodyCanonicalizerTest.java +++ b/main/src/test/java/org/apache/james/jdkim/canon/RelaxedBodyCanonicalizerTest.java @@ -19,6 +19,7 @@ package org.apache.james.jdkim.canon; +import java.io.OutputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.security.NoSuchAlgorithmException; @@ -68,4 +69,26 @@ public class RelaxedBodyCanonicalizerTest extends AbstractOutputStreamTestCase { os.close(); assertArrayEquals("\r\nx\r\n".getBytes(), bos.toByteArray()); } + + protected OutputStream newInstance(ByteArrayOutputStream bos) { + return new RelaxedBodyCanonicalizer(bos); + } + + public void testLoneCRDoesNotEndTheLine() throws IOException { + extensiveChunker("\r".getBytes(), "\r\r\n".getBytes()); + extensiveChunker("a \t\rb\r\n".getBytes(), "a \rb\r\n".getBytes()); + extensiveChunker("\t\r\n \r \r\t".getBytes(), "\r\n \r \r \r\n".getBytes()); + extensiveChunker("a \r \r\n".getBytes(), "a \r\r\n".getBytes()); + } + + public void testSingleBytesAndChunksAgree() throws IOException { + byte[] body = "x \r\r\n\r \t\r\ny\t".getBytes(); + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + OutputStream os = newInstance(bytes); + for (byte b : body) { + os.write(b); + } + os.close(); + extensiveChunker(body, bytes.toByteArray()); + } } diff --git a/main/src/test/java/org/apache/james/jdkim/canon/SimpleBodyCanonicalizerTest.java b/main/src/test/java/org/apache/james/jdkim/canon/SimpleBodyCanonicalizerTest.java index e56b688..23bfed1 100644 --- a/main/src/test/java/org/apache/james/jdkim/canon/SimpleBodyCanonicalizerTest.java +++ b/main/src/test/java/org/apache/james/jdkim/canon/SimpleBodyCanonicalizerTest.java @@ -88,10 +88,6 @@ public class SimpleBodyCanonicalizerTest extends AbstractOutputStreamTestCase { public void testWrongCRSequences() throws NoSuchAlgorithmException, IOException { - // byte[] test = "this is a \r\n canonica\rlizati\r\ron - // \ttest\r\n\r\n\r\r".getBytes(); - // byte[] expected = "this is a \r\n canonica\rlizati\r\ron - // \ttest\r\n\r\n\r\r\n".getBytes(); byte[] test = "this is a \r\n canonica\rlizati".getBytes(); byte[] expected = "this is a \r\n canonica\rlizati\r\n".getBytes(); extensiveChunker(test, expected); @@ -114,13 +110,10 @@ public class SimpleBodyCanonicalizerTest extends AbstractOutputStreamTestCase { public void testWrongCRSequencesAdv() throws NoSuchAlgorithmException, IOException { - // byte[] test = "this is a \r\n canonica\rlizati\r\ron - // \ttest\r\n\r\n\r\r".getBytes(); - // byte[] expected = "this is a \r\n canonica\rlizati\r\ron - // \ttest\r\n\r\n\r\r\n".getBytes(); byte[] test = "this is a \r\n canonica\rlizati\r\ron\r\n\r\n\r" .getBytes(); - byte[] expected = "this is a \r\n canonica\rlizati\r\ron\r\n" + // the final lone CR is content: the CRLFs before it are not trailing + byte[] expected = "this is a \r\n canonica\rlizati\r\ron\r\n\r\n\r\r\n" .getBytes(); extensiveChunker(test, expected); } @@ -129,7 +122,8 @@ public class SimpleBodyCanonicalizerTest extends AbstractOutputStreamTestCase { IOException { byte[] test = "this is a \r\n canonica\rlizati\r\ron\r\n\r\n\r" .getBytes(); - byte[] expected = "this is a \r\n canonica\rlizati\r\ron\r\n" + // the final lone CR is content: the CRLFs before it are not trailing + byte[] expected = "this is a \r\n canonica\rlizati\r\ron\r\n\r\n\r\r\n" .getBytes(); ByteArrayOutputStream bos = new ByteArrayOutputStream(); SimpleBodyCanonicalizer os = new SimpleBodyCanonicalizer(bos); @@ -143,7 +137,8 @@ public class SimpleBodyCanonicalizerTest extends AbstractOutputStreamTestCase { IOException { byte[] test = "this is a \r\n canonica\rlizati\r\ron\r\n\r\n\r" .getBytes(); - byte[] expected = "this is a \r\n canonica\rlizati\r\ron\r\n" + // the final lone CR is content: the CRLFs before it are not trailing + byte[] expected = "this is a \r\n canonica\rlizati\r\ron\r\n\r\n\r\r\n" .getBytes(); ByteArrayOutputStream bos = new ByteArrayOutputStream(); SimpleBodyCanonicalizer os = new SimpleBodyCanonicalizer(bos); @@ -155,4 +150,9 @@ public class SimpleBodyCanonicalizerTest extends AbstractOutputStreamTestCase { assertArrayEquals(expected, bos.toByteArray()); } + public void testLoneCRIsKeptInPlace() throws IOException { + extensiveChunker("\r".getBytes(), "\r\r\n".getBytes()); + extensiveChunker("\r\n\r\r".getBytes(), "\r\n\r\r\r\n".getBytes()); + extensiveChunker("a\r\n\r\nb\r\r\n\r\n".getBytes(), "a\r\n\r\nb\r\r\n".getBytes()); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
