Aditya Garg <gargaditya08@xxxxxxxx> writes: >>Which makes it sound like if (!srvc->auth_method) is enough? >> > > No. If the user specifies PLAIN or LOGIN then it's not enough. That invites another question. Why aren't we checking for PLAIN or LOGIN and when one of them is given use the password? What is written in the patch looks backwards, in that we seem to assume that a method that is not XOAUTH2 and OAUTHBEARER must be PLAIN or LOGIN (or anything that wants us to pass CURLOPT_PASSWORD). IOW, why isn't the code more like if (/* not using the more advanced method interface? */ !srvc->auth_method || /* method interface that takes password? */ !strcmp(srvc->auth_method, "PLAIN") || !strcmp(srvc->auth_method, "LOGIN")) curl_easy_setopt(curl, CURLOPT_PASSWORD, srvc->pass); if, after all, PLAIN/LOGIN are what triggers password authentication? > So, considering the fact that the original code for imap-send > was setting CURLOPT_LOGIN_OPTIONS > unconditionally and > was running the AUTH command even if auth was set to CRAM-MD5 > or whatever, I just preferred to not change that behaviour since I > may cause some regression. There is a tiny possibility that CRAM-MD5 > may work, but I don't really have any free SMTP server which uses > that method itself. > > In short, just to be very safe here, I decided to not mingle with the > logic much and simple decided to use a seperate tested logic > for OAuth2.0 and let the same logic be used for rest cases. Don't be short ;-) Be long in your log message to help future developers. In short, you want to make your proposed log message so clear that future developers who found this commit in "git log -p" to come asking you these questions---that is why reviewers are supposed to ask questions and ask for clarifications. > "Set CURLOPT_LOGIN_OPTIONS irrespective of whether there is > an auth method specified or not, unless it's OAuth2.0, where we > use a different curl API" That is a very good thing to write down either in-code comment and/or the log message to avoid future developers come bugging you with the same questions as I did. Thanks.
