Re: [exim] missing comma in ldap replies

Top Page
Delete this message
Reply to this message
Author: Jeremy Harris
Date:  
To: exim-users
Subject: Re: [exim] missing comma in ldap replies
On 27/08/14 18:03, Heiko Schlittermann wrote:
> Jeremy Harris <jgh@???> (Mi 27 Aug 2014 18:56:04 CEST):
>> On 27/08/14 17:36, Heiko Schlittermann wrote:
>>
>>> |LDAP entry loop
>>> |LDAP attr loop x-MailPrimaryAddress:hs@???
>>> |LDAP attr loop x-MailAlternateAddress:heiko@???
>>> |LDAP attr loop x-MailAlternateAddress:heiko@???
>>> |search ended by ldap_result yielding 101
>>> |ldap_parse_result: 0
>>> |ldap_parse_result yielded 0: Success
>>> |LDAP search: returning: hs@???@m.schlittermann.de,heiko@???
>>
>> Hmm. Looks like we think we only asked for one attribute, and rely
>> on that for formatting the results, but LDAP returned us
>> two (the second being multi-valued, but that doesn't matter here).
>
> All attributes are in fact the same,
>
>        mail <-----------------------,
>                                     |
>        x-MailPrimaryAddress    SUP mail
>        x-MailAlternateAddress  SUP mail

>
> mail is the base object class for these x-Mail… attributes, this way
> it's multivalued. But, in fact, x-MailPrimaryAddress is declared as
> SINGLE.
>
> The schema definiation:
>
> attributetype (
>     eximAttribute:1 NAME ('x-MailPrimaryAddress' 'x-MailAddress' 'x-MailOfficialAddress') 
>     DESC 'official mail address'
>     SINGLE-VALUE
>     SUP mail )

>
> attributetype (
>     eximAttribute:2 NAME ('x-MailAlternateAddress' 'x-MailAlias') 
>     DESC 'alternate mail address (alias)'
>     SUP mail )


You may be able to tell I don't really talk LDAP :)

What do we want the Exim expansion's view of the results to be?
Labelled by the base object class, or by the text
"x-MailAlternateAddress" (or whchever)?

The latter will be closest to the current code; the attached patch
might do the trick;
--
Cheers,
Jeremy

diff --git a/src/src/lookups/ldap.c b/src/src/lookups/ldap.c
index f77229d..64af12d 100644
--- a/src/src/lookups/ldap.c
+++ b/src/src/lookups/ldap.c
@@ -155,7 +155,6 @@ uschar *error1 = NULL; /* string representation of errcode (static) */
uschar *error2 = NULL; /* error message from the server */
uschar *matched = NULL; /* partially matched DN */

-int    attr_count = 0;
 int    error_yield = DEFER;
 int    msgid;
 int    rc, ldap_rc, ldap_parse_rc;
@@ -244,11 +243,6 @@ if (host != NULL)
   #endif
   }


-/* Count the attributes; we need this later to tell us how to format results */
-
-for (attrp = USS ludp->lud_attrs; attrp != NULL && *attrp != NULL; attrp++)
-  attr_count++;
-
 /* See if we can find a cached connection to this host. The port is not
 relevant for ldapi. The host name pointer is set to NULL if no host was given
 (implying the library default), rather than to the empty string. Note that in
@@ -720,7 +714,6 @@ while ((rc = ldap_result(lcp->ld, msgid, 0, timeoutptr, &result)) ==
       e = ldap_next_entry(lcp->ld, e))
     {
     uschar *new_dn;
-    BOOL insert_space = FALSE;


     DEBUG(D_lookup) debug_printf("LDAP entry loop\n");


@@ -769,99 +762,118 @@ while ((rc = ldap_result(lcp->ld, msgid, 0, timeoutptr, &result)) ==
     sequence of name=value pairs, with the value always in quotes. If there are
     multiple values, they are given within the quotes, comma separated. */


-    else for (attr = US ldap_first_attribute(lcp->ld, e, &ber);
-              attr != NULL;
-              attr = US ldap_next_attribute(lcp->ld, e, ber))
+    else
       {
-      if (attr[0] != 0)
-        {
-        /* Get array of values for this attribute. */
-
-        if ((firstval = values = USS ldap_get_values(lcp->ld, e, CS attr))
-             != NULL)
-          {
-          if (attr_count != 1)
-            {
-            if (insert_space)
-              data = string_cat(data, &size, &ptr, US" ", 1);
-            else
-              insert_space = TRUE;
-            data = string_cat(data, &size, &ptr, attr, Ustrlen(attr));
-            data = string_cat(data, &size, &ptr, US"=\"", 2);
-            }
-
-          while (*values != NULL)
-            {
-            uschar *value = *values;
-            int len = Ustrlen(value);
-
-            DEBUG(D_lookup) debug_printf("LDAP attr loop %s:%s\n", attr, value);
-
-            if (values != firstval)
-              data = string_cat(data, &size, &ptr, US",", 1);
-
-            /* For multiple attributes, the data is in quotes. We must escape
-            internal quotes, backslashes, newlines, and must double commas. */
-
-            if (attr_count != 1)
-              {
-              int j;
-              for (j = 0; j < len; j++)
-                {
-                if (value[j] == '\n')
-                  data = string_cat(data, &size, &ptr, US"\\n", 2);
-                else if (value[j] == ',')
-                  data = string_cat(data, &size, &ptr, US",,", 2);
-                else
-                  {
-                  if (value[j] == '\"' || value[j] == '\\')
-                    data = string_cat(data, &size, &ptr, US"\\", 1);
-                  data = string_cat(data, &size, &ptr, value+j, 1);
-                  }
-                }
-              }
-
-            /* For single attributes, just double commas */
-
-        else
-          {
-          int j;
-          for (j = 0; j < len; j++)
-            {
-            if (value[j] == ',')
-              data = string_cat(data, &size, &ptr, US",,", 2);
-            else
-              data = string_cat(data, &size, &ptr, value+j, 1);
-            }
-          }
+      BOOL multi_attr = FALSE;
+      BOOL insert_space = FALSE;


+      for (attr = US ldap_first_attribute(lcp->ld, e, &ber);
+              attr && !multi_attr;
+              attr = US ldap_next_attribute(lcp->ld, e, ber))
+    {
+    if (attr[0])
+      multi_attr = TRUE;


-            /* Move on to the next value */
+#if defined LDAP_LIB_NETSCAPE || defined LDAP_LIB_OPENLDAP2
+    ldap_memfree(attr);
+#endif
+    }


-            values++;
-            attribute_found = TRUE;
-            }
+      for (attr = US ldap_first_attribute(lcp->ld, e, &ber);
+              attr;
+              attr = US ldap_next_attribute(lcp->ld, e, ber))
+    {
+    if (attr[0])
+      {
+      DEBUG(D_lookup) debug_printf("LDAP attr loop %s\n", attr);
+      /* Get array of values for this attribute. */
+
+      if ((firstval = values = USS ldap_get_values(lcp->ld, e, CS attr))
+           != NULL)
+        {
+        if (multi_attr)
+          {
+          if (insert_space)
+        data = string_cat(data, &size, &ptr, US" ", 1);
+          else
+        insert_space = TRUE;
+          data = string_cat(data, &size, &ptr, attr, Ustrlen(attr));
+          data = string_cat(data, &size, &ptr, US"=\"", 2);
+          }


-          /* Closing quote at the end of the data for a named attribute. */
+        while (*values)
+          {
+          uschar *value = *values;
+          int len = Ustrlen(value);
+
+          DEBUG(D_lookup) debug_printf("LDAP value loop %s:%s\n",
+                        attr, value);
+
+          if (values != firstval)
+        data = string_cat(data, &size, &ptr, US",", 1);
+
+          /* For multiple attributes, the data is in quotes. We must escape
+          internal quotes, backslashes, newlines, and must double commas. */
+
+          if (multi_attr)
+        {
+        int j;
+        for (j = 0; j < len; j++)
+          {
+          if (value[j] == '\n')
+            data = string_cat(data, &size, &ptr, US"\\n", 2);
+          else if (value[j] == ',')
+            data = string_cat(data, &size, &ptr, US",,", 2);
+          else
+            {
+            if (value[j] == '\"' || value[j] == '\\')
+              data = string_cat(data, &size, &ptr, US"\\", 1);
+            data = string_cat(data, &size, &ptr, value+j, 1);
+            }
+          }
+        }
+
+          /* For single attributes, just double commas */
+
+          else
+        {
+        int j;
+        for (j = 0; j < len; j++)
+          {
+          if (value[j] == ',')
+            data = string_cat(data, &size, &ptr, US",,", 2);
+          else
+            data = string_cat(data, &size, &ptr, value+j, 1);
+          }
+        }
+
+
+          /* Move on to the next value */
+
+          values++;
+          attribute_found = TRUE;
+          }


-          if (attr_count != 1)
-            data = string_cat(data, &size, &ptr, US"\"", 1);
+        /* Closing quote at the end of the data for a named attribute. */


-          /* Free the values */
+        if (multi_attr)
+          data = string_cat(data, &size, &ptr, US"\"", 1);


-          ldap_value_free(CSS firstval);
-          }
-        }
+        /* Free the values */


-      #if defined LDAP_LIB_NETSCAPE || defined LDAP_LIB_OPENLDAP2
+        ldap_value_free(CSS firstval);
+        }
+      }


-      /* Netscape and OpenLDAP2 LDAP's attrs are dynamically allocated and need
-      to be freed. UMich LDAP stores them in static storage and does not require
-      this. */
+#if defined LDAP_LIB_NETSCAPE || defined LDAP_LIB_OPENLDAP2


-      ldap_memfree(attr);
-      #endif
-      }        /* End "for" loop for extracting attributes from an entry */
+    /* Netscape and OpenLDAP2 LDAP's attrs are dynamically allocated
+    and need to be freed. UMich LDAP stores them in static storage
+    and does not require this. */
+    ldap_memfree(attr);
+#endif
+    }        /* End "for" loop for extracting attributes from an entry */
+      }
     }          /* End "for" loop for extracting entries from a result */


/* Free the result */