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

[Junio C Hamano <gitster@pobox.com>]

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?