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

From: Aditya Garg <gargaditya08@live.com>
Date: Jun 5, 2025, 16:17 UTC
Message-ID: <PN3PR01MB9597690D45F0E539C35FF4F8B86FA@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM>
In-Reply-To: <xmqq7c1q9nnm.fsf@gitster.g>

Show 31 quoted lines

> On 5 Jun 2025, at 9:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Aditya Garg <gargaditya08@live.com> writes:
> 
>>>> 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.

Already sent a v13 :)