[issue7143] get_payload(decode=True) eats last newline in base64 encoded payload
Joaquin Cuenca Abela e98cu...@gmail.com added the comment: Thanks David and Barry! As much as it flatters my ego to be in the first place is MISC/ACK, I have to confess that my family name is Cuenca Abela, Cuenca is not a middle name. Cheers, -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue7143 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7143] get_payload(decode=True) eats last newline in base64 encoded payload
R. David Murray rdmur...@bitdance.com added the comment: Your patch looks good, thank you. I just realized that Barry isn't nosy on this issue. I've checked, and the code in question dates back to email version 1.0...code of that long standing that exists specifically to implement the behavior we propose to change makes me wonder what we will break if we fix it. Yet the current behavior seems clearly contrary to the RFCs to me. Barry? Any memory of why _bdecode exists and what will break if we fix it? In any case I suspect that backporting this fix might be a bad idea, since existing code may be compensating for the current behavior and break if the behavior is fixed. -- nosy: +barry versions: -Python 2.5, Python 2.6, Python 3.1 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue7143 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7143] get_payload(decode=True) eats last newline in base64 encoded payload
Joaquin Cuenca Abela e98cu...@gmail.com added the comment: Unfortunately the only way that I can see to reliably work around this is to bypass entirely get_payload, in this case fixing this bug will not affect people that do that negatively. Some people may have more control over their attachments and they are unconditionally adding a \n to certain type of attachments. In this case they will get an extra \n after this patch. If they are doing this, they certainly realized there was a bug, because this is only necessary for base64 encoded attachments and not for any other encoding. In this case I guess it's reasonable to expect that this bug will be fixed one day. But you're right that the change is risky, an that we (at least I) are not sure why the code is the way it is. Cheers, -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue7143 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7143] get_payload(decode=True) eats last newline in base64 encoded payload
Barry A. Warsaw ba...@python.org added the comment: Thanks for adding me to the nosy list. Yep, this code is pretty old so it doesn't surprise me that its implementation isn't quite right. Of course, I hate get_payload(decode=True) anyway and hope that goes away in email 6. Having said that, I don't think this can be changed in Python 2.6. It's pretty common for people to have workarounds for the devil they know and I'd bet that changing this behavior in 2.6.x would break people's code. I'm okay for doing the sensible thing in Python 2.7 (though we should probably bump the email package's micro version). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue7143 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7143] get_payload(decode=True) eats last newline in base64 encoded payload
R. David Murray rdmur...@bitdance.com added the comment: OK, patch (with comment tweaks to refer to this issue), and email minor version bump, applied to trunk in r78778. It turns out that bdecode was already deleted in email 5 in py3k. I did port the test in r78780. Thanks Joaquin Cuenca Abela, and you now have the interesting distinction of being the first name listed in Misc/ACKS (because we've been maintaining it in sorted order by last name) -- resolution: - fixed stage: patch review - committed/rejected status: open - closed versions: -Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue7143 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7143] get_payload(decode=True) eats last newline in base64 encoded payload
Joaquin Cuenca Abela e98cu...@gmail.com added the comment:
Hi,
I've never before made a patch to Python, so take it with care.
A couple of comments, I reused a test where all the attachments contained an
ending newline, except for the base64 one (conveniently...)
I think the comment in _bdecode that Andreas quoted before refers to
base64.encodestring add an extra \n to de encoded string, but
base64.decodestring can work with and without this extra \n:
import base64
base64.decodestring('MTIzCg==')
'123\n'
base64.decodestring('MTIzCg==\n')
'123\n'
So it's not necessary to work around this in _bdecode. Let me know if it looks
good to you.
--
keywords: +patch
Added file: http://bugs.python.org/file16433/b64crlf.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue7143
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7143] get_payload(decode=True) eats last newline in base64 encoded payload
R. David Murray rdmur...@bitdance.com added the comment: Thanks for working on this. For the most part your patch looks fine. Two comments: (1) it concerns me that by co-opting the existing test, we are no longer testing that decoding does not introduce a spurious newline :). (2) I think we should add a comment in _bdecode that it used to do more and is retained now only for backward compatibility. I don't particularly like proliferating test files, and I plan to fix that in email6, but for now I think you should just add a new test file based on msg_10, and a test that uses it. -- stage: test needed - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue7143 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7143] get_payload(decode=True) eats last newline in base64 encoded payload
Joaquin Cuenca Abela e98cu...@gmail.com added the comment: I added a new subpart to msg_10.txt, that keeps the previous test and also tests the new behavior. Let me know if it's ok like this or if you still prefer to create a different msg file for testing this. Thanks, -- versions: +Python 2.5 Added file: http://bugs.python.org/file16438/b64crlf-2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue7143 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7143] get_payload(decode=True) eats last newline in base64 encoded payload
R. David Murray rdmur...@bitdance.com added the comment: Ah, I misunderstood, and did not see that the newline in question was inside the base64 string. Thank you for pointing out my mistake. Would either of you like to propose a patch, including a test case? (I've removed 2.5 because it is in security-fix-only mode). -- assignee: - r.david.murray keywords: +easy resolution: invalid - stage: committed/rejected - test needed status: closed - open title: get_payload(decode=True) eats last newline - get_payload(decode=True) eats last newline in base64 encoded payload versions: -Python 2.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue7143 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7143] get_payload(decode=True) eats last newline in base64 encoded payload
Changes by R. David Murray rdmur...@bitdance.com: -- versions: -Python 3.0 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue7143 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
