Unify all smtplib commands to be uppercase

Ideas
1

Hello,

I know there was a previous issue made years ago about smtplib being inconsistent with uppercase and lowercase commands, however the succeeding fix made the commands lowercase. I think these should be switched to uppercase and here’s why:

RFC 5321 states “A few SMTP servers, in violation of this specification (and RFC 821) require that command verbs be encoded by clients in upper case. Implementations MAY wish to employ this encoding to accommodate those servers.”

Servers such as Postfix and many others have resolved to simply making all SMTP commands and options uppercase as depicted (though not required) in all the RFC documents. Though it is not strictly necessary, it will have better support (even if marginally) and is a quick fix.

Also, smtplib seems to already send some of these commands and options like STARTTLS and SMTPUTF8 uppercase which technically leaves the inconsistency of the last issue unsolved.

Ultimately, I think it would be best for posterity to simply make all of these uppercase and it should be a quick fix.

I can make the proper edits and make a pull request if there isn’t some obvious reason for this I’m missing.

Let me know your thoughts! :slightly_smiling_face:

2 Likes
2

There are obvious reasons against (wasting of resources, fixing something not broken), but I’m afraid that taking away the pressure on owners of such non-conforming servers is the opposite of what is needed to lower the scale of this problem.

1 Like
3

Hey Xitop, thanks so much for the reply! I see where you’re coming from in terms of not fixing something that’s seems to not be broken. However, it is widely seen as best practice to use uppercase commands in SMTP transactions. I see this as hardening smtplib’s deliverability rather than accompanying non-compliant servers. You’re definitely right that those servers should change, but any of those servers that have that issue are unlikely to change since 90% of MTAs like Postfix and EXIM already use the uppercase version of commands to maximize deliverability. Why should smtplib suffer for a simple change from lowercase to uppercase.

Also it seems the points regarding inconsistency of capitalization in this issue were deemed reason enough for a PR, and that issue still persists. I presume many of the developers that added options like STARTTLS are using the same case as shown in the RFCs which is reasonable, so it makes sense to change the few instances of commands like ‘ehlo’ to ‘EHLO’ to consolidate. There were also other users complaining of this issue, so I figured why not correct this.

Would it help if I made a PR to show the proposed changes?

4

I think the most straightforward approach is to subclass smtplib.SMTP.

5

Subclassing is far from a straightforward fix. You would have to nitpick every smtp command function within the SMTP class. It really is truly quite easy to uppercase the commands in the smtplib source. Since the smtplib source is only ~1100 lines and there are just a few instances to uppercase, I actually simply placed a new file in the local project dir to override the standard module and works like a charm. I’ve already written the fix, just wanted to provide it to the main branch since I know it would help people and there are others out there annoyed by this. What is the advantage of keeping this inconsistent command style? Every MTA uses uppercase, so it seems quite obvious that should be the choice.

1 Like
6

+1 for this, and regard this as a fix to smtplib.

3 Likes
7

Appreciate the support for the fix. Seems like the best way to go and would help many developers. I made an issue on the github but they said to bring it here first to see some thoughts before continuing.

1 Like
8 
class UppercaseSMTP(smtplib.SMTP):
    def putcmd(self, cmd, args=""):
        return super().putcmd(cmd.upper(), args)
Test 
import unittest
from unittest.mock import patch
import smtplib

class UppercaseSMTP(smtplib.SMTP):
    def putcmd(self, cmd, args=""):
        return super().putcmd(cmd.upper(), args)

class TestUppercaseSMTP(unittest.TestCase):
    @patch('smtplib.SMTP.putcmd')
    def test_putcmd_uppercase(self, mock_putcmd):
        smtp = UppercaseSMTP()
        smtp.putcmd("ehlo", "example.com")

        mock_putcmd.assert_called_once_with("EHLO", "example.com")

if __name__ == '__main__':
    unittest.main()
9

Yes I thought about that, but if you check further you’ll notice this still doesn’t address things like MAIL FROM and RCPT TO since the ‘from’ and ‘to’ are technically not part of the command but as part of the option. Also things like size should be ‘SIZE=’ at the end of the MAIL command instead of ‘size=’.

It’s obvious that there lies some home brew solution for this, but why not just correct the actual smtplib? These mixed instances of uppercase and lowercase weren’t intentional and should clearly be fixed.

1 Like
10

That seems the way to go then.

The last “fix” from uppercase to lowercase did nothing but break my test cases. That new fix will break my test cases again. So, I would consider this library unstable and avoid using it in the future because of another “fix”.

11

That’s certainly odd. Why would changing the case of the commands break your test case when the RFC clearly states compliant implementations should be case insensitive? On another note, the original issue was actually requesting the commands to be uppercase as ‘luiji’ and ‘Razza30’ were clearly mentioning in that thread, yet the devs made them lowercase (I think that was a mistake).

Again, I want to reiterate, there are obvious ways to fix this locally, but I think this is a simple enough and worthy fix to apply to the main branch, so there is no need to constantly correct this for everyone who’s trying to correct this issue.

2 Likes
12

It’s not odd because compliant implementations should be case insensitive. See the example test using an uppercase command. The original test checks using a lowercase command. I’m testing what the method returns.

However, I noticed you didn’t mention any backward compatibility issues or address them, as requested in issue.

This brings us back to the second post:

13

In terms of backwards compatibility, this is simply uppercasing the commands and like you’ve mentioned, this shouldn’t break any compliant implementations since RFC states case insensitivity but rather improve support for more SMTP servers. Servers requiring uppercase commands is a known issue but the opposite is not true, so there isn’t any advantage for using lowercase.

As for the test case, yes that definitely will not succeed the assertion if you make it test for lowercase commands but isn’t that inevitable if you set it up like that?

Also, it seems maybe my tone was misconstrued earlier. I simply saw that there was a clear advantage here to use uppercase commands and no advantage in using lowercase and am only trying to help improve things for smtplib users. I definitely want to hear thoughts on backwards compatibility and potential breaking changes which is why I opened the discussion. Other than assertion tests, I can’t see any reason why sending out EHLO instead of ehlo (for example) would be anything other than advantageous for users.

1 Like
14

The same goal can be achieved by fixing those noncompliant servers.
This would solve the problem for all SMTP libraries, not just Python’s, and would also benefit smtplib versions that are already deployed.

15

What are these noncompliant servers?

I am sympathetic to this proposal, but we need to know what we will achieve.

3 Likes
16

yes -sometimes the role here is trying to vet all ideas, briging up all the downides of any changes (And I myself will do it) .

However, in this case, this jsut brought up the answers in this thread, and it is so little controverse, that apparently there is no support: to the contrary, it just means it won’t be a matter for most people.

The arguments brought for the downside are empty, since we are standardizing the protocol usage - there could be some eventual backwards incompatibility, if one had used Python’s smtplib to connect to an incorrect implementation of the protocol (which would reject upper case commands) or maybe break some unittests for someone who compare issued SMTP commands to hardcoded strings in the test code : both are cases that should not, of course, hinder Python evolution towards strict adderence to standards or conventions.

1 Like
17

It’s difficult to point out exactly which servers would have this issue since most comply with the RFC. However, we know RFC 5321 states “A few SMTP servers, in violation of this specification (and RFC 821) require that command verbs be encoded by clients in upper case.” This is why modern MTAs have standardized sending out commands in upper case as best practice, and it would only benefit python to adhere smtplib to that standard to better cover edge case scenarios.

1 Like
18

Yes, exactly. This is simply about bring python to the already long standing industry standard. Although it might seem negligible, it is definitely more advantageous to send the commands in uppercase (which would cover those legacy systems) than to send them in lowercase (which adds no extra support).

3 Likes
19

So, to sum up:

  1. It seems there is no actual issue.
  2. The standard in question predates the Web itself. Supporting outdated implementations is unnecessary today, especially given the lack of evidence for their continued use.
  3. Since smtplib has used all-lowercase commands for over 25 years, it may have led to implementations that violate the specification.

So, the best approach is to leave it as it is, rather than changing decades-old implementation conventions. Note that using all lowercase or all uppercase is merely an implementation detail or a convention within the implementation.

This convention allows me to create test cases without having to implement the entire specification within the tests themselves.

Python’s simplicity and readability allow developers who may not be highly technical to do their work without necessarily knowing what an RFC is or even that such standards exist. That’s why even non-essential changes can have unintended and often unnecessary side effects. That falls under backward incompatibility.

20
  1. There has been a clearly outlined benefit to this, so to say there is “no issue” is an intentional understatement.
  2. Widening support by following a widely-held convention is always a step in the right direction. Just because the use-case may not be noticeable to you does not mean it does not exist.
  3. Smtplib was not even consistently lowercase just 2 years ago if you actually check the change history. For example, MAIL FROM was sent as mail FROM with mixed cases. It was a step in the right direction to unify the case, but it should be uppercase and not lowercase.

Creating test cases that are case sensitive is shortsighted and will obviously break. But that is no reason to hinder python from adhering to the industry standard and force it to pointlessly send lowercase while it makes much more sense to unify as uppercase. Furthermore, it seems your intentions and comments are more combative than they are constructive. So I ask again, aside from arbitrary test cases, what real backwards compatibility issue would there be?

3 Likes