Jeff King <peff@xxxxxxxx> writes: > On Mon, Jun 02, 2025 at 04:29:33PM +0530, Aditya Garg wrote: > >> @@ -1405,7 +1558,11 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred) >> >> server_fill_credential(srvc, cred); >> curl_easy_setopt(curl, CURLOPT_USERNAME, srvc->user); >> - curl_easy_setopt(curl, CURLOPT_PASSWORD, srvc->pass); >> + >> + if (!srvc->auth_method || >> + strcmp(srvc->auth_method, "XOAUTH2") || >> + strcmp(srvc->auth_method, "OAUTHBEARER")) >> + curl_easy_setopt(curl, CURLOPT_PASSWORD, srvc->pass); > > Coverity complains that this "if" will always be true, since one of the > strcmp() calls must return non-zero (srvc->auth_method cannot match both > strings!). > > I'm not sure what the logic is supposed to be here. If we are matching > either string, it should be !strcmp() for both. If we want to match > neither, then it should be &&, not ||. "If XOAUTH2 or OAUTHBEARER, use the password" sounds somewhat strange (unless the bearer token is stored in .pass and passed as if it is a password). "Unless XOAUTH2 or OAUTHBEARER, use the password" sounds even more strange. What about other methods that are not a plain simple password authentication? Will we remember extending this code when we add yet another one to exclude it like XOAUTH2 and OAUTHBEARER are excluded with this patch?
