From: "Óscar Fuentes" <ofv@wanadoo.es>
To: 36834@debbugs.gnu.org
Subject: bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password
Date: Mon, 29 Jul 2019 07:12:03 +0200 [thread overview]
Message-ID: <875znltof0.fsf@telefonica.net> (raw)
Recently observed that Gnus was creating lots of timers for
password-cache-remove, about 6 every time that it fetched new news/mail.
Upon inspection the cause was found in a change to password-cache.el:
commit d66dcde46a87ee8a9064db3d9b05da9b17036f5b
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Fri Jul 28 12:27:00 2017 -0400
* lisp/password-cache.el (password-data): Use a hash-table
* lisp/auth-source.el (auth-source-magic): Remove.
(auth-source-forget+, auth-source-forget-all-cached): Adjust to new
format of password-data.
(auth-source-format-cache-entry): Just use a cons.
(password-cache-remove, password-cache-add, password-reset)
(password-read-from-cache, password-in-cache-p): Adjust accordingly.
Fixes: bug#26699
The points of interest of that change are:
(defun password-in-cache-p (key)
"Check if KEY is in the cache."
(and password-cache
key
- (intern-soft key password-data)))
+ (gethash key password-data)))
and
(defun password-cache-add (key password)
"Add password to cache.
The password is removed by a timer after `password-cache-expiry' seconds."
- (when (and password-cache-expiry (null (intern-soft key password-data)))
+ (when (and password-cache-expiry (null (gethash key password-data)))
The change uses gethash instead of intern-soft, but those functions act
differently when the password (the value associated with the key) was
nil. The effect is that every call to password-cache-add with nil as
password creates a new timer, and password-in-cache-p returns nil if
there exists a (key nil) entry on password-data, when previously it
would return non-nil.
So I propose this patch:
diff --git a/lisp/password-cache.el b/lisp/password-cache.el
index 5a09ae4859..6009fb491e 100644
--- a/lisp/password-cache.el
+++ b/lisp/password-cache.el
@@ -81,7 +81,8 @@ password-in-cache-p
"Check if KEY is in the cache."
(and password-cache
key
- (gethash key password-data)))
+ (not (eq (gethash key password-data 'password-cache-no-data)
+ 'password-cache-no-data))))
(defun password-read (prompt &optional key)
"Read password, for use with KEY, from user, or from cache if wanted.
@@ -125,7 +126,9 @@ password-cache-remove
(defun password-cache-add (key password)
"Add password to cache.
The password is removed by a timer after `password-cache-expiry' seconds."
- (when (and password-cache-expiry (null (gethash key password-data)))
+ (when (and password-cache-expiry
+ (eq (gethash key password-data 'password-cache-no-data)
+ 'password-cache-no-data))
(run-at-time password-cache-expiry nil
#'password-cache-remove
key))
Okay to commit? To emacs-26 or master?
On another topic, before a cache entry is removed we try to overwrite
the stored password (see password-cache-remove). However, the same
change did this:
(defun password-reset ()
"Clear the password cache."
(interactive)
- (fillarray password-data 0))
+ (clrhash password-data))
I don't know if clrhash overwrites the data before releasing it.
next reply other threads:[~2019-07-29 5:12 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-29 5:12 Óscar Fuentes [this message]
2019-07-29 8:36 ` bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password Basil L. Contovounesios
2019-07-29 14:12 ` Óscar Fuentes
2019-08-10 8:19 ` Eli Zaretskii
2019-08-11 16:00 ` Basil L. Contovounesios
2019-08-10 9:14 ` Stefan Monnier
2019-08-10 10:01 ` Eli Zaretskii
2019-08-11 23:45 ` Óscar Fuentes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=875znltof0.fsf@telefonica.net \
--to=ofv@wanadoo.es \
--cc=36834@debbugs.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).