GNU bug report logs - #79486
[PATCH] Lookup cached basic-auth credentials with the correct key

Previous Next

To reply to this bug, email your comments to 79486 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

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

From: Steven Allen <steven <at> stebalien.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Lookup cached basic-auth credentials with the correct key
Date: Sun, 21 Sep 2025 18:46:13 -0700

[Message part 1 (text/plain, inline)]

Tags: patch


This patch fixes credential caching for basic auth in the url
package. Credentials are cached with "server:port" as the key but were
being retrieved from the cache by "server" (leading to a cache miss
every time).

In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, cairo version
 1.18.4) of 2025-09-11 built on Laptop
Repository revision: 819574e13e5dcefdff136033012d6d34f8940848
Repository branch: makepkg
Windowing system distributor 'The X.Org Foundation', version 11.0.12101018
System Description: Arch Linux

Configured using:
 'configure
 'CPPFLAGS=-I/run/user/1000/build/emacs-git/src/mps-git/build/include '
 'LDFLAGS=-L/run/user/1000/build/emacs-git/src/mps-git/build/lib -Wl,-O1
 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now
 -Wl,-z,pack-relative-relocs -flto=auto' --prefix=/usr --sysconfdir=/etc
 --libexecdir=/usr/lib --localstatedir=/var --mandir=/usr/share/man
 --with-gameuser=:games --with-modules --without-m17n-flt
 --without-selinux --without-pop --without-gconf --disable-gc-mark-trace
 --with-mps=yes --enable-autodepend --enable-link-time-optimization
 --with-native-compilation=yes --with-xinput2 --with-x-toolkit=no
 --without-toolkit-scroll-bars --without-xaw3d --without-gsettings
 --with-cairo-xcb --without-xft --with-sound=no --with-tree-sitter
 --without-gpm --without-compress-install
 '--program-transform-name=s/\([ec]tags\)/\1.emacs/'
 'CFLAGS=-march=native -mtune=native -O3 -pipe -fno-plt -fexceptions
 -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security
 -fstack-clash-protection -fcf-protection -fomit-frame-pointer
 -fno-math-errno -fno-trapping-math -Os -fno-math-errno
 -fno-trapping-math -Os -flto=auto''

[0001-Lookup-cached-basic-auth-credentials-with-the-correc.patch (text/x-patch, attachment)]

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

From: Steven Allen <steven <at> stebalien.com>
To: 79486 <at> debbugs.gnu.org
Subject: Re: [PATCH] Lookup cached basic-auth credentials with the correct key
Date: Thu, 25 Sep 2025 12:09:31 -0700

[Message part 1 (text/plain, inline)]

This second patch (applies on top of the first patch) is a followup to
the original fix to Bug#72526 (commit a7a22e7c). That fix removed the
port from the auth-source :host for basic-auth however:

1. It didn't do the same for digest-auth.
2. It used the URL's "type" (http/https) instead of the actual port as
    the auth-source :port.

This commit addresses these two issues. Before this change:

1. basic-auth used the URL's "host" as the auth-source :host and the
    URL's "type" (http/https) as the auth-source ":port".
2. digest-auth used "host:port" as the auth-source :host and the URL's
    "type" as the auth-source :port.

Now, both basic-auth and digest-auth use the URL's "host" (sans port) as
the auth-source :host and the URL's "port" as the auth-source
:port *unless* the URL's port is the "default" port, in which case it
uses the URL's "type" (i.e., "http://foo:99" uses "99" as the port and
"http://foo:80" uses "http").

This second patch is not quite done. TODO:

- NEWS. Should the documentation for this change go under the "Auth
  Source" heading or should I create a new section for "URL"?
- Should this always use the port number instead of using the "type"
  when the port number is the default? I chose to use the "type" in this
  case to (a) make this change less breaking and (b) better distinguish
  between URLs that don't have ports (e.g., unix://...) but I'm happy to
  change this.

[0002-Use-the-correct-host-port-when-searching-auth-source.patch (text/x-patch, attachment)]

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

From: Steven Allen <steven <at> stebalien.com>
To: 79486 <at> debbugs.gnu.org
Subject: Re: bug#79486: Acknowledgement ([PATCH] Lookup cached basic-auth
 credentials with the correct key)
Date: Fri, 17 Oct 2025 08:38:17 -0700

I've been using both of these patches for about a month now with rqbit
[1] and haven't run into any issues. The first patch, at least, is
purely a bug fix (with a test) so it would be nice to get that merged
when someone gets the chance.

[1] https://github.com/emacs-pe/rqbit.el

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Steven Allen <steven <at> stebalien.com>
Cc: 79486 <at> debbugs.gnu.org
Subject: Re: bug#79486: Acknowledgement ([PATCH] Lookup cached basic-auth
 credentials with the correct key)
Date: Sat, 08 Nov 2025 12:27:32 +0200

> From: Steven Allen <steven <at> stebalien.com>
> Date: Fri, 17 Oct 2025 08:38:17 -0700
> 
> 
> I've been using both of these patches for about a month now with rqbit
> [1] and haven't run into any issues. The first patch, at least, is
> purely a bug fix (with a test) so it would be nice to get that merged
> when someone gets the chance.

I wanted to install the first patch, but it causes the tests to fail:

  Test url-auth-test-auth-retrieve-cache backtrace:
    url-basic-auth("http://other.com/path" nil nil nil (("nonce" . "serv
    (setq auth (url-basic-auth (plist-get row :url) nil nil (plist-get r
    (let ((row (car tail))) (setq auth (url-digest-auth (plist-get row :
    (while tail (let ((row (car tail))) (setq auth (url-digest-auth (pli
    (let ((tail (list (list :url "http://other.com/path" :realm nil :exp
    (let* ((url-digest-auth-storage '(("example.org:80" ("/path/auth1" "
    #f(lambda () [t] (let* ((url-digest-auth-storage '...) (url-http-rea
    #f(compiled-function () #<bytecode -0x15d378da706be607>)()
    handler-bind-1(#f(compiled-function () #<bytecode -0x15d378da706be60
    ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
    ert-run-test(#s(ert-test :name url-auth-test-auth-retrieve-cache :do
    ert-run-or-rerun-test(#s(ert--stats :selector (not (or ... ...)) :te
    ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
    ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
    ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
    eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
    command-line-1(("-L" ";." "-l" "ert" "--eval" "(setq treesit-extra-l
    command-line()
    normal-top-level()
  Test url-auth-test-auth-retrieve-cache condition:
      (void-variable url-http-real-basic-auth-storage)
     FAILED  1/8  url-auth-test-auth-retrieve-cache (0.070993 sec) at lisp/url/url-auth-tests.el:136

So please fix this and resubmit the patch.

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

From: Steven Allen <steven <at> stebalien.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79486 <at> debbugs.gnu.org
Subject: Re: bug#79486: Acknowledgement ([PATCH] Lookup cached basic-auth
 credentials with the correct key)
Date: Sat, 08 Nov 2025 07:52:40 -0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Steven Allen <steven <at> stebalien.com>
>> Date: Fri, 17 Oct 2025 08:38:17 -0700
>> 
>> 
>> I've been using both of these patches for about a month now with rqbit
>> [1] and haven't run into any issues. The first patch, at least, is
>> purely a bug fix (with a test) so it would be nice to get that merged
>> when someone gets the chance.
>
> I wanted to install the first patch, but it causes the tests to fail:
>
>   Test url-auth-test-auth-retrieve-cache backtrace:
>     url-basic-auth("http://other.com/path" nil nil nil (("nonce" . "serv
>     (setq auth (url-basic-auth (plist-get row :url) nil nil (plist-get r
>     (let ((row (car tail))) (setq auth (url-digest-auth (plist-get row :
>     (while tail (let ((row (car tail))) (setq auth (url-digest-auth (pli
>     (let ((tail (list (list :url "http://other.com/path" :realm nil :exp
>     (let* ((url-digest-auth-storage '(("example.org:80" ("/path/auth1" "
>     #f(lambda () [t] (let* ((url-digest-auth-storage '...) (url-http-rea
>     #f(compiled-function () #<bytecode -0x15d378da706be607>)()
>     handler-bind-1(#f(compiled-function () #<bytecode -0x15d378da706be60
>     ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>     ert-run-test(#s(ert-test :name url-auth-test-auth-retrieve-cache :do
>     ert-run-or-rerun-test(#s(ert--stats :selector (not (or ... ...)) :te
>     ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
>     ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
>     ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
>     eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
>     command-line-1(("-L" ";." "-l" "ert" "--eval" "(setq treesit-extra-l
>     command-line()
>     normal-top-level()
>   Test url-auth-test-auth-retrieve-cache condition:
>       (void-variable url-http-real-basic-auth-storage)
>      FAILED  1/8  url-auth-test-auth-retrieve-cache (0.070993 sec) at lisp/url/url-auth-tests.el:136

Ah, I tested interactively. Fixed patch attached (adds a missing require
to the test).

[0001-Lookup-cached-basic-auth-credentials-with-the-correc.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Steven Allen <steven <at> stebalien.com>
Cc: 79486 <at> debbugs.gnu.org
Subject: Re: bug#79486: Acknowledgement ([PATCH] Lookup cached basic-auth
 credentials with the correct key)
Date: Sat, 08 Nov 2025 19:23:56 +0200

> From: Steven Allen <steven <at> stebalien.com>
> Cc: 79486 <at> debbugs.gnu.org
> Date: Sat, 08 Nov 2025 07:52:40 -0800
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Steven Allen <steven <at> stebalien.com>
> >> Date: Fri, 17 Oct 2025 08:38:17 -0700
> >> 
> >> 
> >> I've been using both of these patches for about a month now with rqbit
> >> [1] and haven't run into any issues. The first patch, at least, is
> >> purely a bug fix (with a test) so it would be nice to get that merged
> >> when someone gets the chance.
> >
> > I wanted to install the first patch, but it causes the tests to fail:
> >
> >   Test url-auth-test-auth-retrieve-cache backtrace:
> >     url-basic-auth("http://other.com/path" nil nil nil (("nonce" . "serv
> >     (setq auth (url-basic-auth (plist-get row :url) nil nil (plist-get r
> >     (let ((row (car tail))) (setq auth (url-digest-auth (plist-get row :
> >     (while tail (let ((row (car tail))) (setq auth (url-digest-auth (pli
> >     (let ((tail (list (list :url "http://other.com/path" :realm nil :exp
> >     (let* ((url-digest-auth-storage '(("example.org:80" ("/path/auth1" "
> >     #f(lambda () [t] (let* ((url-digest-auth-storage '...) (url-http-rea
> >     #f(compiled-function () #<bytecode -0x15d378da706be607>)()
> >     handler-bind-1(#f(compiled-function () #<bytecode -0x15d378da706be60
> >     ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
> >     ert-run-test(#s(ert-test :name url-auth-test-auth-retrieve-cache :do
> >     ert-run-or-rerun-test(#s(ert--stats :selector (not (or ... ...)) :te
> >     ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
> >     ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
> >     ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
> >     eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
> >     command-line-1(("-L" ";." "-l" "ert" "--eval" "(setq treesit-extra-l
> >     command-line()
> >     normal-top-level()
> >   Test url-auth-test-auth-retrieve-cache condition:
> >       (void-variable url-http-real-basic-auth-storage)
> >      FAILED  1/8  url-auth-test-auth-retrieve-cache (0.070993 sec) at lisp/url/url-auth-tests.el:136
> 
> Ah, I tested interactively. Fixed patch attached (adds a missing require
> to the test).

Thanks, now installed on the master branch.

Should I now close bug#79628?  And what about this bug: should it
remain open?

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

From: Steven Allen <steven <at> stebalien.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79486 <at> debbugs.gnu.org
Subject: Re: bug#79486: Acknowledgement ([PATCH] Lookup cached basic-auth
 credentials with the correct key)
Date: Sat, 08 Nov 2025 10:22:10 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Steven Allen <steven <at> stebalien.com>
>> Cc: 79486 <at> debbugs.gnu.org
>> Date: Sat, 08 Nov 2025 07:52:40 -0800
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >> From: Steven Allen <steven <at> stebalien.com>
>> >> Date: Fri, 17 Oct 2025 08:38:17 -0700
>> >> 
>> >> 
>> >> I've been using both of these patches for about a month now with rqbit
>> >> [1] and haven't run into any issues. The first patch, at least, is
>> >> purely a bug fix (with a test) so it would be nice to get that merged
>> >> when someone gets the chance.
>> >
>> > I wanted to install the first patch, but it causes the tests to fail:
>> >
>> >   Test url-auth-test-auth-retrieve-cache backtrace:
>> >     url-basic-auth("http://other.com/path" nil nil nil (("nonce" . "serv
>> >     (setq auth (url-basic-auth (plist-get row :url) nil nil (plist-get r
>> >     (let ((row (car tail))) (setq auth (url-digest-auth (plist-get row :
>> >     (while tail (let ((row (car tail))) (setq auth (url-digest-auth (pli
>> >     (let ((tail (list (list :url "http://other.com/path" :realm nil :exp
>> >     (let* ((url-digest-auth-storage '(("example.org:80" ("/path/auth1" "
>> >     #f(lambda () [t] (let* ((url-digest-auth-storage '...) (url-http-rea
>> >     #f(compiled-function () #<bytecode -0x15d378da706be607>)()
>> >     handler-bind-1(#f(compiled-function () #<bytecode -0x15d378da706be60
>> >     ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>> >     ert-run-test(#s(ert-test :name url-auth-test-auth-retrieve-cache :do
>> >     ert-run-or-rerun-test(#s(ert--stats :selector (not (or ... ...)) :te
>> >     ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
>> >     ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
>> >     ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
>> >     eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
>> >     command-line-1(("-L" ";." "-l" "ert" "--eval" "(setq treesit-extra-l
>> >     command-line()
>> >     normal-top-level()
>> >   Test url-auth-test-auth-retrieve-cache condition:
>> >       (void-variable url-http-real-basic-auth-storage)
>> >      FAILED  1/8  url-auth-test-auth-retrieve-cache (0.070993 sec) at lisp/url/url-auth-tests.el:136
>> 
>> Ah, I tested interactively. Fixed patch attached (adds a missing require
>> to the test).
>
> Thanks, now installed on the master branch.

Thanks!

> Should I now close bug#79628?

As far as I can tell, yes.

> And what about this bug: should it remain open?

I have a second patch (see the second message) to fix auth-source
integration in url-auth, but I'm also happy to submit it as a separate
bug report. That patch still has two outstanding questions that I'll
need help answering:

- NEWS. Should the documentation for this change go under the "Auth
  Source" heading or should I create a new section for "URL"?
- Should this always use the port number instead of using the "type"
  when the port number is the default? I chose to use the "type" in this
  case to (a) make this change less breaking and (b) better distinguish
  between URLs that don't have ports (e.g., unix://...) but I'm happy to
  change this.

I'm also happy to submit the patch to the dev mailing list if that's a
better place to get such feedback.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Steven Allen <steven <at> stebalien.com>
Cc: 79486 <at> debbugs.gnu.org
Subject: Re: bug#79486: Acknowledgement ([PATCH] Lookup cached basic-auth
 credentials with the correct key)
Date: Sat, 08 Nov 2025 20:42:27 +0200

> From: Steven Allen <steven <at> stebalien.com>
> Cc: 79486 <at> debbugs.gnu.org
> Date: Sat, 08 Nov 2025 10:22:10 -0800
> 
> > And what about this bug: should it remain open?
> 
> I have a second patch (see the second message) to fix auth-source
> integration in url-auth, but I'm also happy to submit it as a separate
> bug report. That patch still has two outstanding questions that I'll
> need help answering:
> 
> - NEWS. Should the documentation for this change go under the "Auth
>   Source" heading or should I create a new section for "URL"?
> - Should this always use the port number instead of using the "type"
>   when the port number is the default? I chose to use the "type" in this
>   case to (a) make this change less breaking and (b) better distinguish
>   between URLs that don't have ports (e.g., unix://...) but I'm happy to
>   change this.
> 
> I'm also happy to submit the patch to the dev mailing list if that's a
> better place to get such feedback.

Yes, please post it there, and I hope someone who knows more than I do
about this stuff will chime in.

Previous Next

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