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

[Filipp Gunbin]

On 19/06/2023 22:09 +0300, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Cc: 64089@debbugs.gnu.org,  jschmidt4gnu@vodafonemail.de,
>>   monnier@iro.umontreal.ca
>> Date: Mon, 19 Jun 2023 21:38:19 +0300
>> 
>> On 19/06/2023 20:24 +0300, Eli Zaretskii wrote:
>> 
>> >> Previously ldap-search withdn=t returned:
>> >> 
>> >> (("<dn value>" ("mail" "...")))
>> >> 
>> >> But now it will return:
>> >> 
>> >> ((("dn" "<dn value>") ("mail" "...")))
>> >
>> > In which situations will the current code cause trouble, where the
>> > original code didn't?
>> 
>> The above formats are what ldap-search returns when given withdn=t.
>> 
>> Emacs codebase doesn't use withdn=t at all, but ldap-search is a "public"
>> API function, and we're not supposed to change its return format?
>
> Where is its return format documented for the withdn=t case?

The docstring only says "If WITHDN is non-nil, each entry in the result
will be prepended with its distinguished name WITHDN.", so nowhere,
which looks like an omission.

Still, returning it as string makes more sense, because:

1) That's how it worked before the change.

2) It basically makes the result an alist with elements "(id . attrs)"
  (dn is a kind of id in ldap).

3) Including dn as attribute, with name, is redundand - the client
  requesting withdn=t already knows the name "dn".

Honestly, I don't get why we need to evaluate this breakage separately,
while the others are not allowed, at least without good reason...

Btw, the patch on emacs-29 also doesn't work (at least for me) because
in the end of the output, there's an additional line "Process ldapsearch
finished", which gets parsed as dn (as start of new ldap record).  So
the error happens, just in another place.  I accounted for that in my
patch.

Thanks.
Filipp