On Thu, Jun 05, 2025 at 09:28:18AM -0700, Junio C Hamano wrote: > 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? That was my gut feeling, but I confess to not engaging my brain and was just relaying the coverity message before logging off for the day. ;) -Peff
