New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
smtplib: verify breaks with Postfix servers #51733
Comments
Hi, The verify method of SMTP objects created with smtplib doesn't work The attached demo file uses the mx1.nic.fr server, which runs Postfix RFC 5321 says the argument of VRFY is a string representing a "user Anyway, even if Postfix was wrong, I think it would be worth trying to Thanks! |
I agree. My reading of the rfc is that the form without the brackets 2.5 is in security-fix-only mode, so this can only be fixed in 2.6 and |
This patch solves the problem with the VRFY command, but I'm still |
I've rewrote those patches to 'default' and 2.7 |
Thanks for working on this. The tests seem to be missing, as is the line that adds 'clean' to the def, so the patches won't work as is. However, now that I've looked at the patch in more detail, adding a parameter to a public method is not something we can do in a bug fix release. So, this solution would work for 3.3, but not for 2.7 and 3.2. In any case, Guido thinks that parameters that have only two values should be replaced by methods with two different names. In this case that makes a lot of sense. I've checked the RFC and the code, and there are two cases: MAIL FROM and RCPT TO, which require the address to be in <>s, and VRFY and EXPN, which prefer that it not be in <>s. So I think we should introduce a new, private function for use in the VRFY and EXPN cases: def _addronly(addr):
(fullname, email) = email.utils.parseaddr(addr)
return email Can you do a new patch, adding the above function and using it at the right places? Tests are also needed...it should be possible to modify the test that the original patch modified so that it checks to make sure the <> are not added. If you need help with that let me know. |
David.. I extracted quoteaddr code to _addrformat and now quoteaddr and _addronly call _addrformat passing a format (<%s> or %s). I've also created quoteaddr and _addronly test functions as well modified VRFY and EXPN tests to make sure they call _addronly and pointed brackets aren't added. Let me know if those patches still need improvements. |
Can anyone take a loot at those patches? Do they need more tests? |
I looked at the Felipe's patch and hopefully made some improvements. Unlike Felipe's patch I didn't change the reply of the SMTP server in the tests but instead use what VRFY and EXPN actually send to index the users and lists dictionaries. If <> would be sent the lookup would fail. Similarly, when VRFY return 550 it echoed the address as received and now it's tested to be equal to something without <>. By the way, but I was wondering:
|
New changeset c4d884d5d86c by R David Murray in branch '2.7': New changeset f8c4ac9aa9e2 by R David Murray in branch '3.2': New changeset 0d9216de8f05 by R David Murray in branch 'default': |
New changeset 50b6c3053c30 by R David Murray in branch 'default': |
Thank you both for your work on this. The patch I committed is a combination of my _addr_only, Filipe's tests, and Catalin's modifications to those tests. quoteaddr, although in the __all__, is not documented and is really an implementation detail, as is the new _addr_only. So I am only testing them indirectly through the documented parts of the API (I added a test for <> address, and one for an IDNA encoded address). Catalin, I think you are correct about the try/except/None stuff. As far as I can tell it is left over from the old days before the email package and its philosophy of never throwing parsing errors. Nowadays if parseaddr throws an error, it is a bug. That's a refactoring not a bug fix, though, so I didn't backport it. |
You're very kind David. Hope I can contribute with something more relevant next time :) best regards, 2011/7/18 R. David Murray <report@bugs.python.org>
|
Don't short change yourself. This bug would still be open if it hadn't been for your work, regardless of how much of it wound up in the final patch :) |
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:
The text was updated successfully, but these errors were encountered: