bug#22027: 25.1.50; read-passwd fail to return default password

bug#22027: 25.1.50; read-passwd fail to return default password

[Tino Calancha]

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


* lisp/subr.el (read-passwd):
read-passwd returns string consisting of (length password)
NULL characters, when:
*) first argument non-nil,
*) second argument string != "",
*) user accepts the default password.


emacs -Q:

(read-passwd "pass: " t "my-pass") RET RET
"--1468197610-674703064-1448617983=:26866
Content-Type: text/plain; charset=US-ASCII; name=subr.patch
Content-Transfer-Encoding: BASE64
Content-ID: <alpine.LRH.2.20.1511271853030.26866@calancha-ilc.kek.jp>
Content-Description: 
Content-Disposition: attachment; filename=subr.patch

KioqIGxpc3Avc3Vici5lbAkgICAgICAgIDIwMTUtMTEtMTggMTE6MDI6MzMu
NzAzMTMwODUwICswOTAwDQotLS0gbGlzcC9zdWJyX3BhdGNoZWQuZWwJMjAx
NS0xMS0yNyAxODo0MTowMy41MTAwMzQ3NzkgKzA5MDANCioqKioqKioqKioq
KioqKg0KKioqIDIxMTQsMjEyMSAqKioqDQogICAgICAgICAgICAgICAgICAo
c2Vjb25kIChyZWFkLXBhc3N3ZCAiQ29uZmlybSBwYXNzd29yZDogIiBuaWwg
ZGVmYXVsdCkpKQ0KICAgICAgICAgICAgICAoaWYgKGVxdWFsIGZpcnN0IHNl
Y29uZCkNCiAgICAgICAgICAgICAgICAgIChwcm9nbg0KISAgICAgICAgICAg
ICAgICAgICAoYW5kIChhcnJheXAgc2Vjb25kKSAoY2xlYXItc3RyaW5nIHNl
Y29uZCkpDQohICAgICAgICAgICAgICAgICAgIChzZXRxIHN1Y2Nlc3MgZmly
c3QpKQ0KICAgICAgICAgICAgICAgIChhbmQgKGFycmF5cCBmaXJzdCkgKGNs
ZWFyLXN0cmluZyBmaXJzdCkpDQogICAgICAgICAgICAgICAgKGFuZCAoYXJy
YXlwIHNlY29uZCkgKGNsZWFyLXN0cmluZyBzZWNvbmQpKQ0KICAgICAgICAg
ICAgICAgIChtZXNzYWdlICJQYXNzd29yZCBub3QgcmVwZWF0ZWQgYWNjdXJh
dGVseTsgcGxlYXNlIHN0YXJ0IG92ZXIiKQ0KLS0tIDIxMTQsMjEyMSAtLS0t
DQogICAgICAgICAgICAgICAgICAoc2Vjb25kIChyZWFkLXBhc3N3ZCAiQ29u
ZmlybSBwYXNzd29yZDogIiBuaWwgZGVmYXVsdCkpKQ0KICAgICAgICAgICAg
ICAoaWYgKGVxdWFsIGZpcnN0IHNlY29uZCkNCiAgICAgICAgICAgICAgICAg
IChwcm9nbg0KISAJCSAgKHNldHEgc3VjY2VzcyAoYXBwbHkgJ3N0cmluZyAo
YXBwZW5kIGZpcnN0IG5pbCkpKQ0KISAgICAgICAgICAgICAgICAgICAoYW5k
IChhcnJheXAgc2Vjb25kKSAoY2xlYXItc3RyaW5nIHNlY29uZCkpKQ0KICAg
ICAgICAgICAgICAgIChhbmQgKGFycmF5cCBmaXJzdCkgKGNsZWFyLXN0cmlu
ZyBmaXJzdCkpDQogICAgICAgICAgICAgICAgKGFuZCAoYXJyYXlwIHNlY29u
ZCkgKGNsZWFyLXN0cmluZyBzZWNvbmQpKQ0KICAgICAgICAgICAgICAgICht
ZXNzYWdlICJQYXNzd29yZCBub3QgcmVwZWF0ZWQgYWNjdXJhdGVseTsgcGxl
YXNlIHN0YXJ0IG92ZXIiKQ0K

bug#22027: Acknowledgement (25.1.50; read-passwd fail to return default password)

[Constantino Calancha]

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



[-- Attachment #2: subr.patch --]
[-- Type: application/octet-stream, Size: 1077 bytes --]

*** lisp/subr.el	        2015-11-18 11:02:33.703130850 +0900
--- lisp/subr_patched.el	2015-11-27 18:41:03.510034779 +0900
***************
*** 2114,2121 ****
                  (second (read-passwd "Confirm password: " nil default)))
              (if (equal first second)
                  (progn
!                   (and (arrayp second) (clear-string second))
!                   (setq success first))
                (and (arrayp first) (clear-string first))
                (and (arrayp second) (clear-string second))
                (message "Password not repeated accurately; please start over")
--- 2114,2121 ----
                  (second (read-passwd "Confirm password: " nil default)))
              (if (equal first second)
                  (progn
! 		  (setq success (apply 'string (append first nil)))
!                   (and (arrayp second) (clear-string second)))
                (and (arrayp first) (clear-string first))
                (and (arrayp second) (clear-string second))
                (message "Password not repeated accurately; please start over")

bug#22027: 25.1.50; Return uncorrupted default string

[Tino Calancha]

>*) first argument non-nil,
>*) second argument string != "",
Should read:
*) second argument non-nil,
*) third argument string != "",



In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 2.24.30)
  of 2016-05-06 built on calancha-pc
Repository revision: 80bea210ff14a64daa1d71765983aa3baa149555


From 1ced98ba1a548749c119a93858f82f1210fe81bc Mon Sep 17 00:00:00 2001
From: Tino Calancha <f92capac@gmail.com>
Date: Mon, 9 May 2016 12:01:31 +0900
Subject: [PATCH 1/3] Return uncorrupted default string

* lisp/subr.el (read-passwd): Return DEFAULT when
CONFIRM evaluates non-nil and user inputs the empty string
(Bug#22027).
---
  lisp/subr.el | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 0fa6404..ef6af1d 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2152,9 +2152,9 @@ read-passwd
            (let ((first (read-passwd prompt nil default))
                  (second (read-passwd "Confirm password: " nil default)))
              (if (equal first second)
-                (progn
-                  (and (arrayp second) (clear-string second))
-                  (setq success first))
+                (prog1
+                  (setq success (copy-sequence first))
+                  (and (arrayp second) (clear-string second)))
                (and (arrayp first) (clear-string first))
                (and (arrayp second) (clear-string second))
                (message "Password not repeated accurately; please start over")
-- 
2.8.1

From 7124a681e17e4f33694e9f1c8ccb0261f6a0bf9c Mon Sep 17 00:00:00 2001
From: Tino Calancha <f92capac@gmail.com>
Date: Mon, 9 May 2016 12:03:18 +0900
Subject: [PATCH 2/3] * lisp/subr.el (read-passwd): Clear also first password.

---
  lisp/subr.el | 1 +
  1 file changed, 1 insertion(+)

diff --git a/lisp/subr.el b/lisp/subr.el
index ef6af1d..5c32299 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2154,6 +2154,7 @@ read-passwd
              (if (equal first second)
                  (prog1
                    (setq success (copy-sequence first))
+                  (and (arrayp first) (clear-string first))
                    (and (arrayp second) (clear-string second)))
                (and (arrayp first) (clear-string first))
                (and (arrayp second) (clear-string second))
-- 
2.8.1

From e3aa5b5d30b901c2533538b3a9d0d5bd69493cf0 Mon Sep 17 00:00:00 2001
From: Tino Calancha <f92capac@gmail.com>
Date: Mon, 9 May 2016 12:08:02 +0900
Subject: [PATCH 3/3] * lisp/subr.el (read-passwd): Clear non-empty passwords.

---
  lisp/subr.el | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 5c32299..344c416 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2153,11 +2153,11 @@ read-passwd
                  (second (read-passwd "Confirm password: " nil default)))
              (if (equal first second)
                  (prog1
-                  (setq success (copy-sequence first))
-                  (and (arrayp first) (clear-string first))
-                  (and (arrayp second) (clear-string second)))
-              (and (arrayp first) (clear-string first))
-              (and (arrayp second) (clear-string second))
+                    (setq success (copy-sequence first))
+                  (when (and (arrayp first)
+                             (not (string= first "")))
+                    (clear-string first)
+                    (clear-string second)))
                (message "Password not repeated accurately; please start over")
                (sit-for 1))))
          success)
-- 
2.8.1

bug#22027: 25.1.50; Return uncorrupted default string

[npostavs]

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

Tino Calancha <f92capac@gmail.com> writes:

> +                    (setq success (copy-sequence first))
> +                  (when (and (arrayp first)
> +                             (not (string= first "")))
> +                    (clear-string first)
> +                    (clear-string second)))

I think that is too complicated, how about this instead:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 960 bytes --]

From 739ab77ca99a57f10088aa8a9f2c079c1727a071 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 3 Jun 2017 23:41:53 -0400
Subject: [PATCH v1] * lisp/subr.el (read-passwd): Don't delete return value
 (Bug#22027).

---
 lisp/subr.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 9b3e53b70b..76c13a8d23 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2268,7 +2268,7 @@ (defun read-passwd (prompt &optional confirm default)
                 (second (read-passwd "Confirm password: " nil default)))
             (if (equal first second)
                 (progn
-                  (and (arrayp second) (clear-string second))
+                  (and (arrayp second) (not (eq first second)) (clear-string second))
                   (setq success first))
               (and (arrayp first) (clear-string first))
               (and (arrayp second) (clear-string second))
-- 
2.11.1

bug#22027: 25.1.50; Return uncorrupted default string

[Tino Calancha]

On Sat, 3 Jun 2017, npostavs@users.sourceforge.net wrote:

> Tino Calancha <f92capac@gmail.com> writes:
>
>> +                    (setq success (copy-sequence first))
>> +                  (when (and (arrayp first)
>> +                             (not (string= first "")))
>> +                    (clear-string first)
>> +                    (clear-string second)))
>
> I think that is too complicated, how about this instead:
Yes, your is better.  Thank you.
We might add a test as well:

commit eb7fdeefadf08d7a781bee60df45fb045b582930
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Mon Jun 5 09:18:55 2017 +0900

     * test/lisp/subr-tests.el (subr-tests-bug22027): Add test.

diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el
index c0bfd40f80..81f61220d3 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -28,6 +28,7 @@
  ;;; Code:

  (require 'ert)
+(eval-when-compile (require 'cl-lib))

  (ert-deftest let-when-compile ()
    ;; good case
@@ -316,5 +317,13 @@ subr-tests--this-file
    (should-not (method-files 'subr-tests--undefined-generic))
    (should-not (method-files 'subr-tests--generic-without-methods)))

+(ert-deftest subr-tests-bug22027 ()
+  "Test for http://debbugs.gnu.org/22027 ."
+  (cl-letf* ((default "foo")
+             ((symbol-function 'read-string)
+              (lambda (&rest args) (mapconcat #'string default ""))))
+    (should (string= default (read-passwd "pass: " 'confirm default)))))
+
+
  (provide 'subr-tests)
  ;;; subr-tests.el ends here

bug#22027: 25.1.50; Return uncorrupted default string

[Tino Calancha]

On Mon, 5 Jun 2017, Tino Calancha wrote:

> We might add a test as well:
but my previous test is wrong: it works before we apply any patch :-)
The following looks right:

commit 3b255c1b04a87442fc8293323f2c827dd44b038a
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Mon Jun 5 09:35:29 2017 +0900

     * test/lisp/subr-tests.el (subr-tests-bug22027): Add test.

diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el
index c0bfd40f80..54f4ab5d1b 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -28,6 +28,7 @@
  ;;; Code:

  (require 'ert)
+(eval-when-compile (require 'cl-lib))

  (ert-deftest let-when-compile ()
    ;; good case
@@ -316,5 +317,14 @@ subr-tests--this-file
    (should-not (method-files 'subr-tests--undefined-generic))
    (should-not (method-files 'subr-tests--generic-without-methods)))

+(ert-deftest subr-tests-bug22027 ()
+  "Test for http://debbugs.gnu.org/22027 ."
+  (let ((default "foo") res)
+    (cl-letf (((symbol-function 'read-string)
+               (lambda (_prompt _init _hist def) def)))
+      (setq res (read-passwd "pass: " 'confirm (mapconcat #'string default "")))
+      (should (string= default res)))))
+
+
  (provide 'subr-tests)
  ;;; subr-tests.el ends here

bug#22027: 25.1.50; Return uncorrupted default string

[npostavs]

tags 22027 fixed
close 22027 26.1
quit

Tino Calancha <f92capac@gmail.com> writes:

>> We might add a test as well:
> but my previous test is wrong: it works before we apply any patch :-)
> The following looks right:

Thanks, pushed to master [1: 695d50fa0a], [2: 882f81fdb4].

[1: 695d50fa0a]: 2017-06-05 18:57:50 -0400
  * test/lisp/subr-tests.el (subr-tests-bug22027): Add test.
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=695d50fa0a11c6bd98f9f4a9b5a2905d33282316

[2: 882f81fdb4]: 2017-06-05 18:57:32 -0400
  * lisp/subr.el (read-passwd): Don't delete return value (Bug#22027).
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=882f81fdb4ffba773af775d6d6f8920f23163e72