On 5/14/20 7:44 PM, Alan So wrote:

I just notice that if I send an email to a list with a Cc header broken into multiple lines and the pair of double quotes are put in different lines, the email addresses afterwards will be wrongly rewritten.

For example,

CC: "ABC DEF (XYZ)" <name@example.com>

tl;dr It is not valid to fold a header inside of a quotes string.

First of all, the above is invalid. Presumably the address is intended to be

"ABC DEF (XYZ)" <name@example.com>

RFC 5322, sec 2.2.3 says in part:

"The general rule is that wherever this specification allows for folding white space (not simply WSP characters), a CRLF may be inserted before any WSP."

and the definitions in sec 3.4 say in part:

address = mailbox / group

mailbox = name-addr / addr-spec

name-addr = [display-name] angle-addr

angle-addr = [CFWS] "<" addr-spec ">" [CFWS] / obs-angle-addr

display-name = phrase

In this case, the quoted-string "ABC DEF (XYZ)" is a phrase which is the display-name and the only places where comments or folding white space are allowed is immediately preceding the "<" or following the ">" surrounding the addr-spec. Folding in the middle of the quoted string is not allowed.

Were there no quotes, e.g.

ABC DEF (XYZ) <name@example.com>

Then ABC DEF is a phrase and (XYZ) is a comment and folding between DEF and (XYZ) would be allowed

It will become the following after going through the mailing list:

CC: XYZ <"ABC DEF"@mail.server.name>

Which is clearly wrong, but GIGO.

It seems that Outlook often split the To/Cc header into multiple lines and they often add double quotes to the full name of the users. Is the rewriting of Cc header handled by Mailman's code or another Python class?

I haven't looked at the code to see exactly what's responsible here. I do note that adding double quotes around a display name is not only allowed, but is required if the display name contains "specials".

-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

Mark Sapiro wrote:

I haven't looked at the code to see exactly what's responsible here. I do note that adding double quotes around a display name is not only allowed, but is required if the display name contains "specials".

So, it looks like that Outlook still consider the part (XYZ) as comment during folding even though it is within the double quotes. It is folded between DEF and (XYZ) quite often in our cases if there are four or more than four email addresses in the Cc header. Since we normally have full name in the format of "Firstname Lastname (Dept)" in our organization, we hit the issue quite easily if users send to a list with more than four addresses in the Cc header.

It is exactly the file I used to test (listname and list server munged). I can send you the full header personally if you want.

Sent====> (I use "cat file | sendmail listname@listserv.domain" to send)

From: ktso@cuhk.edu.hk To: listname@listserv.domain CC: "ABC DEF (XYZ)" <ktso@cuhk.edu.hk> Subject: testing please ignore Message-ID: <sfhifdgdfdsjHHIHIo987sd@98vD9FVhdv.8shy65FGvsd>

hello

Received====> From: ktso@cuhk.edu.hk To: listname@listserv.domain Subject: testing please ignore Message-ID: <sfh39897897jHHIHIo987sd@98vD9FVhdv.8shy65FGvsd> CC: XYZ <"ABC DEF"@fqdn.domain>

Thanks a lot Mark. Just manage to perform the same test using "mailman inject" and the result is the same as yours. Therefore, it is quite probable that Postfix may rewrite the headers before passing the message to Mailman using LMTP. We will continue to check the issue with our Postfix installation. Thanks again for your help!

On 5/21/20 7:52 PM, Alan So wrote:

Since the CC header remains unchanged if we test using "mailman inject", we may further check whether Mailman LMTP Runner change the CC header.

You are correct. If I post your test via email rather than mailman inject, I see the issue. I'm working on figuring out why. (Sorry for spamming you with test Ccs.)

I'm at a loss to explain why mailman inject doesn't show this. I have captured the queued message and the only differences I see between the one from the lmtp runner and the one from inject is the one from lmtp runner has an X-MailFrom: header and

'received_time': datetime.datetime(2020, 5, 22, 3, 45, 24, 532711),
'to_list': True,

and I even tried running mailman inject with -m to_list=True and it still didn't fail.

However, if I take the qfile that dumps as

> $ bin/mailman qfile 1590119124.5909426+9a002936a59e194f70187405ac624173cbf470b5.pck 
> [----- start pickle -----]
> <----- start object 1 ----->
> Received: by msapiro.net (Postfix, from userid 1000)
> 	id 60624340253; Thu, 21 May 2020 20:45:24 -0700 (PDT)
> From: mark@msapiro.net
> To: list@example.com
> CC: "ABC DEF
>  (XYZ)" <mark@msapiro.net>
> Subject: testing please ignore
> Message-ID: <something_more@example.net>
> Date: Fri, 22 May 2020 09:53:58 +0800 (HKT)
> Message-ID-Hash: NDG3FHLPO35F2Y4YXFAKFIKDXAC6FHGM
> X-Message-ID-Hash: NDG3FHLPO35F2Y4YXFAKFIKDXAC6FHGM
> X-MailFrom: mark@msapiro.net
> 
> hello
> 
> <----- start object 2 ----->
> {   '_parsemsg': False,
>     'listid': 'list.example.com',
>     'original_size': 326,
>     'received_time': datetime.datetime(2020, 5, 22, 3, 45, 24, 532711),
>     'to_list': True,
>     'version': 3}
> [----- end pickle -----]

and put it in the 'in' queue the received message from the list does contain

CC: XYZ <ABCDEF@msapiro.net>

Thus, the issue occurs downstream of lmtp runner. It will take more investigation to determine where.

-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

On 5/22/20 2:32 PM, Mark Sapiro wrote:

For Mailman to defend against this, something like the attached patch would do.

The patch attached to the prior post has a typo, a misplaced colon. The one attached here should be correct

-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

On 5/24/20 7:20 PM, Alan So wrote:

Really thanks for your hard work Mark! It is a bit strange that parseaddr behave differently with CRLF and LF. Maybe if the server is a Windows, the behavior is consistent.

It might be that on Windows, mailman inject would create the message with CRLF line endings.

May I know when or whether will this patch be committed to the repository so that it will be effective in future release of Mailman? Thanks.

In the absence of an issue at <https://gitlab.com/mailman/mailman/-/issues>, probably never. <hint>

The bottom line is the original mail created by Outlook is defective.[1] While Mailman could defend against this defect, starting to do that opens the possibility that there are other areas of the code that need a similar defense.

Have you asked Microsoft to fix Outlook to not create these defective messages? That's the real solution.

That said, if an issue is filed, someone might consider adding something along the lines of <https://lists.mailman3.org/archives/list/mailman-users@mailman3.org/message/Y6DCI4Z2J57K5577MGFXUEUXXQVOIOBE/attachment/2/avoid_duplicates_patch.txt> to the code base.

[1] given an address of the form

display-name (comment) <addr-spec>

in a header, it can be folded either between the display-name and the (comment) or between the (comment) and the <addr-spec>, but if display-name (comment) is quoted as in

"display-name (comment)" <addr-spec>

"display-name (comment)" is now a phrase within which folding is not allowed and this can only be folded between the "display-name (comment)" and the <addr-spec>.

-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

On 5/25/20 12:12 AM, Alan So wrote:

Mark Sapiro wrote:

...

In the absence of an issue at https://gitlab.com/mailman/mailman/-/issues, probably never. <hint>

Is that anyone with a gitlab account can file an issue? If it is appropriate, I can try to file one.

Yes, anyone with a gitlab account can file an issue.

...

Have you asked Microsoft to fix Outlook to not create these defective messages? That's the real solution.

Yes and I agree. My colleague has actually done so but it is probably not yet escalated to the appropriate tier of support. Here is the comment of my colleague: "I can submit this problem to Microsoft. However, it may take a long time to fix it or not even important enough to be fixed."

At least you're trying and that's a good thing even if they don't fix it.

-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

On 5/26/20 12:33 AM, Alan So wrote:

Thanks. Just filed an issue: https://gitlab.com/mailman/mailman/-/issues/725

Thank you for your "well formed" issue. I have started to work on this, but I have run into a snag.

The patch at <https://lists.mailman3.org/archives/list/mailman-users@mailman3.org/message/Y6DCI4Z2J57K5577MGFXUEUXXQVOIOBE/attachment/2/avoid_duplicates_patch.txt> doesn't work. The problem is msg.get_all(header, []) does not parse the header and return a list of addresses. It just returns a list, the items of which are the raw values if the header instances in msg. I.e., if there is only one Cc: header, msg.get_all('cc', []) returns a list of one string which is the raw value of that header. If there were two Cc: headers in the message it would return a list of two strings with the two raw values.

Thus, we would need to parse the strings returned into the individual email addresses. The simple minded way to do that is

addrs = []
for hdr in msg.get_all(header, []):
    for addr in hdr.split(','):
        addrs.append(parseaddr(re.sub('[\r\n]', '', addr)))

but that will fail if the header contains a quoted display name containing a comma such as

"last, first" <user@example.com>

So, I need to go back to using getaddresses, but do the unfolding first. Watch the issue for the final fix.

-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

On 5/26/20 9:10 PM, Alan So wrote:

Thanks again for the work and just notice that the issue has been closed already. Looks like unfolding the header line is a logical way but it seems that >=3.3 email package unfold the header before parsing the header according to the following python issue:

https://bugs.python.org/issue11050

Yes, the issue is closed because the fix has been merged and will be in Mailman Core 3.3.2.

I agree that it appears that there would have been no issue if Mailman created messages using the SMTP policy and retrieved the addresses using the addresses attribute of the headers.

However, for historical reasons, Mailman Core still uses the compat32 policy (Python 3.2 compatibility) and uses email.utils.getaddresses() to get the addresses from the header value, so the "fix" for <https://bugs.python.org/issue11050> doesn't apply.

Changing Mailman Core to use email.EmailMessage() objects rather than the legacy email.Message() objects is a worthwhile goal, but would require extensive changes to fix all the things that broke as a result.

Anyway, I really appreciate your diligence in following through repeatedly on this issue. Were it not for that, I wouldn't have been motivated to fix it. For a look at why even a trivial change is not trivial, see the commits at <https://gitlab.com/mailman/mailman/-/merge_requests/652/commits>. The first of these is developing good tests for the issue and then fixing the code so the tests pass, and the second, unanticipated one was due to the fact that Python 3.5 didn't maintain the original ordering of the addresses in Cc: so I couldn't compare against a fixed result. This is due to the fact that the underlying code uses dictionaries, and prior to Python 3.6 dictionaries didn't preserve order.

-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

On 5/27/20 2:35 AM, Alan So wrote:

I tried to read RFC5322 again but I am not sure whether I interpret it correctly. The display-name seem allowing folding in the middle.

3.2.4 quoted-string = [CFWS] DQUOTE *([FWS] qcontent) [FWS] DQUOTE [CFWS]

A quoted-string is treated as a unit. That is, quoted-string is identical to atom, semantically. Since a quoted-string is allowed to contain FWS, folding is permitted. ...

You are correct. I missed that, so the messages created by Outlook are not defective.

-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan