Re: [PATCH v12 02/10] imap-send: add support for OAuth2.0 authentication

> Jeff King <peff@peff.net> 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?