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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 5, 2025, 16:08 UTC
Message-ID: <xmqq7c1q9nnm.fsf@gitster.g>
In-Reply-To: <PN3PR01MB9597EA7301052F34B6FE3E48B86FA@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM>

Aditya Garg <gargaditya08@live.com> writes:

Show 25 quoted lines

>> On 5 Jun 2025, at 1:30 PM, Jeff King <peff@peff.net> wrote:
>> 
>> 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 ||.
>
> Good catch. The aim was to not execute that statement if authentication is
> XOAUTH2 or OAUTHBEARER. I'll fix this logic.

Yup.  I'll refrain from merging it down before the reroll.