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-mime4j.git
commit dbc108d8a34e3ff1f0e76520df34c67a60dac6e1 Author: Benoit TELLIER <[email protected]> AuthorDate: Thu Nov 5 05:30:15 2026 +0100 [FIX] DOM builders: do not silently replace the text a charset cannot encode The text of a TextBody is encoded with CodingErrorAction.REPLACE, so a text its charset cannot encode was written with "?": the sent body differed from the built one, with no error and no log. The builders and BasicBodyFactory now check the text: - a charset chosen by the caller has to encode it, otherwise IllegalArgumentException. So does a lone surrogate, in any charset; - when the charset is only a default, a text that the default cannot encode takes UTF-8, and the part is labelled with it. --- .../org/apache/james/mime4j/util/CharsetUtil.java | 26 +++++++++ .../java/org/apache/james/mime4j/dom/Message.java | 4 +- .../james/mime4j/message/BasicBodyFactory.java | 8 +-- .../james/mime4j/message/BodyPartBuilder.java | 4 +- .../james/mime4j/message/MultipartBuilder.java | 6 ++- .../james/mime4j/message/SingleBodyBuilder.java | 5 +- .../james/mime4j/message/MultipartBuilderTest.java | 61 ++++++++++++++++++++++ 7 files changed, 103 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/org/apache/james/mime4j/util/CharsetUtil.java b/core/src/main/java/org/apache/james/mime4j/util/CharsetUtil.java index 23ae8912..3b10e144 100644 --- a/core/src/main/java/org/apache/james/mime4j/util/CharsetUtil.java +++ b/core/src/main/java/org/apache/james/mime4j/util/CharsetUtil.java @@ -215,4 +215,30 @@ public class CharsetUtil { return Charsets.UTF_8; } + /** + * Returns the charset to encode a text with, so that the text is not silently altered. + * A charset chosen by the caller has to encode the whole text. Without a charset, the text + * stays in the default one (<code>null</code> is returned) or else takes UTF-8. + * + * @param charset the charset the caller chose, may be <code>null</code>. + * @param text the text to encode. + * @return the charset to use and to label the text with, <code>null</code> when the text fits + * the default charset and the caller chose none. + * @throws IllegalArgumentException if the charset cannot encode the text. A lone surrogate + * cannot be encoded by any charset. + */ + public static Charset forEncoding(final Charset charset, final CharSequence text) { + Charset cs = forEncoding(charset); + if (cs == null) { + if (Charsets.DEFAULT_CHARSET.newEncoder().canEncode(text)) { + return null; + } + cs = Charsets.UTF_8; + } + if (!cs.newEncoder().canEncode(text)) { + throw new IllegalArgumentException("Charset " + cs.name() + " cannot encode the text"); + } + return cs; + } + } diff --git a/dom/src/main/java/org/apache/james/mime4j/dom/Message.java b/dom/src/main/java/org/apache/james/mime4j/dom/Message.java index f640a431..4d521897 100644 --- a/dom/src/main/java/org/apache/james/mime4j/dom/Message.java +++ b/dom/src/main/java/org/apache/james/mime4j/dom/Message.java @@ -399,8 +399,8 @@ public interface Message extends Entity, Body { * "xml"). */ public Builder setBody(String text, String subtype, Charset charset) throws IOException { - // The text gets encoded: the label has to name a charset that encodes - charset = CharsetUtil.forEncoding(charset); + // The text gets encoded: the label has to name a charset that encodes it + charset = CharsetUtil.forEncoding(charset, text); String mimeType = "text/" + (subtype != null ? subtype : "plain"); if (charset != null) { setField(Fields.contentType(mimeType, new NameValuePair("charset", charset.name()))); diff --git a/dom/src/main/java/org/apache/james/mime4j/message/BasicBodyFactory.java b/dom/src/main/java/org/apache/james/mime4j/message/BasicBodyFactory.java index d017699c..4c14ebf5 100644 --- a/dom/src/main/java/org/apache/james/mime4j/message/BasicBodyFactory.java +++ b/dom/src/main/java/org/apache/james/mime4j/message/BasicBodyFactory.java @@ -105,7 +105,7 @@ public class BasicBodyFactory implements BodyFactory { if (text == null) { throw new IllegalArgumentException("Text may not be null"); } - return new StringBody1(text, resolveCharset(mimeCharset)); + return new StringBody1(text, CharsetUtil.forEncoding(resolveCharset(mimeCharset), text)); } public TextBody textBody(final byte[] content, final Charset charset) { @@ -126,11 +126,13 @@ public class BasicBodyFactory implements BodyFactory { if (text == null) { throw new IllegalArgumentException("Text may not be null"); } - return new StringBody1(text, charset); + return new StringBody1(text, CharsetUtil.forEncoding(charset, text)); } public TextBody textBody(final String text) { - return textBody(text, Charsets.DEFAULT_CHARSET); + // No charset chosen: the default one, or UTF-8 for a text it cannot encode + Charset charset = CharsetUtil.forEncoding(null, text); + return textBody(text, charset != null ? charset : Charsets.DEFAULT_CHARSET); } public BinaryBody binaryBody(final String content, final Charset charset) { diff --git a/dom/src/main/java/org/apache/james/mime4j/message/BodyPartBuilder.java b/dom/src/main/java/org/apache/james/mime4j/message/BodyPartBuilder.java index 2caf05bc..bc85494e 100644 --- a/dom/src/main/java/org/apache/james/mime4j/message/BodyPartBuilder.java +++ b/dom/src/main/java/org/apache/james/mime4j/message/BodyPartBuilder.java @@ -170,8 +170,8 @@ public class BodyPartBuilder extends AbstractEntityBuilder { * "xml"). */ public BodyPartBuilder setBody(String text, String subtype, Charset charset) throws IOException { - // The text gets encoded: the label has to name a charset that encodes - charset = CharsetUtil.forEncoding(charset); + // The text gets encoded: the label has to name a charset that encodes it + charset = CharsetUtil.forEncoding(charset, text); String mimeType = "text/" + (subtype != null ? subtype : "plain"); if (charset != null) { setField(Fields.contentType(mimeType, new NameValuePair("charset", charset.name()))); diff --git a/dom/src/main/java/org/apache/james/mime4j/message/MultipartBuilder.java b/dom/src/main/java/org/apache/james/mime4j/message/MultipartBuilder.java index d6d5a3ea..c5817a4e 100644 --- a/dom/src/main/java/org/apache/james/mime4j/message/MultipartBuilder.java +++ b/dom/src/main/java/org/apache/james/mime4j/message/MultipartBuilder.java @@ -231,8 +231,10 @@ public class MultipartBuilder { } public MultipartBuilder addTextPart(String text, Charset charset) throws IOException { - // The text gets encoded: the label has to name a charset that encodes - Charset cs = charset != null ? CharsetUtil.forEncoding(charset) : Charsets.ISO_8859_1; + // The text gets encoded: the label has to name a charset that encodes it + Charset cs = charset != null ? charset + : Charsets.ISO_8859_1.newEncoder().canEncode(text) ? Charsets.ISO_8859_1 : Charsets.UTF_8; + cs = CharsetUtil.forEncoding(cs, text); TextBody body = bodyFactory != null ? bodyFactory.textBody( InputStreams.create(text, cs), cs.name()) : BasicBodyFactory.INSTANCE.textBody(text, cs); BodyPart bodyPart = BodyPartBuilder.create() diff --git a/dom/src/main/java/org/apache/james/mime4j/message/SingleBodyBuilder.java b/dom/src/main/java/org/apache/james/mime4j/message/SingleBodyBuilder.java index d8460931..8921049b 100644 --- a/dom/src/main/java/org/apache/james/mime4j/message/SingleBodyBuilder.java +++ b/dom/src/main/java/org/apache/james/mime4j/message/SingleBodyBuilder.java @@ -106,8 +106,9 @@ public class SingleBodyBuilder { public TextBody buildText() throws IOException { Charset cs = this.charset != null ? this.charset : Charsets.DEFAULT_CHARSET; if (this.text != null) { - // The text gets encoded: the label has to name a charset that encodes - cs = CharsetUtil.forEncoding(cs); + // The text gets encoded: the label has to name a charset that encodes it + Charset encoding = CharsetUtil.forEncoding(this.charset, this.text); + cs = encoding != null ? encoding : Charsets.DEFAULT_CHARSET; } if (this.bodyFactory != null) { if (this.text != null) { diff --git a/dom/src/test/java/org/apache/james/mime4j/message/MultipartBuilderTest.java b/dom/src/test/java/org/apache/james/mime4j/message/MultipartBuilderTest.java index 25de31ef..4456dcba 100644 --- a/dom/src/test/java/org/apache/james/mime4j/message/MultipartBuilderTest.java +++ b/dom/src/test/java/org/apache/james/mime4j/message/MultipartBuilderTest.java @@ -19,9 +19,13 @@ package org.apache.james.mime4j.message; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.InputStream; +import java.io.Reader; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; @@ -289,4 +293,61 @@ public class MultipartBuilderTest { } return sb.toString(); } + + @Test + public void textBodyInACharsetThatCannotEncodeItShouldBeRejected() throws Exception { + assertThatThrownBy(() -> Message.Builder.of().setBody("caf\u00e9", "plain", StandardCharsets.US_ASCII)) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> BodyPartBuilder.create().setBody("caf\u00e9", "plain", StandardCharsets.US_ASCII)) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> MultipartBuilder.create("mixed").addTextPart("\u20ac", StandardCharsets.ISO_8859_1)) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> BasicBodyFactory.INSTANCE.textBody("caf\u00e9", StandardCharsets.US_ASCII)) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> SingleBodyBuilder.create().setText("caf\u00e9").setCharset(StandardCharsets.US_ASCII).buildText()) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void textBodyWithALoneSurrogateShouldBeRejected() throws Exception { + assertThatThrownBy(() -> Message.Builder.of().setBody("a\ud83d", "plain", StandardCharsets.UTF_8)) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> MultipartBuilder.create("mixed").addTextPart("a\ud83d", null)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void textWithoutACharsetShouldTakeOneThatCarriesIt() throws Exception { + Message part = Message.Builder.of() + .setBody(MultipartBuilder.create("mixed").addTextPart("\u20ac", null).build()) + .build(); + assertThat(readText(((Multipart) writeAndParse(part).getBody()).getBodyParts().get(0))).isEqualTo("\u20ac"); + + Message message = Message.Builder.of().setBody("caf\u00e9", "plain", null).build(); + assertThat(readText(writeAndParse(message))).isEqualTo("caf\u00e9"); + + assertThat(BasicBodyFactory.INSTANCE.textBody("caf\u00e9").getMimeCharset()).isEqualTo("UTF-8"); + assertThat(BasicBodyFactory.INSTANCE.textBody("cafe").getMimeCharset()).isEqualTo("US-ASCII"); + assertThat(SingleBodyBuilder.create().setText("caf\u00e9").buildText().getMimeCharset()).isEqualTo("UTF-8"); + // What the default charset carries keeps its label + assertThat(((ContentTypeField) ((Multipart) Message.Builder.of() + .setBody(MultipartBuilder.create("mixed").addTextPart("caf\u00e9", null).build()).build().getBody()) + .getBodyParts().get(0).getHeader().getField("Content-Type")).getCharset()).isEqualTo("ISO-8859-1"); + } + + private static Message writeAndParse(Message message) throws Exception { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + new DefaultMessageWriter().writeMessage(message, out); + return new DefaultMessageBuilder().parseMessage(new ByteArrayInputStream(out.toByteArray())); + } + + private static String readText(Entity entity) throws Exception { + StringBuilder read = new StringBuilder(); + try (Reader reader = ((TextBody) entity.getBody()).getReader()) { + for (int c = reader.read(); c != -1; c = reader.read()) { + read.append((char) c); + } + } + return read.toString(); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
