[ 
https://issues.apache.org/jira/browse/GERONIMO-6908?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Richard Zowalla closed GERONIMO-6908.
-------------------------------------
    Resolution: Fixed

Fixed on main in commit 7b37292, shipping in Mail_2.1_1.1.1.

A loading flag, set for the duration of InternetHeaders.load(InputStream, 
boolean), suppresses the single-value check while headers are parsed. The 
policy still applies to headers added programmatically, so the intent of 
GERONIMO-6870 is preserved.

Note that Mail_2.1_1.1.0 was released before this landed, so that release 
cannot read ordinary inbound mail carrying repeated headers.

Covered by new tests in InternetHeadersTest, including one asserting that 
loading does not leave the policy switched off for headers added afterwards. 
Jakarta Mail 2.1 TCK: 321/321.

The outbound half of the over-broad rule is GERONIMO-6909, fixed in the same 
commit.

> InternetHeaders rejects legally repeated headers while parsing, so real-world 
> messages cannot be read
> -----------------------------------------------------------------------------------------------------
>
>                 Key: GERONIMO-6908
>                 URL: https://issues.apache.org/jira/browse/GERONIMO-6908
>             Project: Geronimo
>          Issue Type: Bug
>      Security Level: public(Regular issues) 
>          Components: mail
>            Reporter: Richard Zowalla
>            Priority: Critical
>
> {{InternetHeaders}} rejects any header that appears more than once, except 
> {{Received}} and {{Return\-Path}}. The check is in the anonymous {{List}} in 
> {{InternetHeaders}} and was introduced with GERONIMO\-6870 to stop us from 
> _emitting_ messages with duplicated singleton headers.
> The problem is that the same check also runs on the _inbound_ parse path, via 
> {{InternetHeaders.load\(\)}} \-> {{addHeaderLine\(\)}}. Repeated headers are 
> legal and extremely common on real mail, so ordinary messages can no longer 
> be read at all.
> h2. Reproduction
> Any Gmail\-delivered message works. The attachment on GERONIMO\-6656 
> \(ReceivedMail.txt\) is one:
> {code:java}
> Session s = Session.getInstance\(new Properties\(\)\);
> try \(InputStream in = new FileInputStream\("ReceivedMail.txt"\)\) {
>     MimeMessage m = new MimeMessage\(s, in\);   // throws
> }
> {code}
> {noformat}
> java.lang.IllegalStateException: InternetHeaders cannot contain more than one 
> value for header: authentication\-results
>       at 
> jakarta.mail.internet.InternetHeaders$1.assertNoDuplicates\(InternetHeaders.java:59\)
>       at 
> jakarta.mail.internet.InternetHeaders$1.add\(InternetHeaders.java:51\)
>       at 
> jakarta.mail.internet.InternetHeaders.addHeaderLine\(InternetHeaders.java:607\)
>       at 
> jakarta.mail.internet.InternetHeaders.load\(InternetHeaders.java:178\)
>       at jakarta.mail.internet.InternetHeaders.\(InternetHeaders.java:125\)
>       at 
> jakarta.mail.internet.MimeMessage.createInternetHeaders\(MimeMessage.java:1644\)
>       at jakarta.mail.internet.MimeMessage.parse\(MimeMessage.java:287\)
>       at jakarta.mail.internet.MimeMessage.\(MimeMessage.java:171\)
> {noformat}
> h2. Why this is wrong
> Headers that legitimately repeat and are not whitelisted include 
> {{Authentication\-Results}} \(RFC 8601\), {{ARC\-Seal}} / 
> {{ARC\-Message\-Signature}} / {{ARC\-Authentication\-Results}} \(RFC 8617\), 
> {{DKIM\-Signature}} \(RFC 6376\), and {{Comments}}, {{Keywords}}, 
> {{Resent\-_}} and {{List\-_}} \(RFC 5322 / RFC 2369\). 
> {{InternetHeaders.getHeader\(String\)}} returns a {{String\[\]}} precisely 
> because a header can occur more than once, and Angus/the reference 
> implementation appends without complaint.
> A parser must accept what the wire delivers. Throwing an unchecked 
> {{IllegalStateException}} out of a {{MimeMessage}} constructor also means 
> callers cannot recover: the exception is not a {{MessagingException}} and is 
> not declared.
> h2. Proposed fix
> Keep the single\-value policy for headers added programmatically, so the 
> intent of GERONIMO\-6870 is preserved, but never apply it while parsing a 
> stream. A {{loading}} flag set for the duration of {{load\(InputStream, 
> boolean\)}} and honoured by {{assertNoDuplicates}} is enough.
> h2. Follow\-up worth discussing separately
> The outbound check currently rejects _every_ repeated header name rather than 
> the RFC 5322 section 3.6 singleton set, so {{addHeader}} also refuses 
> legitimate cases such as two {{X\-}} headers or two {{Comments}} headers. 
> Narrowing the policy to the actual singleton fields would be a behaviour 
> change and is deliberately not part of this issue.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to