Subject: bug#72526: 31.0.50; [PATCH] Fix url-basic-auth secret search when passing username and/or port

From: Eli Zaretskii <[email-masked]>
Date: Sun, 18 Aug 2024 08:15:47 +0300

> From: Björn Bidar <[email-masked]>
> Cc: [email-masked]
> Date: Sat, 17 Aug 2024 23:50:51 +0300
> 
> Eli Zaretskii <[email-masked]> writes:
> 
> >> > Sorry, I don't see any experts around to ask to do that.
> >> 
> >> Maybe the maintainer of the url package?
> >
> > Whom did you have in mind?  url.el says "emacs-devel", which is
> > basically no one and everyone.
> 
> I don't know, the person that usually deals with the package?

I couldn't find him or her in the recent logs.  I concluded we didn't
have such a person/

> >> > Maybe if you'd posted a more detailed description of the problem and
> >> > its context, someone could follow your arguments and do a meaningful
> >> > review.  E.g., it sounds from your description like the case of URLs
> >> > where it currently fails was not meant to be supported by this
> >> > library?  If so, perhaps an alternative is to submit to this library
> >> > only URLs that it supports, like after stripping the port part?
> >> 
> >> The problem is that the user in url-basic-auth when handling urls like <uri-type>://<user>@<host> isn't
> >> forwarded to auth-source. Further it also appends to port to the
> >> hostname of host which means that the host is invalid since the hostname
> >> includes the port number.
> >> 
> >> >From what I read when looking at url-auth.el at line 84 it does support
> >> this kind of case of url as it already handles the same type of url when
> >> it deals with <uri-type>://<user>:<password>@<host>.
> >
> > So how come this code was not fixed since the day it was added to
> > Emacs, so long ago?
> 
> I don't know I assume it was never an issue at that time?
> In any case amending the port to the :host key seems like a bug to me.
> Similarly when the user specifies the user in the url it should be
> passed to auth-source so it can find the credentials.

So please take me through your changes with more detailed
explanations, without assuming I know my way around this code.  A few
things bother me just by looking at the diffs: (1) why do we need to
calculate 'server' more than once using the same code in the same
function, and (2) will auth-source-search as called in
url-do-auth-source-search DTRT when called with ':user nil', which
will happen if the last argument is omitted.  I also wonder what is
the semantics of the call to auth-source-search in the current code
where the user is omitted.  AFAIU, in that case Emacs will prompt the
user for username?  If so, why is it a good idea to pass to
auth-source-search USER derived by url-basic-auth, instead of
prompting?

IOW, I'd like to open what the current code and your changes do for a
more detailed discussion, so we could be sure the change is TRT before
we decide what changes to install.

Thanks.