Repository navigation
Inconsistency between quopri.decodestring(), email.quoprimime.decode() and binascii.a2b_qp() #62222
Description
Activity
>>> import quopri, email.quoprimime >>> quopri.decodestring(b'==41') b'=41' >>> email.quoprimime.decode('==41') '=A'
I don't see a rule about double '=' in RFC 1521-1522 or RFCs 2045-2047 and I think quopri is wrong.
Other half of this bug (encoding '=' as '==') was fixed in 9bc52706d283.
There are other inconsistencies. email.quoprimime.decode(), binascii.a2b_qp() and pure Python (by default binascii used) quopri.decodestring() returns different results for following data:
quoprimime binascii quopri b'=' '' b'' b'=' b'==' '=' b'=' b'==' b'= ' '' b'= ' b'= ' b'= \n' '' b'= \n' b'' b'=\r' '' b'' b'=\r' b'==41' '=A' b'=41' b'=A'Most of the variations represent different invalid-input recovery choices. I believe binascii's decoding of b'= \n' is incorrect, as is its decoding of b'==41'. quopri's decoding of b'=\r' is arguably incorrect as well, given that python generally supports universal line ends. Otherwise the decodings are all responses to erroneous input for which the behavior is not specified.
That said, we ought to pick one error recovery scheme and implement it in all places, and IMO it shouldn't be exactly any of the ones we've got. Or better yet, use one common implementation. Untangling quopri is on my (too large) List of Things To Do :)
Perl's MIME::QuotedPrint produces same result as pure Python quopri. konwert qp-8bit produces same result as binascii (except '==41' it decodes as '=A').
RFC 2045 says:
"""A
reasonable approach by a robust implementation might be
to include the "=" character and the following
character in the decoded data without any
transformation and, if possible, indicate to the user
that proper decoding was not possible at this point in
the data.
"""So what scheme we will picked?
As I said, not exactly any of the above.
I'll get back to this after I finish the new email code (which should happen before the end of the month). I need to take some time to look over the RFCs and real world examples and come up with the most appropriate rules.
Ping.
The double equals "==" case for the “quopri” implementation in Python is now consistent with the others thanks to the fix in bpo-23681 (see also bpo-21511).
According to bpo-20121, the quopri (Python) implementation only supports LF (\n) characters as line breaks, and the binascii (C) implementation also supports CRLF. So I agree that the whitespace-before-newline case "= \n" is a genuine bug (see bpo-16473). But the CR case "=\r" is not supported because neither quopri nor binascii support universal newlines or CR line breaks on their own.
Thus currently the table of discrepancies looks as:
quoprimime binascii quopri b'=' '' b'' b'=' b'= ' '' b'= ' b'= ' b'= \n' '' b'= \n' b'' b'=\r' '' b'' b'=\r' b'==41' '=A' b'=41' b'=41'OK, I've finally gotten around to looking at this. It looks like quopri and binascii are not stripping trailing whitespace.
quoprimime binascii quopri preferred b'=' '' b'' b'=' '=' b'= ' '' b'= ' b'= ' '=' b'= \n' '' b'= \n' b'' quoprimime binascii quopri b'=' '' b'' b'=' b'= ' '' b'= ' b'= ' b'= \n' '' b'= \n' b'' b'=\r' '' b'' b'=\r' b'==41' '=A' b'=41' b'=41' '=\n' b'=\r' '' b'' b'=\r' '=\r' b'==41' '=A' b'=41' b'=41' '=A' b'= \n f\n' ' f\n' b'= \n f\n' b'= \n f\n' ' f\n'The RFC recommends that a trailing = be preserved, but that trailing whitespace be ignored. It doesn't speak directly to the ==41 case, but one can infer that the first = in the == pair is most likely to have "come from the source text" and not been encoded, while the =41 was an intentional encoding and so should be decoded.
Now, that said, the actual behavior that our libraries have had for a long time is to treat the "last line" just like all other lines, and strip a trailing =. So I would be inclined to keep that behavior for backward compatibility reasons rather than change it to be more RFC compliant, given that we don't have any actual bug report related to it, and "fixing" it could break things. Given that, the current quoprimime behavior becomes the reference.
However, backward compatibility concerns also arise around starting to strip trailing space in quopri and binascii. Maybe we only make that change in 3.8?
Many thanks David! But sorry, your table confused me. I can't read it. Could you please reformat it?
I should have just deleted the table, actually.
The only important info in it is that per RFC '=', '=\n', and '= \n' all ought to become '='. But I don't think we should make that change, I think we should continue to turn those into ''. So I consider the (current!) bwehavior of quoprimime to be the correct behavior.
I also gave the example of '= \n foo\n', to show that quopri and binascii aren't stripping trailing blanks, as Martin noted in the other issue. They fold lines if they see '=\n', but not if they see '= \n', which is wrong per the (email!) RFC. I'm not clear if it is wrong for non-email uses of quopric, I haven't tried to research that.
Some real-world scenario where our test assumptions were wrong and this issue was (once again) found: https://github.com/python/cpython/actions/runs/12200589926/job/34037087571#step:22:214.
IMO, whatever we choose, I'd like
dec(enc(x)) == x. I don't know however how it could be disruptive to the email part =/- changed the title
[-]Inconsistency between quopri.decodestring() and email.quoprimime.decode()[/-][+]Inconsistency between quopri.decodestring(), email.quoprimime.decode() and `binascii.a2b_qp()`[/+]on Dec 6, 2024 Some current data on how the three functions, and a few external implementations, decode the ambiguous cases.
CPython today
quopri.decodestring()callsbinascii.a2b_qpwhenbinasciican be imported, and otherwise uses its own pure-Python loop inLib/quopri.py. The two differ: the pure-Python loop strips trailing whitespace on\n-terminated lines, thebinasciiC path does not. In the table, "binascii" is therefore also whatquopri.decodestring()returns by default; "quopri(pure)" is thebinascii-unavailable fallback.quoprimime binascii(=default quopri) quopri(pure) b'=' '' b'' b'=' b'==' '=' b'=' b'=' b'= ' '' b'= ' b'= ' b'= \n' '' b'= \n' b'' b'=\r' '' b'' b'=\r' b'==41' '=A' b'=41' b'=41' b'foo \n' 'foo\n' b'foo \n' b'foo\n'(The pure path strips only on
\n-terminated lines, sob'= 'at EOF staysb'= '.)Malformed
=recovery (e.g.==41)RFC 2045 §6.7 calls such a sequence "illegal" and gives only a non-normative suggestion: "A reasonable approach by a robust implementation might be to include the
=character and the following character in the decoded data without any transformation." Observed results for==41:behavior ==41→implementations emit =, advance 1, rescan=Aemail.quoprimime; Thunderbird; PerlMIME::QuotedPrint3.16; Gomime/quotedprintable; Nodelibqp; konwertemit =, advance 2=41binascii/quopri(the second=is dropped)drop the =, advance 1, rescanAGmail (web) reject as an error — PHP quoted_printable_decode(); Apache commons-codecVerification: Perl run locally; Go/Node/PHP/commons-codec read from source; Gmail measured by APPENDing a hand-built QP message and reading the downloaded
.emlplus the rendered body. (Gmail's "Show original" view is normalized — it silently rewrites malformed QP — so only the downloaded message reflects the stored bytes.)Trailing whitespace before a line end
RFC 2045 §6.7, Rule #3: "when decoding a Quoted-Printable body, any trailing white space on a line must be deleted, as it will necessarily have been added by intermediate transport agents."
- Strips it:
email.quoprimime;quopripure-Python path (on\n-terminated lines). - Keeps it:
binascii.a2b_qp(hence defaultquopri); Gmail (measured); Thunderbird (measured; itsmimeenc.cpphas a comment noting the non-compliance).
Interior whitespace and encoded trailing whitespace (
=20/=09) are handled identically by all ofquoprimime/binascii/quopri.- Strips it:
I think we can resolve this in steps, aiming for a single quoted-printable decoder instead of three slightly different ones.
-
Fix
b'==41'first. Herebinascii/quopriare the outliers: they advance by two and silently drop the second=, giving=41, whilequoprimimeand everything else surveyed (Thunderbird, Perl, Go, Node, …) emit the stray=and re-scan, giving=A. I'd changebinascii.a2b_qpto match. This likely resolves some of the other rows too, so it comes first. -
Then trailing whitespace. Either delete it unconditionally (per RFC 2045 §6.7, as
quoprimimedoes), or add a flag tobinascii/quoprito opt into stripping — keeping the non-stripping behavior available for compatibility with Thunderbird and Gmail, which also keep it. -
Then line ends. Decide whether end of input or a bare
\rcounts as a line end for the soft-break and trailing-whitespace rules (theb'=\r'and final-partial-line cases). -
Finally, consolidate. With
binascii/quopricorrect,quoprimimecan defer to them. The pure-Python fallback inquoprican likely go too, sincebinasciiis already required unconditionally (base64depends on it). That leaves one implementation.
I'll start with step 1.
-
One caveat on
email.quoprimime.decode, which we have been comparing against: the email module does not actually use it for body decoding.get_payload(decode=True)decodes viaquopri, not viaquoprimime, andquopridoes not strip trailing whitespace — so the body path is non-compliant with RFC 2045, even though the package ships a compliant decoder (quoprimime.decode) that it bypasses.This also looks unintentional. The body path used to strip: it went through
quopri.decodestring, which was pure Python and stripped trailing whitespace per line. Patch #462190 (16dc7f4, 2001) addeda2b_qptobinasciiand madequopridelegate to it for speed; the C decoder never replicated the stripping, so the body path silently became non-compliant and has stayed that way.
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs