Milan Oberkirch added the comment: Thanks a lot for your feedback!
> Your modifications to the handling of max_command_size are technically buggy > (and clearly there's no test for it): when we aren't in extended smtp mode, > the limit should be the hardcoded value, as in the original code. My reading > of the RFC (specifically 4.1.1.5) indicates that it is legal to send a new > HELO or EHLO command without issuing a QUIT, meaning a single SMTPChannel > instance might have to handle both. This isn't a likely scenario except in > testing...but testing is probably one of the main uses of smtpd, so I think > we ought to support it. I agree that Section 4.1.1 should be interpreted that way and that should probably get fixed. The old implementation is very strict about EHLO/HELO being allowed exactly once. And there are tests to verify this bug exists. We could allow it for SMTPUTF8 since we do not need to care about bug compatibility if someone uses enable_SMTPUTF8 and we document the change. My intention was to leave the command_size_limits untouched on HELO so it will return 512 on every request (as max_command_size does then). And the existing implementation guaranteed me there is only one of HELO or EHLO. > I think it would be clearer to use an unconditional set of the value based on > the command_size_limit constant, which you'll have to restore to address my > previous comment, instead of the pop (though the pop is a nice trick :). I agree on replacing the .pop() with command_size_limits['MAIL'] = command_size_limit. I see no easier/shorter way to add up the values right now however. > There is one piece missing here: providing a way for process_message to know > that the client claimed the message was using SMTPUTF8 (we know it might lie, > but we need to transmit what the client said). Since process_message is an > 'override in subclass' API, we can't just extend it's call signature. So I > think we need to provide a new process_smtputf8_message method that will be > called for such messages. Thanks for pointing that out, I totally missed that! > Note that you also need to reset the 'this message is SMTPUTF8 flag' on RSET, > HELO/EHLO, and at the completion of the call to the new process_message API. Agreed. I already implemented the things that are clear but am not sure how to handle the first point about multiple HELO/EHLO. - Milan ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue21725> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
