From unknown Sun Jun 22 00:47:01 2025 X-Loop: help-debbugs@gnu.org Subject: bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password Resent-From: =?UTF-8?Q?=C3=93scar?= Fuentes Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 29 Jul 2019 05:13:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 36834 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: 36834@debbugs.gnu.org X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.15643771331432 (code B ref -1); Mon, 29 Jul 2019 05:13:01 +0000 Received: (at submit) by debbugs.gnu.org; 29 Jul 2019 05:12:13 +0000 Received: from localhost ([127.0.0.1]:47572 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hrxxE-0000N2-Uv for submit@debbugs.gnu.org; Mon, 29 Jul 2019 01:12:13 -0400 Received: from lists.gnu.org ([209.51.188.17]:35066) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hrxxD-0000Mv-Cl for submit@debbugs.gnu.org; Mon, 29 Jul 2019 01:12:11 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:40747) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hrxxC-00080F-8K for bug-gnu-emacs@gnu.org; Mon, 29 Jul 2019 01:12:11 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.1 required=5.0 tests=BAYES_50,RCVD_IN_DNSWL_LOW, URIBL_BLOCKED autolearn=disabled version=3.3.2 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hrxxB-0006EB-4B for bug-gnu-emacs@gnu.org; Mon, 29 Jul 2019 01:12:10 -0400 Received: from relayout03-redir.e.movistar.es ([86.109.101.203]:44747) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hrxxA-00069N-Pd for bug-gnu-emacs@gnu.org; Mon, 29 Jul 2019 01:12:09 -0400 Received: from sky (162.red-79-151-6.dynamicip.rima-tde.net [79.151.6.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: 981711563@telefonica.net) by relayout03.e.movistar.es (Postfix) with ESMTPSA id 45xnqS3WVbzMl2b for ; Mon, 29 Jul 2019 07:12:04 +0200 (CEST) From: =?UTF-8?Q?=C3=93scar?= Fuentes Date: Mon, 29 Jul 2019 07:12:03 +0200 Message-ID: <875znltof0.fsf@telefonica.net> MIME-Version: 1.0 Content-Type: text/plain X-CTCH-Score: 0.000 X-CTCH-ScoreCust: 0.000 X-TnetOut-Country: IP: 79.151.6.162 | Country: ES X-TnetOut-Information: AntiSPAM and AntiVIRUS on relayout03 X-TnetOut-MsgID: 45xnqS3WVbzMl2b.AAD0A X-TnetOut-SpamCheck: no es spam, Unknown X-TnetOut-From: ofv@wanadoo.es X-TnetOut-Watermark: 1564981925.87401@HFFts58ZxH+Mb5q9FWZa8A X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 86.109.101.203 X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) 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 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. From unknown Sun Jun 22 00:47:01 2025 X-Loop: help-debbugs@gnu.org Subject: bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password Resent-From: "Basil L. Contovounesios" Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 29 Jul 2019 08:37:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 36834 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: =?UTF-8?Q?=C3=93scar?= Fuentes Cc: Stefan Monnier , 36834@debbugs.gnu.org Received: via spool by 36834-submit@debbugs.gnu.org id=B36834.156438939121264 (code B ref 36834); Mon, 29 Jul 2019 08:37:02 +0000 Received: (at 36834) by debbugs.gnu.org; 29 Jul 2019 08:36:31 +0000 Received: from localhost ([127.0.0.1]:47648 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hs18w-0005Wu-LK for submit@debbugs.gnu.org; Mon, 29 Jul 2019 04:36:31 -0400 Received: from mail-wm1-f45.google.com ([209.85.128.45]:38870) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hs18t-0005Wd-CE for 36834@debbugs.gnu.org; Mon, 29 Jul 2019 04:36:28 -0400 Received: by mail-wm1-f45.google.com with SMTP id s15so31282727wmj.3 for <36834@debbugs.gnu.org>; Mon, 29 Jul 2019 01:36:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tcd-ie.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-transfer-encoding; bh=2br9Q1+m8BzhCbhD9d7xqNxxaLYP7gTiT04i2aZvYSc=; b=do2WvIHRk62YIV/tH/DYlrtZpmRjkURDnMXxzYjwrRWXR3D51eYtu8oKQHKZ7mqQsX UutedPPOdjw43wOFgAOi7ibKJ1dpzc1Dmfz9BAaFSI5cgD/sHg0oFv20i25tVe0sHjjy ec+k5oxtD2dRhRIsdelY9raw2okYT/HGJyiOj8at0XLEhf7TVhJP3fRBZTOOC0nJ1GfJ 5vgeGrSFoHe3p1F5s5a6w1FKuDiQPG7b4nQRf/1Qc1tfmv0RUxglcjViNgdqqY+CRKKz EucRTjYW1OER3bDUiDlsHniIxbWR60geyyRxJAEDoetdMBPopvEpxiSqf7uq1VG7n+p4 YTyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=2br9Q1+m8BzhCbhD9d7xqNxxaLYP7gTiT04i2aZvYSc=; b=EjP7IB69IEKgagadWafygJ2KyJdHmrQmPx99AmLvQIugOaCX0nWm/00DzdufG2K8aj tbWisLp01Cq5lv4pzNdGBCCHN3Ad6BCHrrxYnjRGZWz/oaqRF2zqCjXVq0aaDjE7wuIB lFoI7huGzngNprLEyIl7MVmjF0fk/R+PNn6+LsvHmOIz9rIkMU90BDEoVzslHp378ucG 703E/jeQPpnxRVlDwUg6QCy4t4oX5khIG+4JdB/wJFFf5/r+TosP6xRKMzz5hk+kMTz7 DYn8F/llEXgkw7qXLBm0I9udOjb0KkIC9rZ565CK2a4txXeIxCrx8tbGDyMLcKkO4UIm 86EQ== X-Gm-Message-State: APjAAAW4k4oJTuCQ1MPIwQmui5hD19gsbw88/d2iizVD3Zi6j0dcy/qf nc+EmedYfa5PLG+fxNvgoT5TJg== X-Google-Smtp-Source: APXvYqznIp279Xu72azxL1CS7uc78D51+KlngvGWN0EA+BjVip+oIF+pR4eHVhIoGsykxd7651pMPw== X-Received: by 2002:a05:600c:c4:: with SMTP id u4mr98367594wmm.96.1564389380845; Mon, 29 Jul 2019 01:36:20 -0700 (PDT) Received: from localhost ([88.128.80.170]) by smtp.gmail.com with ESMTPSA id c30sm115148739wrb.15.2019.07.29.01.36.19 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 29 Jul 2019 01:36:20 -0700 (PDT) From: "Basil L. Contovounesios" References: <875znltof0.fsf@telefonica.net> Date: Mon, 29 Jul 2019 09:36:17 +0100 In-Reply-To: <875znltof0.fsf@telefonica.net> ("=?UTF-8?Q?=C3=93scar?= Fuentes"'s message of "Mon, 29 Jul 2019 07:12:03 +0200") Message-ID: <87v9vlck5a.fsf@tcd.ie> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: 0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) =C3=93scar Fuentes writes: > 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: (CCing the author.) > commit d66dcde46a87ee8a9064db3d9b05da9b17036f5b > Author: Stefan Monnier > Date: Fri Jul 28 12:27:00 2017 -0400 > > * lisp/password-cache.el (password-data): Use a hash-table >=20=20=20=20=20 > * 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. >=20=20=20=20=20 > (password-cache-remove, password-cache-add, password-reset) > (password-read-from-cache, password-in-cache-p): Adjust accordingly. >=20=20=20=20=20 > 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. Is it valid for the password to be nil? The logic in password-read suggests otherwise. > The effect is that every call to password-cache-add with nil as > password creates a new timer, Where is password-cache-add being passed a nil password? > and password-in-cache-p returns nil if > there exists a (key nil) entry on password-data, when previously it > would return non-nil. I think a nil key is also not expected. > 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)))) Note that password-in-cache-p is currently identical to password-read-from-cache. One can probably be written in terms of the other. > (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)) Even if these "memhash" checks are TRT, I suggest either reusing or copying the hash table method of map-contains-key, rather than comparing against an interned symbol. > 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. I don't either. Thanks, --=20 Basil From unknown Sun Jun 22 00:47:01 2025 X-Loop: help-debbugs@gnu.org Subject: bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password Resent-From: =?UTF-8?Q?=C3=93scar?= Fuentes Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 29 Jul 2019 14:13:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 36834 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: "Basil L. Contovounesios" Cc: Stefan Monnier , 36834@debbugs.gnu.org Received: via spool by 36834-submit@debbugs.gnu.org id=B36834.15644095814779 (code B ref 36834); Mon, 29 Jul 2019 14:13:02 +0000 Received: (at 36834) by debbugs.gnu.org; 29 Jul 2019 14:13:01 +0000 Received: from localhost ([127.0.0.1]:48861 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hs6OZ-0001Ez-Tw for submit@debbugs.gnu.org; Mon, 29 Jul 2019 10:13:00 -0400 Received: from relayout01-redir.e.movistar.es ([86.109.101.201]:16861) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hs6OX-0001Ej-1Z for 36834@debbugs.gnu.org; Mon, 29 Jul 2019 10:12:58 -0400 Received: from sky (162.red-79-151-6.dynamicip.rima-tde.net [79.151.6.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: 981711563@telefonica.net) by relayout01.e.movistar.es (Postfix) with ESMTPSA id 45y1qM6zC5zfb8y; Mon, 29 Jul 2019 16:12:46 +0200 (CEST) From: =?UTF-8?Q?=C3=93scar?= Fuentes References: <875znltof0.fsf@telefonica.net> <87v9vlck5a.fsf@tcd.ie> Date: Mon, 29 Jul 2019 16:12:45 +0200 In-Reply-To: <87v9vlck5a.fsf@tcd.ie> (Basil L. Contovounesios's message of "Mon, 29 Jul 2019 09:36:17 +0100") Message-ID: <87wog1rkte.fsf@telefonica.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-CTCH-Score: 0.000 X-CTCH-ScoreCust: 0.000 X-TnetOut-Country: IP: 79.151.6.162 | Country: ES X-TnetOut-Information: AntiSPAM and AntiVIRUS on relayout01 X-TnetOut-MsgID: 45y1qM6zC5zfb8y.A9687 X-TnetOut-SpamCheck: no es spam, Unknown X-TnetOut-From: ofv@wanadoo.es X-TnetOut-Watermark: 1565014369.12494@uWr3Cl82H/TluDD2FlvZCw X-Spam-Status: No X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) "Basil L. Contovounesios" writes: >> The change uses gethash instead of intern-soft, but those functions act >> differently when the password (the value associated with the key) was >> nil. > > Is it valid for the password to be nil? The logic in password-read > suggests otherwise. Callers are sending nil. If it is not valid, there is a problem elsewhere, but my understanding is that a nil password means "no password" and it is cached in the memoization sense. >> The effect is that every call to password-cache-add with nil as >> password creates a new timer, > > Where is password-cache-add being passed a nil password? The caller is auth-source-remember, IIRC, which itself is called from auth-source-search. >> and password-in-cache-p returns nil if >> there exists a (key nil) entry on password-data, when previously it >> would return non-nil. > > I think a nil key is also not expected. (key nil) means a hash table entry with `key' as key and nil as value, not that key is nil. > Note that password-in-cache-p is currently identical to > password-read-from-cache. One can probably be written in terms of the > other. Yes, right now they are identical, which causes a problem, because checking for key existence shall not be the same as retrieving the value when value can be nil. > Even if these "memhash" checks are TRT, I suggest either reusing or > copying the hash table method of map-contains-key, rather than comparing > against an interned symbol. Is map-contains-key available by default? I'm wary of introducing new dependencies for saving just a few characters. From unknown Sun Jun 22 00:47:01 2025 X-Loop: help-debbugs@gnu.org Subject: bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 10 Aug 2019 08:20:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 36834 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: =?UTF-8?Q?=C3=93scar?= Fuentes Cc: contovob@tcd.ie, monnier@iro.umontreal.ca, 36834@debbugs.gnu.org Received: via spool by 36834-submit@debbugs.gnu.org id=B36834.156542515419393 (code B ref 36834); Sat, 10 Aug 2019 08:20:02 +0000 Received: (at 36834) by debbugs.gnu.org; 10 Aug 2019 08:19:14 +0000 Received: from localhost ([127.0.0.1]:43416 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hwMao-00052j-DQ for submit@debbugs.gnu.org; Sat, 10 Aug 2019 04:19:14 -0400 Received: from eggs.gnu.org ([209.51.188.92]:44116) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hwMam-00052S-5T for 36834@debbugs.gnu.org; Sat, 10 Aug 2019 04:19:12 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:50877) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hwMag-0005gr-Bm; Sat, 10 Aug 2019 04:19:06 -0400 Received: from [176.228.60.248] (port=3804 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1hwMaf-0002oy-SE; Sat, 10 Aug 2019 04:19:06 -0400 Date: Sat, 10 Aug 2019 11:19:01 +0300 Message-Id: <831rxta0wa.fsf@gnu.org> From: Eli Zaretskii In-reply-to: <87wog1rkte.fsf@telefonica.net> (message from =?UTF-8?Q?=C3=93scar?= Fuentes on Mon, 29 Jul 2019 16:12:45 +0200) References: <875znltof0.fsf@telefonica.net> <87v9vlck5a.fsf@tcd.ie> <87wog1rkte.fsf@telefonica.net> MIME-version: 1.0 Content-type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Ping! Any followups? Stefan? Should we just push this? > From: Óscar Fuentes > Date: Mon, 29 Jul 2019 16:12:45 +0200 > Cc: Stefan Monnier , 36834@debbugs.gnu.org > > "Basil L. Contovounesios" writes: > > >> The change uses gethash instead of intern-soft, but those functions act > >> differently when the password (the value associated with the key) was > >> nil. > > > > Is it valid for the password to be nil? The logic in password-read > > suggests otherwise. > > Callers are sending nil. If it is not valid, there is a problem > elsewhere, but my understanding is that a nil password means "no > password" and it is cached in the memoization sense. > > >> The effect is that every call to password-cache-add with nil as > >> password creates a new timer, > > > > Where is password-cache-add being passed a nil password? > > The caller is auth-source-remember, IIRC, which itself is called from > auth-source-search. Can you show a reproducer? From unknown Sun Jun 22 00:47:01 2025 X-Loop: help-debbugs@gnu.org Subject: bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 10 Aug 2019 09:15:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 36834 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: =?UTF-8?Q?=C3=93scar?= Fuentes Cc: 36834@debbugs.gnu.org Received: via spool by 36834-submit@debbugs.gnu.org id=B36834.156542847424653 (code B ref 36834); Sat, 10 Aug 2019 09:15:02 +0000 Received: (at 36834) by debbugs.gnu.org; 10 Aug 2019 09:14:34 +0000 Received: from localhost ([127.0.0.1]:43459 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hwNSL-0006PZ-T9 for submit@debbugs.gnu.org; Sat, 10 Aug 2019 05:14:34 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:27508) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hwNSK-0006PP-EM for 36834@debbugs.gnu.org; Sat, 10 Aug 2019 05:14:32 -0400 Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id E5D1E103310; Sat, 10 Aug 2019 05:14:26 -0400 (EDT) Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 61CE91032F4; Sat, 10 Aug 2019 05:14:25 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1565428465; bh=r/jGDIKg0XNebnQ53PyzV/pLAWI3MfbDGsVFZhWsq58=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=kV+htCZ5kD5Ofe2qVrZhmamCO9hWpw03nbNBX/TZpf352FUq79CupnEQwveUe28L8 3Psocvyg5fFHN6ch3cYpMIus9IgBkrRgPCWXRxe++g/JTSeUFOfjU97XIa14OlpFwl whhEA2oTnveC3QRxRCgeMIqeOpmjisq6KXRB7wxL63vlJ+7w2Zkk4iKINFG19FwF93 2T6cWy5a3tDqkg8SZYqB2X2lOChTzXLOVIrRPwwYBpaF1FFAazm6/DlG6eklJ3ik0k 3bRM/f87hcAIL0ge16Gsf4ADlofX55B314wq/+a8DdZPmWUzA37AK7ldf0L7Y36cjI Tv+8yAn/jko5g== Received: from alfajor (dyn.83-228-179-131.dsl.vtx.ch [83.228.179.131]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id AB340120A4C; Sat, 10 Aug 2019 05:14:24 -0400 (EDT) From: Stefan Monnier Message-ID: References: <875znltof0.fsf@telefonica.net> Date: Sat, 10 Aug 2019 05:14:22 -0400 In-Reply-To: <875znltof0.fsf@telefonica.net> ("=?UTF-8?Q?=C3=93scar?= Fuentes"'s message of "Mon, 29 Jul 2019 07:12:03 +0200") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP AWL 0.132 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain X-SPAM-LEVEL: X-Spam-Score: 0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) > 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)) Looks good to me, thanks. > 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)) Obarrays are just arrays of symbols (i.e. arrays of pointers to symbol objects), so (fillarray password-data 0) does not overwrite the passwords stored in the symbols, it just overwrites the pointers to those symbols. (clrhash password-data) has basically the same effect. Stefan From unknown Sun Jun 22 00:47:01 2025 X-Loop: help-debbugs@gnu.org Subject: bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 10 Aug 2019 10:02:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 36834 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Stefan Monnier Cc: ofv@wanadoo.es, 36834@debbugs.gnu.org Received: via spool by 36834-submit@debbugs.gnu.org id=B36834.156543129828982 (code B ref 36834); Sat, 10 Aug 2019 10:02:02 +0000 Received: (at 36834) by debbugs.gnu.org; 10 Aug 2019 10:01:38 +0000 Received: from localhost ([127.0.0.1]:43480 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hwOBs-0007XL-Sg for submit@debbugs.gnu.org; Sat, 10 Aug 2019 06:01:37 -0400 Received: from eggs.gnu.org ([209.51.188.92]:54684) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hwOBr-0007XA-Fv for 36834@debbugs.gnu.org; Sat, 10 Aug 2019 06:01:35 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:51637) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hwOBk-0006fz-3Q; Sat, 10 Aug 2019 06:01:29 -0400 Received: from [176.228.60.248] (port=2134 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1hwOBg-0000xx-El; Sat, 10 Aug 2019 06:01:25 -0400 Date: Sat, 10 Aug 2019 13:01:18 +0300 Message-Id: <83lfw18hld.fsf@gnu.org> From: Eli Zaretskii In-reply-to: (message from Stefan Monnier on Sat, 10 Aug 2019 05:14:22 -0400) References: <875znltof0.fsf@telefonica.net> MIME-version: 1.0 Content-type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > From: Stefan Monnier > Date: Sat, 10 Aug 2019 05:14:22 -0400 > Cc: 36834@debbugs.gnu.org > > > 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)) > > Looks good to me, thanks. Thanks. Óscar, please push to the emacs-26 branch, and thanks. From unknown Sun Jun 22 00:47:01 2025 X-Loop: help-debbugs@gnu.org Subject: bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password Resent-From: "Basil L. Contovounesios" Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 11 Aug 2019 16:01:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 36834 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: =?UTF-8?Q?=C3=93scar?= Fuentes Cc: Stefan Monnier , 36834@debbugs.gnu.org Received: via spool by 36834-submit@debbugs.gnu.org id=B36834.156553923932245 (code B ref 36834); Sun, 11 Aug 2019 16:01:01 +0000 Received: (at 36834) by debbugs.gnu.org; 11 Aug 2019 16:00:39 +0000 Received: from localhost ([127.0.0.1]:45715 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hwqGt-0008O0-2T for submit@debbugs.gnu.org; Sun, 11 Aug 2019 12:00:39 -0400 Received: from mail-wm1-f45.google.com ([209.85.128.45]:32965) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hwqGq-0008Nm-Mm for 36834@debbugs.gnu.org; Sun, 11 Aug 2019 12:00:37 -0400 Received: by mail-wm1-f45.google.com with SMTP id p77so9300662wme.0 for <36834@debbugs.gnu.org>; Sun, 11 Aug 2019 09:00:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tcd-ie.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-transfer-encoding; bh=4eWNLTpIPjUB05ELbbeTowg+XMRNOAIDofTrSq0fL5I=; b=OZ9+DF5mT3X70YWJe2nFEVMWppQuVJGF16ygHKfpmdG/r76+gNmlCf+MrSxq5knG9X PKVaPRevDwdtleSZes8T4FssjJw0Xa6zYO3ksdxtnG0KVVuMPFL7CoaHnNpe/B8rHNMW IZu0xciyBrtrRhpCAZg1flCmPE2aHMfyNyWxFA0KZtReTaB4V/h/jYbuqDt4m3Horr+4 Ow3qmJtU4GAQwaofC8VNQ/vkmPmao0kpseNSVYqRbvnL8tMq8HrpAxoaqDgiN/Vr7gzy 4VDxnbo3YTsmvQHwD7tBWkZSb/0tehXZ9LW4/NXQ1mr3KTulzkLEEnMS1k4JftJ6Avv8 WWFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=4eWNLTpIPjUB05ELbbeTowg+XMRNOAIDofTrSq0fL5I=; b=QRl9NOUbO3OtznN1TlQZN4F/mJpUikki0GJxwIuQ9+s1t2WNwm1M5O4BF/RTyEGMxs gRAqtSSBnzAZjBU83HRtwThB0ynQDwocZ3KI+ixw2cp/aGnUoC6Y7vOM/ftYJ1vLCPpW 55b1BFsxyEXC/WlGVfTvYxaoiz9ugASXIBczLD15iz5/xee94TjDUZUcoql6lOq4Rlzy yJQyqf0sXRbGXH43kkcVeLWtGWwR1NcMXcxLFzQJqAaBvPF2oR++yf45V1ke+2iXVkv7 cdPoTaPCvqZYzxV+eYFJtIvAlw1tQ6clppZ/kMThv40pRS3BLSgbq7tMDDbQDTyMD2BM RzHw== X-Gm-Message-State: APjAAAU6SDb4hlY/mkXpmEQAXfhtSM5BwGBHuxBexCAyl7xisJhSu80j vbjPRh81uDH3GEaE4OTnqukyfw== X-Google-Smtp-Source: APXvYqye9gHnv9eHxbOMP9Wj+ic7AG0jcgDSsEfbTI4RJF9pHDesBFHJ6hzwTnZFF1SD9DwchpeLjA== X-Received: by 2002:a1c:20c3:: with SMTP id g186mr22494706wmg.15.1565539230655; Sun, 11 Aug 2019 09:00:30 -0700 (PDT) Received: from localhost (adsl-243.37.6.53.tellas.gr. [37.6.53.243]) by smtp.gmail.com with ESMTPSA id a26sm15598338wmg.45.2019.08.11.09.00.26 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Sun, 11 Aug 2019 09:00:29 -0700 (PDT) From: "Basil L. Contovounesios" References: <875znltof0.fsf@telefonica.net> <87v9vlck5a.fsf@tcd.ie> <87wog1rkte.fsf@telefonica.net> Date: Sun, 11 Aug 2019 19:00:21 +0300 In-Reply-To: <87wog1rkte.fsf@telefonica.net> ("=?UTF-8?Q?=C3=93scar?= Fuentes"'s message of "Mon, 29 Jul 2019 16:12:45 +0200") Message-ID: <877e7jn14a.fsf@tcd.ie> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: 0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) =C3=93scar Fuentes writes: > "Basil L. Contovounesios" writes: > >>> The change uses gethash instead of intern-soft, but those functions act >>> differently when the password (the value associated with the key) was >>> nil. >> >> Is it valid for the password to be nil? The logic in password-read >> suggests otherwise. > > Callers are sending nil. If it is not valid, there is a problem > elsewhere, but my understanding is that a nil password means "no > password" and it is cached in the memoization sense. If clients of password-cache.el already rely on this, IWBNI it were mentioned somewhere. >>> The effect is that every call to password-cache-add with nil as >>> password creates a new timer, >> >> Where is password-cache-add being passed a nil password? > > The caller is auth-source-remember, IIRC, which itself is called from > auth-source-search. Indeed, that's what it looks like: ;; note we remember the lack of result too, if it's applicable (when auth-source-do-cache (auth-source-remember spec found))) >>> and password-in-cache-p returns nil if >>> there exists a (key nil) entry on password-data, when previously it >>> would return non-nil. >> >> I think a nil key is also not expected. > > (key nil) means a hash table entry with `key' as key and nil as value, > not that key is nil. Ah, sorry. >> Note that password-in-cache-p is currently identical to >> password-read-from-cache. One can probably be written in terms of the >> other. > > Yes, right now they are identical, which causes a problem, because > checking for key existence shall not be the same as retrieving the value > when value can be nil. > >> Even if these "memhash" checks are TRT, I suggest either reusing or >> copying the hash table method of map-contains-key, rather than comparing >> against an interned symbol. > > Is map-contains-key available by default? I'm wary of introducing new > dependencies for saving just a few characters. That's why I said "either reusing or _copying_" what map-contains-key does, namely not using an interned symbol: (let ((v '(nil))) (not (eq v (gethash key map v)))) But as long as passwords are strings or nil it doesn't really matter. Thanks, --=20 Basil From unknown Sun Jun 22 00:47:01 2025 MIME-Version: 1.0 X-Mailer: MIME-tools 5.505 (Entity 5.505) X-Loop: help-debbugs@gnu.org From: help-debbugs@gnu.org (GNU bug Tracking System) To: =?UTF-8?Q?=C3=93scar?= Fuentes Subject: bug#36834: closed (Re: bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password) Message-ID: References: <878srznu59.fsf@telefonica.net> <875znltof0.fsf@telefonica.net> X-Gnu-PR-Message: they-closed 36834 X-Gnu-PR-Package: emacs X-Gnu-PR-Keywords: patch Reply-To: 36834@debbugs.gnu.org Date: Sun, 11 Aug 2019 23:46:02 +0000 Content-Type: multipart/mixed; boundary="----------=_1565567162-3730-1" This is a multi-part message in MIME format... ------------=_1565567162-3730-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Your bug report #36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil p= assword which was filed against the emacs package, has been closed. The explanation is attached below, along with your original report. If you require more details, please reply to 36834@debbugs.gnu.org. --=20 36834: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D36834 GNU Bug Tracking System Contact help-debbugs@gnu.org with problems ------------=_1565567162-3730-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at 36834-done) by debbugs.gnu.org; 11 Aug 2019 23:45:49 +0000 Received: from localhost ([127.0.0.1]:45907 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hwxX3-0000xm-8A for submit@debbugs.gnu.org; Sun, 11 Aug 2019 19:45:49 -0400 Received: from relayout04.e.movistar.es ([86.109.101.204]:47253 helo=relayout04-redir.e.movistar.es) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hwxX1-0000xX-MG for 36834-done@debbugs.gnu.org; Sun, 11 Aug 2019 19:45:48 -0400 Received: from sky (162.red-79-151-6.dynamicip.rima-tde.net [79.151.6.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: 981711563@telefonica.net) by relayout04.e.movistar.es (Postfix) with ESMTPSA id 466FwL6rDYz1065 for <36834-done@debbugs.gnu.org>; Mon, 12 Aug 2019 01:45:38 +0200 (CEST) From: =?utf-8?Q?=C3=93scar_Fuentes?= To: 36834-done@debbugs.gnu.org Subject: Re: bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password References: <875znltof0.fsf@telefonica.net> <83lfw18hld.fsf@gnu.org> Date: Mon, 12 Aug 2019 01:45:38 +0200 In-Reply-To: <83lfw18hld.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 10 Aug 2019 13:01:18 +0300") Message-ID: <878srznu59.fsf@telefonica.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-CTCH-Score: 0.000 X-CTCH-ScoreCust: 0.000 X-TnetOut-Country: IP: 79.151.6.162 | Country: ES X-TnetOut-Information: AntiSPAM and AntiVIRUS on relayout04 X-TnetOut-MsgID: 466FwL6rDYz1065.AD4BF X-TnetOut-SpamCheck: no es spam, Unknown X-TnetOut-From: ofv@wanadoo.es X-TnetOut-Watermark: 1566171939.17675@/OAigQQhvZ0ZOZrybqdTXw X-Spam-Status: No X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 36834-done X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Fixed in f01365f62c921407acead13bb350816a313a8c42 ------------=_1565567162-3730-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at submit) by debbugs.gnu.org; 29 Jul 2019 05:12:13 +0000 Received: from localhost ([127.0.0.1]:47572 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hrxxE-0000N2-Uv for submit@debbugs.gnu.org; Mon, 29 Jul 2019 01:12:13 -0400 Received: from lists.gnu.org ([209.51.188.17]:35066) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hrxxD-0000Mv-Cl for submit@debbugs.gnu.org; Mon, 29 Jul 2019 01:12:11 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:40747) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hrxxC-00080F-8K for bug-gnu-emacs@gnu.org; Mon, 29 Jul 2019 01:12:11 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.1 required=5.0 tests=BAYES_50,RCVD_IN_DNSWL_LOW, URIBL_BLOCKED autolearn=disabled version=3.3.2 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hrxxB-0006EB-4B for bug-gnu-emacs@gnu.org; Mon, 29 Jul 2019 01:12:10 -0400 Received: from relayout03-redir.e.movistar.es ([86.109.101.203]:44747) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hrxxA-00069N-Pd for bug-gnu-emacs@gnu.org; Mon, 29 Jul 2019 01:12:09 -0400 Received: from sky (162.red-79-151-6.dynamicip.rima-tde.net [79.151.6.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: 981711563@telefonica.net) by relayout03.e.movistar.es (Postfix) with ESMTPSA id 45xnqS3WVbzMl2b for ; Mon, 29 Jul 2019 07:12:04 +0200 (CEST) From: =?utf-8?Q?=C3=93scar_Fuentes?= To: bug-gnu-emacs@gnu.org Subject: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password Date: Mon, 29 Jul 2019 07:12:03 +0200 Message-ID: <875znltof0.fsf@telefonica.net> MIME-Version: 1.0 Content-Type: text/plain X-CTCH-Score: 0.000 X-CTCH-ScoreCust: 0.000 X-TnetOut-Country: IP: 79.151.6.162 | Country: ES X-TnetOut-Information: AntiSPAM and AntiVIRUS on relayout03 X-TnetOut-MsgID: 45xnqS3WVbzMl2b.AAD0A X-TnetOut-SpamCheck: no es spam, Unknown X-TnetOut-From: ofv@wanadoo.es X-TnetOut-Watermark: 1564981925.87401@HFFts58ZxH+Mb5q9FWZa8A X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 86.109.101.203 X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) 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 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. ------------=_1565567162-3730-1--