bug#64089: 30.0.50; `ldap-search' errors out with `wrong-type-argument listp' when called WITHDN == t

[Jens Schmidt via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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


On 2023-06-18  10:56, Eli Zaretskii wrote:

> I'm uncomfortable with non-trivial changes, so if this is anywhere 
> like Filipp's proposal, I'd rather leave it unfixed on emacs-29.

So here are patch

   0001-Undo-suboptimal-fix-for-dn-line-parsing.patch

to back out my initial fix, plus *alternative* patches

   0002-filipp-Do-not-process-dn-entry-with-ldap-decode-attribute.patch

   0002-jschmidt-Do-not-process-dn-entry-with-ldap-decode-attribute.patch

Both fix this issue while keeping complete API stability.  Filipps is a
bit shorter, but IMHO harder to read plus slightly less efficient on
runtime, mine is a bit longer since it (partially) duplicates code.

Both give the same results, in particular independently of the setting
of `ldap-ignore-attribute-codings':

   (ldap-search "(uid=jeschmid)"
                "ldap://ldap.company.com"
                '("mail"))
   => ((("mail" "jens.schmidt@company.com")))

   (let ((ldap-ignore-attribute-codings t))
     (ldap-search "(uid=jeschmid)"
                  "ldap://ldap.company.com"
                  '("mail")
                  nil
                  t)
   => (("dn: cn=JENS_SCHMIDT,L=REGION,DC=COMPANY,DC=COM"
        ("mail" "jens.schmidt@company.com")))

   (let ((ldap-ignore-attribute-codings nil))
     (ldap-search "(uid=jeschmid)"
                  "ldap://ldap.company.com"
                  '("mail")
                  nil
                  t)
   => (("dn: cn=JENS_SCHMIDT,L=REGION,DC=COMPANY,DC=COM"
        ("mail" "jens.schmidt@company.com")))

Feel free to apply whatever you think appropriate.  Please on *emacs-29
only* (I haven't found how to mark that).  Then please close this bug.

Thanks

[-- Attachment #2: 0001-Undo-suboptimal-fix-for-dn-line-parsing.patch --]
[-- Type: text/x-patch, Size: 1667 bytes --]

From 24a8481b09411a1d4295c6ef7d5488a4e353f424 Mon Sep 17 00:00:00 2001
From: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
Date: Sun, 18 Jun 2023 11:42:35 +0200
Subject: [PATCH 1/2] Undo suboptimal fix for dn line parsing

The previous fix for `ldap-search' erroring out with
`wrong-type-argument listp' does not provide API stability in all
cases.
* lisp/net/ldap.el (ldap-search-internal): Back out that fix.
(Bug#64089)
---
 lisp/net/ldap.el | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/lisp/net/ldap.el b/lisp/net/ldap.el
index 8897c3b6d54..78405414a28 100644
--- a/lisp/net/ldap.el
+++ b/lisp/net/ldap.el
@@ -703,17 +703,7 @@ ldap-search-internal
 	(while (progn
 		 (skip-chars-forward " \t\n")
 		 (not (eobp)))
-          ;; Ignore first (dn) line if WITHDN equals nil.  If WITHDN
-          ;; is non-nil, check syntax of the line and split it into a
-          ;; pair as expected by `ldap-decode-attribute' (Bug#64089).
-          ;; If the syntax is wrong, better throw an error here, since
-          ;; otherwise `ldap-decode-attribute' would throw a much less
-          ;; comprehensible error later.
-          (cond ((not withdn))
-                ((looking-at "dn[=:\t ]+\\(.*\\)$")
-                 (setq dn (list "dn" (match-string 1))))
-                (t (error "Incorrect dn line \"%s\" in ldapsearch result"
-                          (buffer-substring (point) (line-end-position)))))
+          (setq dn (buffer-substring (point) (line-end-position)))
 	  (forward-line 1)
           (while (looking-at "^\\([A-Za-z][-A-Za-z0-9]*\
 \\|[0-9]+\\(?:\\.[0-9]+\\)*\\)\\(;[-A-Za-z0-9]+\\)*[=:\t ]+\
-- 
2.30.2


[-- Attachment #3: 0002-filipp-Do-not-process-dn-entry-with-ldap-decode-attribute.patch --]
[-- Type: text/x-patch, Size: 1318 bytes --]

From 02165cdaf880787b99a0253aec11e072b049c3ef Mon Sep 17 00:00:00 2001
From: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
Date: Sun, 18 Jun 2023 12:10:29 +0200
Subject: [PATCH 2/2] Do not process dn entry with `ldap-decode-attribute'

Function `ldap-search' errors out with `wrong-type-argument listp'
when called with WITHDN non-nil.
* lisp/net/ldap.el (ldap-search): Do not process first element of
RECORD with `ldap-decode-attribute' if WITHDN is non-nil.  (Bug#64089)
---
 lisp/net/ldap.el | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lisp/net/ldap.el b/lisp/net/ldap.el
index 78405414a28..5f42fd6ac2c 100644
--- a/lisp/net/ldap.el
+++ b/lisp/net/ldap.el
@@ -487,8 +487,13 @@ ldap-search
     (if ldap-ignore-attribute-codings
 	result
       (mapcar (lambda (record)
-		(mapcar #'ldap-decode-attribute record))
-	      result))))
+                ;; Do not process first element of RECORD with
+                ;; `ldap-decode-attribute' if WITHDN is non-nil.
+                ;; (Bug#64089)
+                (append (and withdn (list (car record)))
+                        (mapcar #'ldap-decode-attribute
+                                (if withdn (cdr record) record))))
+              result))))
 
 (defun ldap-password-read (host)
   "Read LDAP password for HOST.
-- 
2.30.2


[-- Attachment #4: 0002-jschmidt-Do-not-process-dn-entry-with-ldap-decode-attribute.patch --]
[-- Type: text/x-patch, Size: 1625 bytes --]

From c9ce15f52b61b55fa9655b81fb99ff7b5095001c Mon Sep 17 00:00:00 2001
From: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
Date: Sun, 18 Jun 2023 12:32:54 +0200
Subject: [PATCH 2/2] Do not process dn entry with `ldap-decode-attribute'

Function `ldap-search' errors out with `wrong-type-argument listp'
when called with WITHDN non-nil.
* lisp/net/ldap.el (ldap-search): Do not process first element of
RECORD with `ldap-decode-attribute' if WITHDN is non-nil.  (Bug#64089)
---
 lisp/net/ldap.el | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/lisp/net/ldap.el b/lisp/net/ldap.el
index 78405414a28..1269db908d7 100644
--- a/lisp/net/ldap.el
+++ b/lisp/net/ldap.el
@@ -484,11 +484,21 @@ ldap-search
                                          attrsonly ,attrsonly
                                          withdn ,withdn
                                          ,@host-plist))))
-    (if ldap-ignore-attribute-codings
-	result
+    (cond
+     (ldap-ignore-attribute-codings
+      result)
+     (withdn
+      ;; Do not process first element of RECORD with
+      ;; `ldap-decode-attribute' if WITHDN is non-nil. (Bug#64089)
       (mapcar (lambda (record)
-		(mapcar #'ldap-decode-attribute record))
-	      result))))
+                (cons (car record)
+                      (mapcar #'ldap-decode-attribute
+                              (cdr record))))
+              result))
+     (t
+      (mapcar (lambda (record)
+                (mapcar #'ldap-decode-attribute record))
+              result)))))
 
 (defun ldap-password-read (host)
   "Read LDAP password for HOST.
-- 
2.30.2