unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [Patch] Avoid recording chars when reading passwords
@ 2022-06-08 13:46 Manuel Giraud
  2022-06-08 15:48 ` Eli Zaretskii
  0 siblings, 1 reply; 18+ messages in thread
From: Manuel Giraud @ 2022-06-08 13:46 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 81 bytes --]

Hi,

With this patch, passwords won't appear in the lossage or the dribble
file.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-recording-chars-when-reading-passwords.patch --]
[-- Type: text/x-patch, Size: 2635 bytes --]

From a15d43e2cd80307e2597f7888a4920d73fa58362 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Wed, 8 Jun 2022 15:39:39 +0200
Subject: [PATCH] Avoid recording chars when reading passwords

* lisp/subr.el (read-passwd): inhibit char recording when reading
a password.
* src/keyboard.c (syms_of_keyboard, read_char): introduce
`do-not-record-char' to inhibit char recording.
---
 lisp/subr.el   |  1 +
 src/keyboard.c | 18 +++++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 50ae357a13..1007237fb6 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3048,6 +3048,7 @@ read-passwd
             (use-local-map read-passwd-map)
             (setq-local inhibit-modification-hooks nil) ;bug#15501.
 	    (setq-local show-paren-mode nil)		;bug#16091.
+            (setq-local do-not-record-char t)
             (add-hook 'post-command-hook #'read-password--hide-password nil t))
         (unwind-protect
             (let ((enable-recursive-minibuffers t)
diff --git a/src/keyboard.c b/src/keyboard.c
index 55d710ed62..a628c2d2df 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3046,10 +3046,13 @@ read_char (int commandflag, Lisp_Object map,
 
   /* Store these characters into recent_keys, the dribble file if any,
      and the keyboard macro being defined, if any.  */
-  record_char (c);
-  recorded = true;
-  if (! NILP (also_record))
-    record_char (also_record);
+  if (!do_not_record_char)
+    {
+      record_char (c);
+      recorded = true;
+      if (! NILP (also_record))
+	record_char (also_record);
+    }
 
   /* Wipe the echo area.
      But first, if we are about to use an input method,
@@ -3172,7 +3175,7 @@ read_char (int commandflag, Lisp_Object map,
   /* When we consume events from the various unread-*-events lists, we
      bypass the code that records input, so record these events now if
      they were not recorded already.  */
-  if (!recorded)
+  if (!recorded && !do_not_record_char)
     {
       record_char (c);
       recorded = true;
@@ -12561,6 +12564,11 @@ syms_of_keyboard (void)
 \(Even if the operating system has support for stopping a process.)  */);
   cannot_suspend = false;
 
+  DEFVAR_BOOL ("do-not-record-char", do_not_record_char,
+	       doc: /* Non-nil means to not record chars.
+Those chars would not appear in the dribble file or in `view-lossage'.  */);
+  do_not_record_char = false;
+
   DEFVAR_BOOL ("menu-prompting", menu_prompting,
 	       doc: /* Non-nil means prompt with menus when appropriate.
 This is done when reading from a keymap that has a prompt string,
-- 
2.36.0


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-08 13:46 [Patch] Avoid recording chars when reading passwords Manuel Giraud
@ 2022-06-08 15:48 ` Eli Zaretskii
  2022-06-13 13:26   ` Manuel Giraud
  2022-06-14  9:38   ` Manuel Giraud
  0 siblings, 2 replies; 18+ messages in thread
From: Eli Zaretskii @ 2022-06-08 15:48 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: emacs-devel

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Date: Wed, 08 Jun 2022 15:46:19 +0200
> 
> With this patch, passwords won't appear in the lossage or the dribble
> file.

We already have a facility for this, see 'no-record' in the
description of unread-command-events in the ELisp manual.  Please see
if you can use that instead.

And if you must have a variable, I'd prefer to un-obsolete
inhibit--record-char, which was once used for this purpose, but no
longer is, and restore the code in keyboard.c which supported it.

Thanks.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-08 15:48 ` Eli Zaretskii
@ 2022-06-13 13:26   ` Manuel Giraud
  2022-06-14  9:38   ` Manuel Giraud
  1 sibling, 0 replies; 18+ messages in thread
From: Manuel Giraud @ 2022-06-13 13:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Manuel Giraud, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Date: Wed, 08 Jun 2022 15:46:19 +0200
>> 
>> With this patch, passwords won't appear in the lossage or the dribble
>> file.
>
> We already have a facility for this, see 'no-record' in the
> description of unread-command-events in the ELisp manual.  Please see
> if you can use that instead.

Ok but I don't quite understand how unread-command-events works. For
instance, if I put this code into `read-passwd':

--8<---------------cut here---------------start------------->8---
(setq-local unread-command-events
            (mapcar #'(lambda (x) (cons 'no-record x))
                    (listify-key-sequence "mypassword")))
--8<---------------cut here---------------end--------------->8---

It will prefilled (and not recorded) at prompt with "mypassord" but I'd
like to be able to type the real password that I don't know in
advance. In `read-passwd', the password is read with `read-string' that
is C code… Anyway, can someone explain what is the purpose (and usage)
of unread-command-events?

> And if you must have a variable, I'd prefer to un-obsolete
> inhibit--record-char, which was once used for this purpose, but no
> longer is, and restore the code in keyboard.c which supported it.

I was not aware of this code. But yes, maybe we should do that.

Thanks.
-- 
Manuel Giraud



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-08 15:48 ` Eli Zaretskii
  2022-06-13 13:26   ` Manuel Giraud
@ 2022-06-14  9:38   ` Manuel Giraud
  2022-06-14 11:35     ` Eli Zaretskii
  1 sibling, 1 reply; 18+ messages in thread
From: Manuel Giraud @ 2022-06-14  9:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

Hi,

So I digged into the log and found that `inhibit--record-char' was
obsoleted in bd5c740419.

The reason seems to be that "quail.el" was the last user of this
variable and use it to inhibit a second recording after the first
one. AFAIU, quail.el now have function (quail-add-unread-command-events)
that put a copy of those events with 'no-record into
unread-command-events to have this behaviour.

I was not able to do the same when reading password (there is just one
read_char involved), so as Eli suggested, I choose to un-obsolete
`inhibit--record-char' and use it in `read-passwd' only.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-recording-passwords-chars.patch --]
[-- Type: text/x-patch, Size: 2416 bytes --]

From 8bfbc312ef7cb1f8f2cfd4ea8feda99af86e48a3 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Tue, 14 Jun 2022 11:14:02 +0200
Subject: [PATCH] Avoid recording passwords' chars

* src/keyboard.c (syms_of_keyboard): un-obsolete
`inhibit--record-char'.
* lisp/subr.el (read-passwd): use `inhibit--record-char' to
inhibit passwords recording.
---
 lisp/subr.el   |  7 +------
 src/keyboard.c | 12 ++++++++++++
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 50ae357a13..29f9706c39 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1883,12 +1883,6 @@ 'inhibit-nul-byte-detection
 (make-obsolete-variable 'load-dangerous-libraries
                         "no longer used." "27.1")
 
-(defvar inhibit--record-char nil
-  "Obsolete variable.
-This was used internally by quail.el and keyboard.c in Emacs 27.
-It does nothing in Emacs 28.")
-(make-obsolete-variable 'inhibit--record-char nil "28.1")
-
 (define-obsolete-function-alias 'compare-window-configurations
   #'window-configuration-equal-p "29.1")
 
@@ -3048,6 +3042,7 @@ read-passwd
             (use-local-map read-passwd-map)
             (setq-local inhibit-modification-hooks nil) ;bug#15501.
 	    (setq-local show-paren-mode nil)		;bug#16091.
+            (setq-local inhibit--record-char t)
             (add-hook 'post-command-hook #'read-password--hide-password nil t))
         (unwind-protect
             (let ((enable-recursive-minibuffers t)
diff --git a/src/keyboard.c b/src/keyboard.c
index 55d710ed62..fbdc0e9107 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3307,6 +3307,10 @@ help_char_p (Lisp_Object c)
 static void
 record_char (Lisp_Object c)
 {
+  /* subr.el/read-passwd binds this to avoid recording passwords.  */
+  if (inhibit_record_char)
+    return;
+
   int recorded = 0;
 
   if (CONSP (c) && (EQ (XCAR (c), Qhelp_echo) || EQ (XCAR (c), Qmouse_movement)))
@@ -12992,6 +12996,14 @@ syms_of_keyboard (void)
 changed.  */);
   Vdisplay_monitors_changed_functions = Qnil;
 
+  DEFVAR_BOOL ("inhibit--record-char",
+	       inhibit_record_char,
+	       doc: /* If non-nil, don't record input events.
+This inhibits recording input events for the purposes of keyboard
+macros, dribble file, and `recent-keys'.
+Internal use only.  */);
+  inhibit_record_char = false;
+
   pdumper_do_now_and_after_load (syms_of_keyboard_for_pdumper);
 }
 
-- 
2.36.1


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-14  9:38   ` Manuel Giraud
@ 2022-06-14 11:35     ` Eli Zaretskii
  2022-06-14 12:34       ` Stefan Monnier
  0 siblings, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2022-06-14 11:35 UTC (permalink / raw)
  To: Manuel Giraud, Stefan Monnier; +Cc: emacs-devel

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: emacs-devel@gnu.org
> Date: Tue, 14 Jun 2022 11:38:00 +0200
> 
> So I digged into the log and found that `inhibit--record-char' was
> obsoleted in bd5c740419.
> 
> The reason seems to be that "quail.el" was the last user of this
> variable and use it to inhibit a second recording after the first
> one. AFAIU, quail.el now have function (quail-add-unread-command-events)
> that put a copy of those events with 'no-record into
> unread-command-events to have this behaviour.
> 
> I was not able to do the same when reading password (there is just one
> read_char involved), so as Eli suggested, I choose to un-obsolete
> `inhibit--record-char' and use it in `read-passwd' only.

Thanks.

Stefan, do you see any way of producing (no-record . CH) from "normal"
input APIs, for reading input that shouldn't be echoed or recorded?

If not, we will resurrect that variable and use it instead.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-14 11:35     ` Eli Zaretskii
@ 2022-06-14 12:34       ` Stefan Monnier
  2022-06-14 17:46         ` Eli Zaretskii
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2022-06-14 12:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Manuel Giraud, emacs-devel

Eli Zaretskii [2022-06-14 14:35:37] wrote:
> Stefan, do you see any way of producing (no-record . CH) from "normal"
> input APIs, for reading input that shouldn't be echoed or recorded?

No: the (no-record . CH) is specific to `unread-command-events`, i.e. to
events that are "synthesized" (typically from previously read events)
whereas those events that come from the keyboard get added directly to
the lossage buffer before we have a chance to do anything about it.

> If not, we will resurrect that variable and use it instead.

Sounds good to me.


        Stefan




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-14 12:34       ` Stefan Monnier
@ 2022-06-14 17:46         ` Eli Zaretskii
  2022-06-15  7:59           ` Manuel Giraud
  2022-06-15 12:27           ` Stefan Monnier
  0 siblings, 2 replies; 18+ messages in thread
From: Eli Zaretskii @ 2022-06-14 17:46 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: manuel, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Manuel Giraud <manuel@ledu-giraud.fr>,  emacs-devel@gnu.org
> Date: Tue, 14 Jun 2022 08:34:22 -0400
> 
> > If not, we will resurrect that variable and use it instead.
> 
> Sounds good to me.

OK, thanks.

The next question is: do we want to enable this by default and in a
way that users cannot have the previous behavior back?

In any case, this needs a NEWS entry, I think.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-14 17:46         ` Eli Zaretskii
@ 2022-06-15  7:59           ` Manuel Giraud
  2022-06-18 11:16             ` Eli Zaretskii
  2022-06-15 12:27           ` Stefan Monnier
  1 sibling, 1 reply; 18+ messages in thread
From: Manuel Giraud @ 2022-06-15  7:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Stefan Monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: Manuel Giraud <manuel@ledu-giraud.fr>,  emacs-devel@gnu.org
>> Date: Tue, 14 Jun 2022 08:34:22 -0400
>> 
>> > If not, we will resurrect that variable and use it instead.
>> 
>> Sounds good to me.
>
> OK, thanks.
>
> The next question is: do we want to enable this by default and in a
> way that users cannot have the previous behavior back?

I think that having this as a default is a good thing™ (no more clear
password in lossage).  About being able to revert this behaviour, I know
I won't use it but maybe it has some use case with the dribble file: do
people use the dribble file as some automation tool for example?

> In any case, this needs a NEWS entry, I think.

Ok, I'd one to this patch when we have decide the default and if it
needs a user option.
-- 
Manuel Giraud



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-14 17:46         ` Eli Zaretskii
  2022-06-15  7:59           ` Manuel Giraud
@ 2022-06-15 12:27           ` Stefan Monnier
  2022-06-15 13:27             ` Eli Zaretskii
  1 sibling, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2022-06-15 12:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: manuel, emacs-devel

> The next question is: do we want to enable this by default

I'm strongly in favor of it being enabled by default, yes.
I think most people expect that their passwords aren't trivially
available in clear from the lossage.

> and in a way that users cannot have the previous behavior back?

I'd think being able to redefine `read-password` is sufficient, because
it should really be extremely uncommon to need your password to appear
in lossage and/or a dribble file.


        Stefan




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-15 12:27           ` Stefan Monnier
@ 2022-06-15 13:27             ` Eli Zaretskii
  0 siblings, 0 replies; 18+ messages in thread
From: Eli Zaretskii @ 2022-06-15 13:27 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: manuel, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: manuel@ledu-giraud.fr,  emacs-devel@gnu.org
> Date: Wed, 15 Jun 2022 08:27:51 -0400
> 
> > and in a way that users cannot have the previous behavior back?
> 
> I'd think being able to redefine `read-password` is sufficient, because
> it should really be extremely uncommon to need your password to appear
> in lossage and/or a dribble file.

Dribble file is a debug facility, and quite an important one at that.
Lossage is also a troubleshooting facility, at least in some of its
uses.

So asking that people redefine functions for that is too harsh,
IMNSHO.  What's the harm from having a variable that people could flip
if they need it?



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-15  7:59           ` Manuel Giraud
@ 2022-06-18 11:16             ` Eli Zaretskii
  2022-06-18 11:29               ` Akib Azmain Turja
  0 siblings, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2022-06-18 11:16 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: monnier, emacs-devel

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  emacs-devel@gnu.org
> Date: Wed, 15 Jun 2022 09:59:31 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > The next question is: do we want to enable this by default and in a
> > way that users cannot have the previous behavior back?
> 
> I think that having this as a default is a good thing™ (no more clear
> password in lossage).  About being able to revert this behaviour, I know
> I won't use it but maybe it has some use case with the dribble file: do
> people use the dribble file as some automation tool for example?
> 
> > In any case, this needs a NEWS entry, I think.
> 
> Ok, I'd one to this patch when we have decide the default and if it
> needs a user option.

Sounds like having this on by default is what people want.  But we
need a knob to revert to the previous behavior, and we need a NEWS
entry.  Could you please prepare an updated patch with those two
additions?

Thanks.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-18 11:16             ` Eli Zaretskii
@ 2022-06-18 11:29               ` Akib Azmain Turja
  2022-06-18 11:33                 ` Akib Azmain Turja
  0 siblings, 1 reply; 18+ messages in thread
From: Akib Azmain Turja @ 2022-06-18 11:29 UTC (permalink / raw)
  To: Eli Zaretskii, Manuel Giraud; +Cc: monnier, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  emacs-devel@gnu.org
>> Date: Wed, 15 Jun 2022 09:59:31 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > The next question is: do we want to enable this by default and in a
>> > way that users cannot have the previous behavior back?
>> 
>> I think that having this as a default is a good thing™ (no more clear
>> password in lossage).  About being able to revert this behaviour, I know
>> I won't use it but maybe it has some use case with the dribble file: do
>> people use the dribble file as some automation tool for example?
>> 
>> > In any case, this needs a NEWS entry, I think.
>> 
>> Ok, I'd one to this patch when we have decide the default and if it
>> needs a user option.
>
> Sounds like having this on by default is what people want.  But we
> need a knob to revert to the previous behavior, and we need a NEWS
> entry.  Could you please prepare an updated patch with those two
> additions?
>
> Thanks.
>

Yes, there should be an option to revert it, otherwise it'll break my
"workflow"[1].

[1]: https://xkcd.com/1172/

-- 
Akib Azmain Turja

This message is signed by me with my GnuPG key.  It's fingerprint is:

    7001 8CE5 819F 17A3 BBA6  66AF E74F 0EFA 922A E7F5

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-18 11:29               ` Akib Azmain Turja
@ 2022-06-18 11:33                 ` Akib Azmain Turja
  2022-06-20  9:58                   ` Manuel Giraud
  0 siblings, 1 reply; 18+ messages in thread
From: Akib Azmain Turja @ 2022-06-18 11:33 UTC (permalink / raw)
  To: Eli Zaretskii, Manuel Giraud; +Cc: monnier, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1774 bytes --]

Akib Azmain Turja <akib@disroot.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  emacs-devel@gnu.org
>>> Date: Wed, 15 Jun 2022 09:59:31 +0200
>>> 
>>> Eli Zaretskii <eliz@gnu.org> writes:
>>> 
>>> > The next question is: do we want to enable this by default and in a
>>> > way that users cannot have the previous behavior back?
>>> 
>>> I think that having this as a default is a good thing™ (no more clear
>>> password in lossage).  About being able to revert this behaviour, I know
>>> I won't use it but maybe it has some use case with the dribble file: do
>>> people use the dribble file as some automation tool for example?
>>> 
>>> > In any case, this needs a NEWS entry, I think.
>>> 
>>> Ok, I'd one to this patch when we have decide the default and if it
>>> needs a user option.
>>
>> Sounds like having this on by default is what people want.  But we
>> need a knob to revert to the previous behavior, and we need a NEWS
>> entry.  Could you please prepare an updated patch with those two
>> additions?
>>
>> Thanks.
>>
>
> Yes, there should be an option to revert it, otherwise it'll break my
> "workflow"[1].
>
> [1]: https://xkcd.com/1172/
>
> -- 
> Akib Azmain Turja
>
> This message is signed by me with my GnuPG key.  It's fingerprint is:
>
>     7001 8CE5 819F 17A3 BBA6  66AF E74F 0EFA 922A E7F5

And I think it would be useful to record non self-insert keys (like
"M-x") while typing password, since those key aren't related to the
password.

-- 
Akib Azmain Turja

This message is signed by me with my GnuPG key.  It's fingerprint is:

    7001 8CE5 819F 17A3 BBA6  66AF E74F 0EFA 922A E7F5

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-18 11:33                 ` Akib Azmain Turja
@ 2022-06-20  9:58                   ` Manuel Giraud
  2022-06-20 11:28                     ` Eli Zaretskii
  0 siblings, 1 reply; 18+ messages in thread
From: Manuel Giraud @ 2022-06-20  9:58 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Eli Zaretskii, monnier, emacs-devel

Akib Azmain Turja <akib@disroot.org> writes:

[...]

> And I think it would be useful to record non self-insert keys (like
> "M-x") while typing password, since those key aren't related to the
> password.

Hi,

I've just tried and this patch does just that: you can "M-x something"
in the middle of entering a password and the execute-extended-command
will be recorded but not the password.

Eli, Stefan, should I try to have a custom to revert this back?  What
would be the name of such custom?  record-password?
-- 
Manuel Giraud



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-20  9:58                   ` Manuel Giraud
@ 2022-06-20 11:28                     ` Eli Zaretskii
  2022-06-20 15:28                       ` Manuel Giraud
  0 siblings, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2022-06-20 11:28 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: akib, monnier, emacs-devel

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: Eli Zaretskii <eliz@gnu.org>,  monnier@iro.umontreal.ca,
>  emacs-devel@gnu.org
> Date: Mon, 20 Jun 2022 11:58:30 +0200
> 
> Eli, Stefan, should I try to have a custom to revert this back?

Yes.  And a NEWS entry, please.

> What would be the name of such custom?  record-password?

I think it would be better to have a more general name, in case we
would want to exempt more stuff from being recorded.  Something like
record-all-keys, perhaps (with an explanation in the doc string that
currently it only affects password entry)?



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-20 11:28                     ` Eli Zaretskii
@ 2022-06-20 15:28                       ` Manuel Giraud
  2022-06-20 15:52                         ` Eli Zaretskii
  0 siblings, 1 reply; 18+ messages in thread
From: Manuel Giraud @ 2022-06-20 15:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: akib, monnier, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 347 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

[...]

> I think it would be better to have a more general name, in case we
> would want to exempt more stuff from being recorded.  Something like
> record-all-keys, perhaps (with an explanation in the doc string that
> currently it only affects password entry)?

Ok. Here is an updated version of the patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-recording-passwords-chars.patch --]
[-- Type: text/x-patch, Size: 3909 bytes --]

From 06ead0deb3faa0b2cb9124c2c68966d9d3e3d015 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Tue, 14 Jun 2022 11:14:02 +0200
Subject: [PATCH] Avoid recording passwords' chars

* lisp/cus-start.el (standard): New user custom `record-all-keys'.
* src/keyboard.c (syms_of_keyboard): Un-obsolete
`inhibit--record-char'.
* lisp/subr.el (read-passwd): Use `inhibit--record-char' to
inhibit passwords recording.
---
 etc/NEWS          |  6 ++++++
 lisp/cus-start.el |  1 +
 lisp/subr.el      |  7 +------
 src/keyboard.c    | 18 ++++++++++++++++++
 4 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 1b8560a923..bee92b62f8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -365,6 +365,12 @@ This inhibits putting empty strings onto the kill ring.
 These options allow adjusting point and scrolling a window when
 dragging items from another program.
 
++++
+** New user option 'record-all-keys'.
+If non-nil, this option will force recording of all input keys
+including in passwords prompt (this was the previous behaviour).  It
+now defaults to NIL and inhibits recording of passwords.
+
 +++
 ** New function 'command-query'.
 This function makes its argument command prompt the user for
diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index d8c4b48035..ca2fca4eb7 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -398,6 +398,7 @@ minibuffer-prompt-properties--setter
 	     ;;    			(const :tag " current dir" nil)
 	     ;;    			(directory :format "%v"))))
 	     (load-prefer-newer lisp boolean "24.4")
+             (record-all-keys keyboard boolean)
 	     ;; minibuf.c
 	     (minibuffer-follows-selected-frame
               minibuffer (choice (const :tag "Always" t)
diff --git a/lisp/subr.el b/lisp/subr.el
index 50ae357a13..29f9706c39 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1883,12 +1883,6 @@ 'inhibit-nul-byte-detection
 (make-obsolete-variable 'load-dangerous-libraries
                         "no longer used." "27.1")
 
-(defvar inhibit--record-char nil
-  "Obsolete variable.
-This was used internally by quail.el and keyboard.c in Emacs 27.
-It does nothing in Emacs 28.")
-(make-obsolete-variable 'inhibit--record-char nil "28.1")
-
 (define-obsolete-function-alias 'compare-window-configurations
   #'window-configuration-equal-p "29.1")
 
@@ -3048,6 +3042,7 @@ read-passwd
             (use-local-map read-passwd-map)
             (setq-local inhibit-modification-hooks nil) ;bug#15501.
 	    (setq-local show-paren-mode nil)		;bug#16091.
+            (setq-local inhibit--record-char t)
             (add-hook 'post-command-hook #'read-password--hide-password nil t))
         (unwind-protect
             (let ((enable-recursive-minibuffers t)
diff --git a/src/keyboard.c b/src/keyboard.c
index 55d710ed62..51a424e61f 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3307,6 +3307,11 @@ help_char_p (Lisp_Object c)
 static void
 record_char (Lisp_Object c)
 {
+  /* subr.el/read-passwd binds inhibit_record_char to avoid recording
+     passwords.  */
+  if (!record_all_keys && inhibit_record_char)
+    return;
+
   int recorded = 0;
 
   if (CONSP (c) && (EQ (XCAR (c), Qhelp_echo) || EQ (XCAR (c), Qmouse_movement)))
@@ -12992,6 +12997,19 @@ syms_of_keyboard (void)
 changed.  */);
   Vdisplay_monitors_changed_functions = Qnil;
 
+  DEFVAR_BOOL ("inhibit--record-char",
+	       inhibit_record_char,
+	       doc: /* If non-nil, don't record input events.
+This inhibits recording input events for the purposes of keyboard
+macros, dribble file, and `recent-keys'.
+Internal use only.  */);
+  inhibit_record_char = false;
+
+  DEFVAR_BOOL ("record-all-keys", record_all_keys,
+	       doc: /* Non-nil means to record all typed keys.  When
+nil, only passwords' keys are not recorded.  */);
+  record_all_keys = false;
+
   pdumper_do_now_and_after_load (syms_of_keyboard_for_pdumper);
 }
 
-- 
2.36.1


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-20 15:28                       ` Manuel Giraud
@ 2022-06-20 15:52                         ` Eli Zaretskii
  2022-06-25  9:35                           ` Eli Zaretskii
  0 siblings, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2022-06-20 15:52 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: akib, monnier, emacs-devel

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: akib@disroot.org,  monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Mon, 20 Jun 2022 17:28:05 +0200
> 
> Ok. Here is an updated version of the patch.

Thanks.  Two minor nits (but you don't have to submit another patch,
this will be fixed when installing):

> +** New user option 'record-all-keys'.
> +If non-nil, this option will force recording of all input keys
> +including in passwords prompt (this was the previous behaviour).  It
> +now defaults to NIL and inhibits recording of passwords.
                   ^^^
We use 'nil' (lower-case, without the quotes).  Also, the "now" part
is redundant.

> +  DEFVAR_BOOL ("record-all-keys", record_all_keys,
> +	       doc: /* Non-nil means to record all typed keys.  When
> +nil, only passwords' keys are not recorded.  */);

The first line of a doc string should be a single complete sentence
(for the benefit of help commands, like 'apropos', which only show the
first lines).



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Patch] Avoid recording chars when reading passwords
  2022-06-20 15:52                         ` Eli Zaretskii
@ 2022-06-25  9:35                           ` Eli Zaretskii
  0 siblings, 0 replies; 18+ messages in thread
From: Eli Zaretskii @ 2022-06-25  9:35 UTC (permalink / raw)
  To: manuel; +Cc: akib, monnier, emacs-devel

> Date: Mon, 20 Jun 2022 18:52:17 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: akib@disroot.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > From: Manuel Giraud <manuel@ledu-giraud.fr>
> > Cc: akib@disroot.org,  monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> > Date: Mon, 20 Jun 2022 17:28:05 +0200
> > 
> > Ok. Here is an updated version of the patch.
> 
> Thanks.  Two minor nits (but you don't have to submit another patch,
> this will be fixed when installing):

Now installed on master, thanks.



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-06-25  9:35 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-08 13:46 [Patch] Avoid recording chars when reading passwords Manuel Giraud
2022-06-08 15:48 ` Eli Zaretskii
2022-06-13 13:26   ` Manuel Giraud
2022-06-14  9:38   ` Manuel Giraud
2022-06-14 11:35     ` Eli Zaretskii
2022-06-14 12:34       ` Stefan Monnier
2022-06-14 17:46         ` Eli Zaretskii
2022-06-15  7:59           ` Manuel Giraud
2022-06-18 11:16             ` Eli Zaretskii
2022-06-18 11:29               ` Akib Azmain Turja
2022-06-18 11:33                 ` Akib Azmain Turja
2022-06-20  9:58                   ` Manuel Giraud
2022-06-20 11:28                     ` Eli Zaretskii
2022-06-20 15:28                       ` Manuel Giraud
2022-06-20 15:52                         ` Eli Zaretskii
2022-06-25  9:35                           ` Eli Zaretskii
2022-06-15 12:27           ` Stefan Monnier
2022-06-15 13:27             ` Eli Zaretskii

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).