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

Previous Next

Full log

Message #26 received at 72526 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: 72526 <at> debbugs.gnu.org
Subject: Re: bug#72526: 31.0.50; [PATCH] Fix url-basic-auth secret search
 when passing username and/or port
Date: Sun, 18 Aug 2024 08:15:47 +0300

> From: Björn Bidar <bjorn.bidar <at> thaodan.de>
> Cc: 72526 <at> debbugs.gnu.org
> Date: Sat, 17 Aug 2024 23:50:51 +0300
> 
> Eli Zaretskii <eliz <at> gnu.org> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.