--- Begin Message ---
|
Subject: |
27.0.50; [PATCH] password-cache.el: confuses key absence with nil password |
|
Date: |
Mon, 29 Jul 2019 07:12:03 +0200 |
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 <address@hidden>
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.
--- End Message ---