Email body encoding is hard coded to ISO-8859-1
karelmaxa opened this issue · 5 comments
Current implementation of EmailClient disallow specification of content type charset. Therefore, encoding is always set to ISO-8859-1 and some characters are not encoded properly (charset is resolved here).
Email client should allow content type with charset as it is valid header (See RFC 2616).
I had a look into what is happening. The underlying issue is that javax.mail is checking target encoding by capturing output generated via javax.activation's DataHandler and under the hood it is javax.mail's own text_plain handler that is being picked and used. However text_plain uses ISO charset as default charset so the encoding test is being done on broken stream.
Thread [main] (Suspended)
owns: OutputStreamWriter (id=40)
owns: MimeMessage (id=19)
AsciiOutputStream.check(int) line: 1681
AsciiOutputStream.write(int) line: 1647
text_plain$NoCloseOutputStream(FilterOutputStream).write(int) line: 77
text_plain$NoCloseOutputStream(FilterOutputStream).write(byte[], int, int) line: 125
StreamEncoder.writeBytes() line: 221 [local variables unavailable]
StreamEncoder.implClose() line: 316 [local variables unavailable]
StreamEncoder.close() line: 149 [local variables unavailable]
OutputStreamWriter.close() line: 233
text_plain.writeTo(Object, String, OutputStream) line: 138
ObjectDataContentHandler.writeTo(Object, String, OutputStream) line: 889
DataHandler.writeTo(OutputStream) line: 317
MimeUtility.getEncoding(DataHandler) line: 316
MimeBodyPart.updateHeaders(MimePart) line: 1551
MimeMessage.updateHeaders() line: 2247
MimeMessage.saveChanges() line: 2207
MailTest.main(String[]) line: 24
Previously as javax.activation was part of JRE it was not able to resolve handlers from javax.mail as they were on bundle classpath. So the current issue is that either javax.activation has access to javax.mail handlers via bundle wiring or we have thread context class-loader set.
And finally - I am not sure that it is safe to simply call MimeMessage#setContent like we do in EmailClient. Depending on javax.mail to correctly guess charset seems unstable. So it might be better to rewrite message construction logic as well (EmailClient)... or at least make sure we allow charset in content type.
Btw. we are using Activation from ServiceMix and Mail from Sun maven groups. I think we should switch to Jakarta artifacts (hope they have correct OSGi headers). Also those artifacts should be defined in Wren:IDM and not transitively resolved from some "random" (wrensec-commons/json-resource) library.
Related to wrensec-commons#28 .
Bundle manifest information for Activation and Mail variants:
javax.activation:activation:1.1.1
/mnt/cache/mvn/javax/activation/activation/1.1.1$ bnd print -Cci activation-1.1.1.jar
[IMPEXP]
[CAPABILITIES]
[COMPONENTS]
org.apache.servicemix.specs:org.apache.servicemix.specs.activation-api-1.1:2.9.0
/mnt/cache/mvn/org/apache/servicemix/specs/org.apache.servicemix.specs.activation-api-1.1/2.9.0$ bnd print -Cci org.apache.servicemix.specs.activation-api-1.1-2.9.0.jar
[IMPEXP]
Import-Package
org.osgi.framework {version=[1.5,2)}
Export-Package
javax.activation {version=1.1}
[CAPABILITIES]
[COMPONENTS]
com.sun.activation:jakarta.activation:1.2.2
/mnt/cache/mvn/com/sun/activation/jakarta.activation/1.2.2$ bnd print -Cci jakarta.activation-1.2.2.jar
[IMPEXP]
Import-Package
com.sun.activation.registries {version=[1.2,2)}
javax.activation {version=[1.2,2)}
Export-Package
com.sun.activation.registries {version=1.2.2, imported-as=[1.2,2)}
com.sun.activation.viewers {version=1.2.2}
javax.activation {version=1.2, imported-as=[1.2,2)}
[CAPABILITIES]
Require-Capability
osgi.ee {filter:=(&(osgi.ee=JavaSE)(version=9.0))}
[COMPONENTS]
com.sun.activation:jakarta.activation::1.2.1
/mnt/cache/mvn/com/sun/activation/jakarta.activation/1.2.1$ bnd print -Cci jakarta.activation-1.2.1.jar
[IMPEXP]
Import-Package
com.sun.activation.registries {version=1.2}
com.sun.activation.viewers {version=1.2}
javax.activation {version=1.2}
Export-Package
com.sun.activation.registries {version=1.2.1, imported-as=1.2}
com.sun.activation.viewers {version=1.2.1, imported-as=1.2}
javax.activation {version=1.2, imported-as=1.2}
[CAPABILITIES]
[COMPONENTS]
com.sun.mail:javax.mail:1.6.2
/mnt/cache/mvn/com/sun/mail/javax.mail/1.6.2$ bnd print -Cci javax.mail-1.6.2.jar
[IMPEXP]
Import-Package
com.sun.mail.iap {version=[1.6,2)}
com.sun.mail.imap {version=[1.6,2)}
com.sun.mail.util {version=[1.6,2)}
javax.activation
javax.crypto
javax.crypto.spec
javax.mail {version=[1.6,2)}
javax.mail.event {version=[1.6,2)}
javax.mail.internet {version=[1.6,2)}
javax.mail.search {version=[1.6,2)}
javax.mail.util {version=[1.6,2)}
javax.net
javax.net.ssl
javax.security.auth.callback
javax.security.auth.x500
javax.security.sasl {resolution:=optional}
javax.xml.transform
javax.xml.transform.stream
sun.security.util {resolution:=optional}
Export-Package
com.sun.mail.handlers {version=1.6.2}
com.sun.mail.iap {version=1.6.2, imported-as=[1.6,2)}
com.sun.mail.imap {version=1.6.2, imported-as=[1.6,2)}
com.sun.mail.imap.protocol {version=1.6.2}
com.sun.mail.pop3 {version=1.6.2}
com.sun.mail.smtp {version=1.6.2}
com.sun.mail.util {version=1.6.2, imported-as=[1.6,2)}
com.sun.mail.util.logging {version=1.6.2}
javax.mail {version=1.6, imported-as=[1.6,2)}
javax.mail.event {version=1.6, imported-as=[1.6,2)}
javax.mail.internet {version=1.6, imported-as=[1.6,2)}
javax.mail.search {version=1.6, imported-as=[1.6,2)}
javax.mail.util {version=1.6, imported-as=[1.6,2)}
[CAPABILITIES]
Require-Capability
osgi.ee {filter:=(&(osgi.ee=JavaSE)(version=1.7))}
[COMPONENTS]
com.sun.mail:jakarta.mail:1.6.5
/mnt/cache/mvn/com/sun/mail/jakarta.mail/1.6.5$ bnd print -Cci jakarta.mail-1.6.5.jar
[IMPEXP]
Import-Package
com.sun.mail.iap {version=[1.6,2)}
com.sun.mail.imap {version=[1.6,2)}
com.sun.mail.util {version=[1.6,2)}
javax.activation
javax.crypto
javax.crypto.spec
javax.mail {version=[1.6,2)}
javax.mail.event {version=[1.6,2)}
javax.mail.internet {version=[1.6,2)}
javax.mail.search {version=[1.6,2)}
javax.mail.util {version=[1.6,2)}
javax.net
javax.net.ssl
javax.security.auth.callback
javax.security.auth.x500
javax.security.sasl {resolution:=optional}
javax.xml.transform
javax.xml.transform.stream
sun.security.util {resolution:=optional}
Export-Package
com.sun.mail.handlers {version=1.6.5}
com.sun.mail.iap {version=1.6.5, imported-as=[1.6,2)}
com.sun.mail.imap {version=1.6.5, imported-as=[1.6,2)}
com.sun.mail.imap.protocol {version=1.6.5}
com.sun.mail.pop3 {version=1.6.5}
com.sun.mail.smtp {version=1.6.5}
com.sun.mail.util {version=1.6.5, imported-as=[1.6,2)}
com.sun.mail.util.logging {version=1.6.5}
javax.mail {version=1.6, imported-as=[1.6,2)}
javax.mail.event {version=1.6, imported-as=[1.6,2)}
javax.mail.internet {version=1.6, imported-as=[1.6,2)}
javax.mail.search {version=1.6, imported-as=[1.6,2)}
javax.mail.util {version=1.6, imported-as=[1.6,2)}
[CAPABILITIES]
Require-Capability
osgi.ee {filter:=(&(osgi.ee=JavaSE)(version=1.7))}
[COMPONENTS]
Few observations regarding Activation:
- ServiceMix Activation variant was historically required as the original activation artifact doesn't have bundle metadata
- Jakarta's legacy variant has bundle metadata so we don't need ServiceMix and can use that one
- We can't use Activation 1.2.2 as it requires JRE 9+ (we are still maintaining JRE 8 compatibility)
Few observations regarding Mail:
- We can switch to Jakarta's legacy variant
Our next major version should consider switiching to full Jakarta artifacts. However that would require major refactor in commons (changing package names).
Ehm... the issue is probably custom activator in ServiceMix bundle that eagerly registers command map from all bundles - https://github.com/apache/servicemix-specs/blob/master/activation-api-1.2.1/src/main/java/org/apache/servicemix/specs/activation/Activator.java#L56 . We should get rid of this dependency.
Daemon Thread [FelixStartLevel] (Suspended (breakpoint at line 252 in MailcapCommandMap))
owns: OsgiMailcapCommandMap (id=550)
owns: Activator (id=551)
OsgiMailcapCommandMap(MailcapCommandMap).addMailcap(String) line: 252
OsgiMailcapCommandMap.addMailcap(String, Bundle) line: 40
Activator.rebuildCommandMap() line: 99
Activator.register(Bundle) line: 79
Activator(Activator).start(BundleContext) line: 70
Activator.start(BundleContext) line: 46
SecureAction.startActivator(BundleActivator, BundleContext) line: 698
Felix.activateBundle(BundleImpl, boolean) line: 2402
Felix.startBundle(BundleImpl, int) line: 2308
Felix.setActiveStartLevel(int, FrameworkListener[]) line: 1539
FrameworkStartLevelImpl.run() line: 308
Thread.run() line: 834
Fixed by @krystofNovotny and released as 6.0.2.