Skip to content
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

Switch to SASL subprotocol on AUTH command #277

Merged
merged 3 commits into from
Sep 12, 2021

Conversation

seriyps
Copy link
Collaborator

@seriyps seriyps commented Sep 12, 2021

I noticed that Python's SMTPLIB sends all verbs in lowercase and gen_smtp fails to recognize rset\r\n command.

UPD: while digging deeper, I found https://datatracker.ietf.org/doc/html/rfc4954 and it says that AUTH xxxx verb actually makes client/server switch to a SASL subprotocol which uses base64-encoded data exchange. So, it does not make sense to use the same handle_request/parse_request functions as the ones we use for regular SMTP.
RFC also says that client may interrupt SASL subprotocol by sending *\r\n, but I'm not implementing this feature here.

@seriyps seriyps marked this pull request as draft September 12, 2021 13:17
@seriyps seriyps changed the title Always convert SMTP verbs to uppercase, even when no parameters Switch to SASL subprotocol on AUTH command Sep 12, 2021
@seriyps seriyps marked this pull request as ready for review September 12, 2021 16:09
@seriyps seriyps requested review from mworrell and arjan September 12, 2021 16:09
@mworrell
Copy link
Collaborator

That is quite a change (and a good one) for something that looked so simple.

@seriyps
Copy link
Collaborator Author

seriyps commented Sep 12, 2021

Thanks for your review! The change itself is not that big, mostly moved some clauses of handle_request to their own function. But yes, quite surpizing!

@seriyps seriyps merged commit c709fda into gen-smtp:master Sep 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants