Bug 633150 - Change nsILDAPURL::setAttributes to take a string rather than an array; r=neil sr=bienvenu
authorMark Banner <bugzilla@standard8.plus.com>
Thu, 24 Feb 2011 17:08:16 +0000
changeset 7219 e9817d0c0d024f2a8c9c4372e8ce41c8119ff243
parent 7218 268e2837835521c334a6ddf700761d12fc33a20e
child 7220 8310d0fd89a6199e34c19bc73dd71ca15abe7525
push idunknown
push userunknown
push dateunknown
reviewersneil, bienvenu
bugs633150
Bug 633150 - Change nsILDAPURL::setAttributes to take a string rather than an array; r=neil sr=bienvenu
ldap/xpcom/public/nsILDAPOperation.idl
ldap/xpcom/public/nsILDAPURL.idl
ldap/xpcom/src/nsLDAPOperation.cpp
ldap/xpcom/src/nsLDAPSyncQuery.cpp
ldap/xpcom/src/nsLDAPSyncQuery.h
ldap/xpcom/src/nsLDAPURL.cpp
ldap/xpcom/src/nsLDAPURL.h
ldap/xpcom/tests/unit/test_nsLDAPURL.js
mailnews/addrbook/public/nsIAbLDAPAttributeMap.idl
mailnews/addrbook/public/nsILDAPAutoCompFormatter.idl
mailnews/addrbook/src/nsAbLDAPAttributeMap.js
mailnews/addrbook/src/nsAbLDAPAutoCompFormatter.cpp
mailnews/addrbook/src/nsAbLDAPDirectoryQuery.cpp
mailnews/addrbook/src/nsAbLDAPReplicationData.cpp
mailnews/addrbook/src/nsLDAPAutoCompleteSession.cpp
mailnews/addrbook/src/nsLDAPAutoCompleteSession.h
--- a/ldap/xpcom/public/nsILDAPOperation.idl
+++ b/ldap/xpcom/public/nsILDAPOperation.idl
@@ -51,17 +51,17 @@ interface nsIArray;
 %{C++
 #define NS_LDAPOPERATION_CONTRACTID "@mozilla.org/network/ldap-operation;1" 
 %}
 
 // XXXdmose check to make sure ctl-related err codes documented
 
 typedef PRUint32 PRIntervalTime;
 
-[scriptable, uuid(40cc61c6-539b-4157-b0f0-ed81ecd29b8e)]
+[scriptable, uuid(4dfb1b19-fc8f-4525-92e7-f97b78a9747a)]
 interface nsILDAPOperation : nsISupports
 {
     /**
      * The connection this operation is on. 
      * 
      * @exception NS_ERROR_ILLEGAL_VALUE        a NULL pointer was passed in
      */
     readonly attribute nsILDAPConnection connection;
@@ -262,18 +262,18 @@ interface nsILDAPOperation : nsISupports
      * Kicks off an asynchronous search request.  The "ext" stands for 
      * "extensions", and is intended to convey that this method will
      * eventually support the extensions described in the
      * draft-ietf-ldapext-ldap-c-api-04.txt Internet Draft.
      *
      * @param aBaseDn           Base DN to search
      * @param aScope            One of SCOPE_{BASE,ONELEVEL,SUBTREE}
      * @param aFilter           Search filter
-     * @param aAttrCount        Number of attributes we request (0 for all)
-     * @param aAttributes       Array of strings, holding the attrs we need
+     * @param aAttributes       Comma separated list of values, holding the
+     *                          attributes we need
      * @param aTimeOut          How long to wait
      * @param aSizeLimit        Maximum number of entries to return.
      *
      * @exception NS_ERROR_NOT_INITIALIZED      operation not initialized
      * @exception NS_ERROR_LDAP_ENCODING_ERROR  error during BER-encoding
      * @exception NS_ERROR_LDAP_SERVER_DOWN     the LDAP server did not
      *                                          receive the request or the
      *                                          connection was lost
@@ -281,18 +281,17 @@ interface nsILDAPOperation : nsISupports
      * @exception NS_ERROR_INVALID_ARG          invalid argument
      * @exception NS_ERROR_LDAP_NOT_SUPPORTED   not supported in the version
      *                                          of the LDAP protocol that the
      *                                          client is using
      * @exception NS_ERROR_LDAP_FILTER_ERROR
      * @exception NS_ERROR_UNEXPECTED
      */
     void searchExt(in AUTF8String aBaseDn, in PRInt32 aScope,
-                   in AUTF8String aFilter, in unsigned long aAttrCount,
-                   [array, size_is(aAttrCount)] in string aAttributes,
+                   in AUTF8String aFilter, in ACString aAttributes,
                    in PRIntervalTime aTimeOut, in PRInt32 aSizeLimit);
 
     /**  
      * Cancels an async operation that is in progress.
      *
      * XXX controls not supported yet
      *
      * @exception NS_ERROR_NOT_IMPLEMENTED      server or client controls
--- a/ldap/xpcom/public/nsILDAPURL.idl
+++ b/ldap/xpcom/public/nsILDAPURL.idl
@@ -47,17 +47,17 @@
 /**
  * Strings in methods inherited from nsIURI, which are using XPIDL
  * |string| types, are expected to be UTF8 encoded. All such strings
  * in this interface, except attribute types (e.g. "cn"), should in fact
  * be UTF8. It's important to remember that attributes can not be UTF8,
  * they can only be of a limited subset of ASCII (see RFC 2251).
  */
 
-[scriptable, uuid(1018ee06-f708-4bd7-b7df-11285332a404)]
+[scriptable, uuid(acc5cd8f-95a4-454b-acc4-4646b9424ef2)]
 interface nsILDAPURL : nsIURI {
     /**
      * Initialize an LDAP URL
      *
      * @param aUrlType       - one of the URLTYPE_ flags @seealso nsIStandardURL
      * @param aDefaultPort   - if the port parsed from the URL string matches
      *                         this port, then the port will be removed from the
      *                         canonical form of the URL.
@@ -87,59 +87,46 @@ interface nsILDAPURL : nsIURI {
      * for the getter:
      *
      * @exception NS_ERROR_NULL_POINTER     NULL pointer to GET method
      * @exception NS_ERROR_OUT_OF_MEMORY	Ran out of memory
      */
     attribute AUTF8String dn;
 
     /**
-     * Return all LDAP attributes currently set. The empty array indicates
-     * that all attributes are requested.
-     *
-     * @exception NS_ERROR_NULL_POINTER     NULL pointer to GET method
-     * @exception NS_ERROR_OUT_OF_MEMORY	Ran out of memory
+     * The attributes to get for this URL, in comma-separated format. If the
+     * list is empty, all attributes are requested.
      */
-    void getAttributes(out unsigned long aCount, 
-                       [retval, array, size_is(aCount)] out string aAttrs);
-
-    /**
-     * Set the array of attributes, dropping whatever was there before.
-     *
-     * @param aAttrs                        An array of LDAP attributes
-     * @exception NS_ERROR_OUT_OF_MEMORY	Ran out of memory
-     */
-    void setAttributes(in unsigned long aCount,
-                       [array, size_is(aCount)] in string aAttrs);
+    attribute ACString attributes;
 
     /**
      * Add one attribute to the array of attributes to request. If the
      * attribute is already in our array, this becomes a noop.
      *
      * @param aAttribute          An LDAP attribute (e.g. "cn")
      */
-    void addAttribute(in string aAttribute);
+    void addAttribute(in ACString aAttribute);
 
     /**
      * Remove one attribute from the array of attributes to request. If
      * the attribute didn't exist in the array, this becomes a noop.
      *
      * @param aAttribute                    An LDAP attribute (e.g. "cn")
      * @exception NS_ERROR_OUT_OF_MEMORY	Ran out of memory
      */
-    void removeAttribute(in string aAttribute);
+    void removeAttribute(in ACString aAttribute);
 
     /**
      * Test if an attribute is in our list of attributes already
      *
      * @param aAttribute                    An LDAP attribute (e.g. "cn")
      * @return boolean                      Truth value
      * @exception NS_ERROR_NULL_POINTER     NULL pointer to GET method
      */
-    boolean hasAttribute(in string aAttribute);
+    boolean hasAttribute(in ACString aAttribute);
 
     /**
      * The scope of the search.  defaults to SCOPE_BASE. 
      *
      * @exception NS_ERROR_NULL_POINTER     NULL pointer to GET method
      * @exception NS_ERROR_MALFORMED_URI	Illegal base to SET method
      */
     attribute long scope;
--- a/ldap/xpcom/src/nsLDAPOperation.cpp
+++ b/ldap/xpcom/src/nsLDAPOperation.cpp
@@ -424,93 +424,86 @@ convertControlArray(nsIArray *aXpcomArra
 
     *aArray = controls;
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsLDAPOperation::SearchExt(const nsACString& aBaseDn, PRInt32 aScope,
                            const nsACString& aFilter,
-                           PRUint32 aAttrCount, const char **aAttributes,
+                           const nsACString &aAttributes,
                            PRIntervalTime aTimeOut, PRInt32 aSizeLimit)
 {
     if (!mMessageListener) {
         NS_ERROR("nsLDAPOperation::SearchExt(): mMessageListener not set");
         return NS_ERROR_NOT_INITIALIZED;
     }
 
     // XXX add control logging
     PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
            ("nsLDAPOperation::SearchExt(): called with aBaseDn = '%s'; "
-            "aFilter = '%s', aAttrCounts = %u, aSizeLimit = %d",
+            "aFilter = '%s'; aAttributes = %s; aSizeLimit = %d",
             PromiseFlatCString(aBaseDn).get(),
             PromiseFlatCString(aFilter).get(),
-            aAttrCount, aSizeLimit));
-
-    char **attrs = 0;
-
-    // Convert our XPCOM style C-Array to one that the C-SDK will like, i.e.
-    // add a last NULL element.
-    //
-    if (aAttrCount && aAttributes) {
-        attrs = static_cast<char **>
-                           (nsMemory::Alloc((aAttrCount + 1) * sizeof(char *)));
-        if (!attrs) {
-            NS_ERROR("nsLDAPOperation::SearchExt: out of memory ");
-            return NS_ERROR_OUT_OF_MEMORY;
-        }
-        memcpy(attrs, aAttributes, aAttrCount * sizeof(char *));
-        attrs[aAttrCount] = 0;
-    }
+            PromiseFlatCString(aAttributes).get(), aSizeLimit));
 
     LDAPControl **serverctls = 0;
     nsresult rv;
     if (mServerControls) {
         rv = convertControlArray(mServerControls, &serverctls);
         if (NS_FAILED(rv)) {
             PR_LOG(gLDAPLogModule, PR_LOG_ERROR,
                    ("nsLDAPOperation::SearchExt(): error converting server "
                     "control array: %x", rv));
-
-            if (attrs) {
-                nsMemory::Free(attrs);
-            }
             return rv;
         }
     }
 
-
     LDAPControl **clientctls = 0;
     if (mClientControls) {
         rv = convertControlArray(mClientControls, &clientctls);
         if (NS_FAILED(rv)) {
             PR_LOG(gLDAPLogModule, PR_LOG_ERROR,
                    ("nsLDAPOperation::SearchExt(): error converting client "
                     "control array: %x", rv));
-            if (attrs) {
-                nsMemory::Free(attrs);
-            }
             ldap_controls_free(serverctls);
             return rv;
         }
     }
 
+    // Convert our comma separated string to one that the C-SDK will like, i.e.
+    // convert to a char array and add a last NULL element.
+    nsTArray<nsCString> attrArray;
+    ParseString(aAttributes, ',', attrArray);
+    char **attrs = nsnull;
+    PRUint32 origLength = attrArray.Length();
+    if (origLength)
+    {
+      attrs = static_cast<char **> (NS_Alloc((origLength + 1) * sizeof(char *)));
+      if (!attrs)
+        return NS_ERROR_OUT_OF_MEMORY;
+
+      for (PRUint32 i = 0; i < origLength; ++i)
+        attrs[i] = ToNewCString(attrArray[i]);
+
+      attrs[origLength] = 0;
+    }
+
     // XXX deal with timeout here
     int retVal = ldap_search_ext(mConnectionHandle,
                                  PromiseFlatCString(aBaseDn).get(),
                                  aScope, PromiseFlatCString(aFilter).get(),
                                  attrs, 0, serverctls, clientctls, 0,
                                  aSizeLimit, &mMsgID);
 
     // clean up
     ldap_controls_free(serverctls);
     ldap_controls_free(clientctls);
-    if (attrs) {
-        nsMemory::Free(attrs);
-    }
+    // The last entry is null, so no need to free that.
+    NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(origLength, attrs);
 
     rv = TranslateLDAPErrorToNSError(retVal);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // make sure the connection knows where to call back once the messages
     // for this operation start coming in
     //
     rv = mConnection->AddPendingOperation(mMsgID, this);
--- a/ldap/xpcom/src/nsLDAPSyncQuery.cpp
+++ b/ldap/xpcom/src/nsLDAPSyncQuery.cpp
@@ -1,9 +1,9 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
  *
@@ -49,17 +49,17 @@
 // nsISupports Implementation
 
 NS_IMPL_THREADSAFE_ISUPPORTS2(nsLDAPSyncQuery, nsILDAPSyncQuery, nsILDAPMessageListener)
 
 // Constructor
 //
 nsLDAPSyncQuery::nsLDAPSyncQuery() :
     mFinished(PR_FALSE), // This is a control variable for event loop
-    mAttrCount(0), mAttrs(0), mProtocolVersion(nsILDAPConnection::VERSION3)
+    mProtocolVersion(nsILDAPConnection::VERSION3)
 {
 }
 
 // Destructor
 //
 nsLDAPSyncQuery::~nsLDAPSyncQuery()
 {
 }
@@ -201,67 +201,67 @@ nsLDAPSyncQuery::OnLDAPBind(nsILDAPMessa
     // ok, we're starting a search
     //
     return StartLDAPSearch();
 }
 
 nsresult
 nsLDAPSyncQuery::OnLDAPSearchEntry(nsILDAPMessage *aMessage)
 {
-    nsresult rv;       
-
-    // Attributes are retrieved in StartLDAPSearch
-    // iterate through them
-    //
-    for (PRUint32 i = 0; i < mAttrCount; i++) {
-
-        PRUnichar **vals;
-        PRUint32 valueCount;
+  PRUint32 attrCount;
+  char** attributes;
+  nsresult rv = aMessage->GetAttributes(&attrCount, &attributes);
+  if (NS_FAILED(rv))
+  {
+    NS_WARNING("nsLDAPSyncQuery:OnLDAPSearchEntry(): "
+               "aMessage->GetAttributes() failed");
+    FinishLDAPQuery();
+    return rv;
+  }
 
-        // get the values of this attribute
-        // XXX better failure handling
-        //
-        rv = aMessage->GetValues(mAttrs[i], &valueCount, &vals);
-        if (NS_FAILED(rv)) {
-            NS_WARNING("nsLDAPSyncQuery:OnLDAPSearchEntry(): "
-                       "aMessage->GetValues() failed\n");
-            FinishLDAPQuery();
-            return rv;;
-        }
+  // Iterate through the attributes received in this message
+  for (PRUint32 i = 0; i < attrCount; i++)
+  {
+    PRUnichar **vals;
+    PRUint32 valueCount;
 
-        // store  all values of this attribute in the mResults.
-        //
-        for (PRUint32 j = 0; j < valueCount; j++) {
-            mResults.Append(PRUnichar('\n'));
-            mResults.AppendASCII(mAttrs[i]);
-            mResults.Append(PRUnichar('='));
-            mResults.Append(vals[j]);
-        }
-        
-        NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(valueCount, vals);
-
+    // Get the values of this attribute.
+    // XXX better failure handling
+    rv = aMessage->GetValues(attributes[i], &valueCount, &vals);
+    if (NS_FAILED(rv))
+    {
+      NS_WARNING("nsLDAPSyncQuery:OnLDAPSearchEntry(): "
+                 "aMessage->GetValues() failed\n");
+      FinishLDAPQuery();
+      break;
     }
 
-    return NS_OK;
+    // Store all values of this attribute in the mResults.
+    for (PRUint32 j = 0; j < valueCount; j++) {
+      mResults.Append(PRUnichar('\n'));
+      mResults.AppendASCII(attributes[i]);
+      mResults.Append(PRUnichar('='));
+      mResults.Append(vals[j]);
+    }
+
+    NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(valueCount, vals);
+  }
+  NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(attrCount, attributes);
+
+  return rv;
 }
 
 
 nsresult
 nsLDAPSyncQuery::OnLDAPSearchResult(nsILDAPMessage *aMessage)
 {
     // We are done with the LDAP search.
     // Release the control variable for the eventloop and other members
     // 
     FinishLDAPQuery();
-    
-    // Release memory allocated for mAttrs
-    
-    if (mAttrCount > 0)
-        NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(mAttrCount, mAttrs);
-
     return NS_OK;
 }
 
 nsresult
 nsLDAPSyncQuery::StartLDAPSearch()
 {
     nsresult rv; 
     nsCOMPtr<nsILDAPMessageListener> selfProxy; // for callback
@@ -325,28 +325,25 @@ nsLDAPSyncQuery::StartLDAPSearch()
     //
     PRInt32 scope;
     rv = mServerURL->GetScope(&scope);
     if (NS_FAILED(rv)) {
         FinishLDAPQuery();
         return NS_ERROR_UNEXPECTED;
     }
 
-    
-    rv = mServerURL->GetAttributes(&mAttrCount, &mAttrs);
+    nsCAutoString attributes;
+    rv = mServerURL->GetAttributes(attributes);
     if (NS_FAILED(rv)) {
         FinishLDAPQuery();
         return NS_ERROR_UNEXPECTED;
     }
 
-
     // time to kick off the search.
-    //
-    rv = mOperation->SearchExt(dn, scope, urlFilter, mAttrCount,
-                               const_cast<const char **>(mAttrs), 0, 0);
+    rv = mOperation->SearchExt(dn, scope, urlFilter, attributes, 0, 0);
 
     if (NS_FAILED(rv)) {
         FinishLDAPQuery();
         return NS_ERROR_FAILURE;
     }
 
     return NS_OK;
 }
--- a/ldap/xpcom/src/nsLDAPSyncQuery.h
+++ b/ldap/xpcom/src/nsLDAPSyncQuery.h
@@ -65,18 +65,16 @@ class nsLDAPSyncQuery : public nsILDAPSy
     virtual ~nsLDAPSyncQuery();
 
   protected:
 
     nsCOMPtr<nsILDAPConnection> mConnection; // connection used for search
     nsCOMPtr<nsILDAPOperation> mOperation;   // current ldap op
     nsCOMPtr<nsILDAPURL> mServerURL;         // LDAP URL
     PRBool mFinished;                        // control variable for eventQ
-    PRUint32 mAttrCount;                     // No. of attrbiutes
-    char **mAttrs;                           // Attributes to search
     nsString mResults;                       // values to return
     PRUint32 mProtocolVersion;               // LDAP version to use
 
     nsresult InitConnection();
     // check that we bound ok and start then call StartLDAPSearch
     nsresult OnLDAPBind(nsILDAPMessage *aMessage); 
 
     // add to the results set
--- a/ldap/xpcom/src/nsLDAPURL.cpp
+++ b/ldap/xpcom/src/nsLDAPURL.cpp
@@ -34,23 +34,23 @@
  * decision by deleting the provisions above and replace them with the notice
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsLDAPURL.h"
-#include "nsReadableUtils.h"
 #include "netCore.h"
 #include "plstr.h"
 #include "nsCOMPtr.h"
 #include "nsNetCID.h"
 #include "nsComponentManagerUtils.h"
 #include "nsIStandardURL.h"
+#include "nsMsgUtils.h"
 
 // The two schemes we support, LDAP and LDAPS
 //
 NS_NAMED_LITERAL_CSTRING(LDAP_SCHEME, "ldap");
 NS_NAMED_LITERAL_CSTRING(LDAP_SSL_SCHEME, "ldaps");
 
 NS_IMPL_THREADSAFE_ISUPPORTS2(nsLDAPURL, nsILDAPURL, nsIURI)
 
@@ -95,33 +95,27 @@ nsLDAPURL::Init(PRUint32 aUrlType, PRInt
 void
 nsLDAPURL::GetPathInternal(nsCString &aPath)
 {
   aPath.Assign('/');
 
   if (!mDN.IsEmpty())
     aPath.Append(mDN);
 
-  PRUint32 count = mAttributes.Count();
-  if (count)
-  {
+  if (!mAttributes.IsEmpty())
     aPath.Append('?');
-    PRUint32 index = 0;
 
-    while (index < count)
-    {
-      aPath.Append(*(mAttributes.CStringAt(index++)));
-      if (index < count)
-        aPath.Append(',');
-    }
-  }
+  // If mAttributes isn't empty, cut off the internally stored commas at start
+  // and end, and append to the path.
+  if (!mAttributes.IsEmpty())
+    aPath.Append(Substring(mAttributes, 1, mAttributes.Length() - 2));
 
   if (mScope || !mFilter.IsEmpty())
   {
-    aPath.Append((count ? "?" : "??"));
+    aPath.Append((mAttributes.IsEmpty() ? "??" : "?"));
     if (mScope)
     {
       if (mScope == SCOPE_ONELEVEL)
         aPath.Append("one");
       else if (mScope == SCOPE_SUBTREE)
         aPath.Append("sub");
     }
     if (!mFilter.IsEmpty())
@@ -130,51 +124,34 @@ nsLDAPURL::GetPathInternal(nsCString &aP
       aPath.Append(mFilter);
     }
   }
 }
 
 nsresult
 nsLDAPURL::SetPathInternal(const nsCString &aPath)
 {
-  PRUint32 rv, count;
+  PRUint32 rv;
   LDAPURLDesc *desc;
-  nsCString str;
-  char **attributes;
 
   // This is from the LDAP C-SDK, which currently doesn't
   // support everything from RFC 2255... :(
   //
   rv = ldap_url_parse(aPath.get(), &desc);
   switch (rv) {
   case LDAP_SUCCESS:
     // The base URL can pick up the host & port details and deal with them
     // better than we can
     mDN = desc->lud_dn;
     mScope = desc->lud_scope;
     mFilter = desc->lud_filter;
     mOptions = desc->lud_options;
-
-    // Set the attributes array, need to count it first.
-    //
-    count = 0;
-    attributes = desc->lud_attrs;
-    while (attributes && *attributes++)
-      count++;
-
-    if (count) {
-      rv = SetAttributes(count, const_cast<const char **>(desc->lud_attrs));
-      // This error could only be out-of-memory, so pass it up
-      //
-      if (NS_FAILED(rv)) {
-        return rv;
-      }
-    } else {
-      mAttributes.Clear();
-    }
+    rv = SetAttributeArray(desc->lud_attrs);
+    if (NS_FAILED(rv))
+      return rv;
 
     ldap_free_urldesc(desc);
     return NS_OK;
 
   case LDAP_URL_ERR_NOTLDAP:
   case LDAP_URL_ERR_NODN:
   case LDAP_URL_ERR_BADSCOPE:
     return NS_ERROR_MALFORMED_URI;
@@ -487,132 +464,158 @@ NS_IMETHODIMP nsLDAPURL::SetDn(const nsA
   // Now get the current path
   nsCString newPath;
   GetPathInternal(newPath);
 
   // and update the base url
   return mBaseURL->SetPath(newPath);
 }
 
-// void getAttributes (out unsigned long aCount, 
-//                     [array, size_is (aCount), retval] out string aAttrs);
-//
-NS_IMETHODIMP nsLDAPURL::GetAttributes(PRUint32 *aCount, char ***_retval)
+NS_IMETHODIMP nsLDAPURL::GetAttributes(nsACString &aAttributes)
 {
-    NS_ENSURE_ARG_POINTER(aCount);
-    NS_ENSURE_ARG_POINTER(_retval);
-
-    PRUint32 index = 0;
-    PRUint32 count;
-    char **cArray = nsnull;
-
-    if (!_retval) {
-        NS_ERROR("nsLDAPURL::GetAttributes: null pointer ");
-        return NS_ERROR_NULL_POINTER;
-    }
+  if (mAttributes.IsEmpty())
+  {
+    aAttributes.Truncate();
+    return NS_OK;
+  }
 
-    count = mAttributes.Count();
-    if (count > 0) {
-        cArray = static_cast<char **>(nsMemory::Alloc(count * sizeof(char *)));
-        if (!cArray) {
-            NS_ERROR("nsLDAPURL::GetAttributes: out of memory ");
-            return NS_ERROR_OUT_OF_MEMORY;
-        }
+  NS_ASSERTION(mAttributes[0] == ',' &&
+               mAttributes[mAttributes.Length() - 1] == ',',
+               "mAttributes does not begin and end with a comma");
 
-        // Loop through the string array, and build up the C-array.
-        //
-        while (index < count) {
-            if (!(cArray[index] = ToNewCString(*(mAttributes.CStringAt(index))))) {
-                NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(index, cArray);
-                NS_ERROR("nsLDAPURL::GetAttributes: out of memory ");
-                return NS_ERROR_OUT_OF_MEMORY;
-            }
-            index++;
-        }
-    }
-    *aCount = count;
-    *_retval = cArray;
+  // We store the string internally with comma before and after, so strip
+  // them off here.
+  aAttributes = Substring(mAttributes, 1, mAttributes.Length() - 2);
+  return NS_OK;
+}
 
-    return NS_OK;
-}
-// void setAttributes (in unsigned long aCount,
-//                     [array, size_is (aCount)] in string aAttrs); */
-NS_IMETHODIMP nsLDAPURL::SetAttributes(PRUint32 count, const char **aAttrs)
+NS_IMETHODIMP nsLDAPURL::SetAttributes(const nsACString &aAttributes)
 {
   if (!mBaseURL)
     return NS_ERROR_NOT_INITIALIZED;
 
-  if (count)
-    NS_ENSURE_ARG_POINTER(aAttrs);
-
-  mAttributes.Clear();
-  for (PRUint32 i = 0; i < count; ++i)
+  if (aAttributes.IsEmpty())
+    mAttributes.Truncate();
+  else
   {
-    if (!mAttributes.AppendCString(nsDependentCString(aAttrs[i])))
-    {
-      NS_ERROR("nsLDAPURL::SetAttributes: out of memory ");
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
+    // We need to make sure we start off the string with a comma.
+    if (aAttributes[0] != ',')
+      mAttributes = ',';
+
+    mAttributes.Append(aAttributes);
+
+    // Also end with a comma if appropriate.
+    if (mAttributes[mAttributes.Length() - 1] != ',')
+      mAttributes.Append(',');
   }
 
   // Now get the current path
   nsCString newPath;
   GetPathInternal(newPath);
 
   // and update the base url
   return mBaseURL->SetPath(newPath);
 }
 
-NS_IMETHODIMP nsLDAPURL::AddAttribute(const char *aAttribute)
+nsresult nsLDAPURL::SetAttributeArray(char** aAttributes)
+{
+  mAttributes.Truncate();
+
+  while (aAttributes && *aAttributes)
+  {
+    // Always start with a comma as that's what we store internally.
+    mAttributes.Append(',');
+    mAttributes.Append(*aAttributes);
+    ++aAttributes;
+  }
+
+  // Add a comma on the end if we have something.
+  if (!mAttributes.IsEmpty())
+    mAttributes.Append(',');
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP nsLDAPURL::AddAttribute(const nsACString &aAttribute)
 {
   if (!mBaseURL)
     return NS_ERROR_NOT_INITIALIZED;
 
-  NS_ENSURE_ARG_POINTER(aAttribute);
-
-  nsDependentCString str(aAttribute);
+  if (mAttributes.IsEmpty())
+  {
+    mAttributes = ',';
+    mAttributes.Append(aAttribute);
+    mAttributes.Append(',');
+  }
+  else
+  {
+    // Wrap the attribute in commas, so that we can do an exact match.
+    nsCAutoString findAttribute(',');
+    findAttribute.Append(aAttribute);
+    findAttribute.Append(',');
 
-  if (mAttributes.IndexOfIgnoreCase(str) >= 0)
-    return NS_OK;
+    // Check to see if the attribute is already stored. If it is, then also
+    // check to see if it is the last attribute in the string, or if the next
+    // character is a comma, this means we won't match substrings.
+    PRInt32 pos = mAttributes.Find(findAttribute, CaseInsensitiveCompare);
+    if (pos != -1)
+      return NS_OK;
 
-  if (!mAttributes.AppendCString(str)) {
-    NS_ERROR("nsLDAPURL::AddAttribute: out of memory ");
-    return NS_ERROR_OUT_OF_MEMORY;
+    mAttributes.Append(Substring(findAttribute, 1));
   }
 
   // Now get the current path
   nsCString newPath;
   GetPathInternal(newPath);
 
   // and update the base url
   return mBaseURL->SetPath(newPath);
 }
 
-NS_IMETHODIMP nsLDAPURL::RemoveAttribute(const char *aAttribute)
+NS_IMETHODIMP nsLDAPURL::RemoveAttribute(const nsACString &aAttribute)
 {
   if (!mBaseURL)
     return NS_ERROR_NOT_INITIALIZED;
 
-  NS_ENSURE_ARG_POINTER(aAttribute);
-  mAttributes.RemoveCString(nsDependentCString(aAttribute));
+  if (mAttributes.IsEmpty())
+    return NS_OK;
+
+  nsCAutoString findAttribute(',');
+  findAttribute.Append(aAttribute);
+  findAttribute.Append(',');
+
+  if (mAttributes.Equals(findAttribute, nsCaseInsensitiveCStringComparator()))
+    mAttributes.Truncate();
+  else
+  {
+    PRInt32 pos = mAttributes.Find(findAttribute, CaseInsensitiveCompare);
+    if (pos == -1)
+      return NS_OK;
+
+    mAttributes.Cut(pos, findAttribute.Length() - 1);
+  }
 
   // Now get the current path
   nsCString newPath;
   GetPathInternal(newPath);
 
   // and update the base url
   return mBaseURL->SetPath(newPath);
 }
 
-NS_IMETHODIMP nsLDAPURL::HasAttribute(const char *aAttribute, PRBool *_retval)
+NS_IMETHODIMP nsLDAPURL::HasAttribute(const nsACString &aAttribute,
+                                      PRBool *_retval)
 {
-  NS_ENSURE_ARG_POINTER(aAttribute);
   NS_ENSURE_ARG_POINTER(_retval);
 
-  *_retval = mAttributes.IndexOfIgnoreCase(nsDependentCString(aAttribute)) >= 0;
+  nsCAutoString findAttribute(',');
+  findAttribute.Append(aAttribute);
+  findAttribute.Append(',');
+
+  *_retval = mAttributes.Find(findAttribute, CaseInsensitiveCompare) != -1;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsLDAPURL::GetScope(PRInt32 *_retval)
 {
   NS_ENSURE_ARG_POINTER(_retval);
   *_retval = mScope;
   return NS_OK;
--- a/ldap/xpcom/src/nsLDAPURL.h
+++ b/ldap/xpcom/src/nsLDAPURL.h
@@ -34,18 +34,17 @@
  * decision by deleting the provisions above and replace them with the notice
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "ldap.h"
-#include "nsString.h"
-#include "nsVoidArray.h"
+#include "nsStringGlue.h"
 #include "nsILDAPURL.h"
 #include "nsCOMPtr.h"
 
 // cb7c67f8-0053-4072-89e9-501cbd1b35ab
 #define NS_LDAPURL_CID \
 { 0xcb7c67f8, 0x0053, 0x4072, \
   { 0x89, 0xe9, 0x50, 0x1c, 0xbd, 0x1b, 0x35, 0xab}}
 
@@ -73,16 +72,17 @@ public:
 
   nsLDAPURL();
   virtual ~nsLDAPURL();
 
 protected:
 
   void GetPathInternal(nsCString &aPath);
   nsresult SetPathInternal(const nsCString &aPath);
+  nsresult SetAttributeArray(char** aAttributes);
 
   nsCString mDN;                // Base Distinguished Name (Base DN)
   PRInt32 mScope;               // Search scope (base, one or sub)
   nsCString mFilter;            // LDAP search filter
   PRUint32 mOptions;            // Options
-  nsCStringArray mAttributes;  // List of attributes
+  nsCString mAttributes;
   nsCOMPtr<nsIURI> mBaseURL;
 };
--- a/ldap/xpcom/tests/unit/test_nsLDAPURL.js
+++ b/ldap/xpcom/tests/unit/test_nsLDAPURL.js
@@ -191,89 +191,90 @@ function run_test() {
   do_check_true(url.equals(url2));
 
   url2.spec = "ldap://localhost:389/dc=short??sub?(objectclass=*)";
 
   do_check_false(url.equals(url2));
 
   // Test Attributes
 
-  var attrs = url.getAttributes({});
-
-  do_check_eq(attrs.length, 0);
+  do_check_eq(url.attributes.length, 0);
 
   // Nothing should happend if the attribute doesn't exist
   url.removeAttribute("abc");
 
-  attrs = url.getAttributes({});
-  do_check_eq(attrs.length, 0);
+  do_check_eq(url.attributes.length, 0);
   do_check_eq(url.spec, "ldap://localhost" + portAdpt + "/dc=short??one?(objectclass=*)");
 
   url.addAttribute("dn");
   do_check_eq(url.spec, "ldap://localhost" + portAdpt + "/dc=short?dn?one?(objectclass=*)");
 
-  attrs = url.getAttributes({});
-  do_check_eq(attrs.length, 1);
-  do_check_eq(attrs[0], "dn");
+  do_check_eq(url.attributes, "dn");
 
   url.removeAttribute("dn");
 
-  attrs = url.getAttributes({});
-  do_check_eq(attrs.length, 0);
+  do_check_eq(url.attributes.length, 0);
   do_check_eq(url.spec, "ldap://localhost" + portAdpt + "/dc=short??one?(objectclass=*)");
 
-  var newAttrs = [ "abc", "def", "ghi", "jkl" ];
-
-  url.setAttributes(newAttrs.length, newAttrs);
+  var newAttrs = "abc,def,ghi,jkl";
+  url.attributes = newAttrs;
 
   var i;
 
-  attrs = url.getAttributes({});
-  do_check_eq(attrs.length, newAttrs.length);
-  for (i = 0; i < newAttrs.length; ++i)
-    do_check_eq(attrs[i], newAttrs[i]);
+  do_check_eq(url.attributes, newAttrs);
+  do_check_eq(url.spec,
+              "ldap://localhost" + portAdpt + "/dc=short?" +
+              newAttrs + "?one?(objectclass=*)");
 
-  do_check_eq(url.spec, "ldap://localhost" + portAdpt + "/dc=short?abc,def,ghi,jkl?one?(objectclass=*)");
+  // Try adding an existing attribute - should do nothing
+  url.addAttribute("def");
+  do_check_eq(url.attributes, newAttrs);
+
+  //  url.addAttribute("jk");
 
   do_check_true(url.hasAttribute("jkl"));
   do_check_true(url.hasAttribute("def"));
   do_check_true(url.hasAttribute("ABC"));
   do_check_false(url.hasAttribute("cde"));
   do_check_false(url.hasAttribute("3446"));
+  do_check_false(url.hasAttribute("kl"));
+  do_check_false(url.hasAttribute("jk"));
+
+  // Sub-string of an attribute, so this shouldn't change anything.
+  url.removeAttribute("kl");
+  url.removeAttribute("jk");
+  url.removeAttribute("ef");
+  do_check_eq(url.attributes, newAttrs);
 
   url.removeAttribute("abc");
+  newAttrs = newAttrs.substring(4);
 
-  attrs = url.getAttributes({});
-  do_check_eq(attrs.length, newAttrs.length - 1);
-  for (i = 0; i < newAttrs.length - 1; ++i)
-    do_check_eq(attrs[i], newAttrs[i + 1]);
-
-  do_check_eq(url.spec, "ldap://localhost" + portAdpt + "/dc=short?def,ghi,jkl?one?(objectclass=*)");
+  do_check_eq(url.attributes, newAttrs);
+  do_check_eq(url.spec,
+              "ldap://localhost" + portAdpt + "/dc=short?" +
+              newAttrs + "?one?(objectclass=*)");
 
   // This shouldn't fail, just clear the list
-  url.setAttributes(0, []);
+  url.attributes = "";
 
-  attrs = url.getAttributes({});
-  do_check_eq(attrs.length, 0);
+  do_check_eq(url.attributes.length, 0);
   do_check_eq(url.spec, "ldap://localhost" + portAdpt + "/dc=short??one?(objectclass=*)");
 
   // Set attributes via the url spec
 
-  url.spec = "ldap://localhost/dc=short?abc,def,ghi,jkl?one?(objectclass=*)";
+  newAttrs = "abc,def,ghi,jkl";
+  url.spec = "ldap://localhost/dc=short?" + newAttrs + "?one?(objectclass=*)";
 
-  attrs = url.getAttributes({});
-  do_check_eq(attrs.length, newAttrs.length);
-  for (i = 0; i < newAttrs.length; ++i)
-    do_check_eq(attrs[i], newAttrs[i]);
-
-  do_check_eq(url.spec, "ldap://localhost/dc=short?abc,def,ghi,jkl?one?(objectclass=*)");
+  do_check_eq(url.attributes, newAttrs);
+  do_check_eq(url.spec,
+              "ldap://localhost/dc=short?" + newAttrs + "?one?(objectclass=*)");
 
   url.spec = "ldap://localhost/dc=short??one?(objectclass=*)";
 
-  attrs = url.getAttributes({});
+  attrs = url.attributes;
   do_check_eq(attrs.length, 0);
   do_check_eq(url.spec, "ldap://localhost/dc=short??one?(objectclass=*)");
 
   // Test - clone
 
   url.spec = "ldap://localhost/dc=short?abc,def,ghi,jkl?one?(objectclass=*)";
 
   var newUrl = url.clone();
--- a/mailnews/addrbook/public/nsIAbLDAPAttributeMap.idl
+++ b/mailnews/addrbook/public/nsIAbLDAPAttributeMap.idl
@@ -62,17 +62,17 @@ interface nsIAbCard;
  * property.  The checkState method is a useful tool in enforcing
  * this.  Failure to enforce it may make it impossible to guarantee
  * that getProperty will do something consistent and reasonable.
  *
  * Maybe someday once we support ldap autoconfig stuff (ie
  * draft-joslin-config-schema-11.txt), we can simplify this and other
  * code and only allow a property to map to a single attribute.
  */
-[scriptable, uuid(dbfb9974-45ee-41c3-a005-2e8f002c1c9f)]
+[scriptable, uuid(fa019fd1-7f3d-417a-8957-154cca0240be)]
 interface nsIAbLDAPAttributeMap : nsISupports
 {
   /**
    * Get all the LDAP attributes associated with a given property
    * name, in order of precedence (highest to lowest).
    *
    * @param       aProperty   the address book property to return attrs for   
    *    
@@ -134,22 +134,21 @@ interface nsIAbLDAPAttributeMap : nsISup
    */
   ACString getProperty(in ACString aAttribute);
 
   /**
    * Get all attributes that may be used in an addressbook card via this
    * property map (used for passing to to an LDAP search when you want
    * everything that could be in a card returned).
    *
-   * @return                      an array of attribute names
+   * @return                      a comma-separated list of attribute names
    *
    * @exception NS_ERROR_FAILURE  there are no attributes in this property map
    */
-  void getAllCardAttributes(out unsigned long aCount,
-                            [retval, array, size_is(aCount)] out string aAttrs);
+  ACString getAllCardAttributes();
   
   /**
    * Get all properties that may be used in an addressbook card via this
    * property map.
    *
    * @return                      an array of properties
    *
    * @exception NS_ERROR_FAILURE  there are no attributes in this property map
--- a/mailnews/addrbook/public/nsILDAPAutoCompFormatter.idl
+++ b/mailnews/addrbook/public/nsILDAPAutoCompFormatter.idl
@@ -41,17 +41,17 @@
 
 interface nsIAutoCompleteItem;
 interface nsILDAPMessage;
 
 /**
  * An interface to allow different users of nsILDAPAutoCompleteSession to 
  * format each nsILDAPMessage into an nsIAutoCompleteItem as it sees fit.
  */
-[scriptable, uuid(cbec617c-22d2-4286-8308-0ef8bf57c0ce)]
+[scriptable, uuid(450cb44e-1752-49d0-890a-f895f8163cc3)]
 interface nsILDAPAutoCompFormatter : nsISupports {
     
     /**
      * Returns an nsIAutoCompleteItem generated from the data in 
      * the given nsILDAPMessage.
      *
      * @param aMessage  message to be formatted
      * @return          resulting nsIAutoCompleteItem
@@ -68,21 +68,19 @@ interface nsILDAPAutoCompFormatter : nsI
      * nsILDAPAutoCompleteSession implementation when the
      * nsILDAPAutoCompleteSession::formatter IDL attribute is set.  .
      * So if for some reason, the LDAP attributes to be returned by
      * searches has to change (eg because the user changed a
      * preference), the nsILDAPAutoCompleteSession::formatter IDL
      * attribute should be re-set to the same object to force a new
      * getAttributes() call.
      * 
-     * @param aCount    number of attributes in the array
-     * @param aAttrs    list of LDAP attributes to request
+     * @return  A comma-separated list of attributes.
      */
-    void getAttributes(out unsigned long aCount, 
-                       [retval, array, size_is(aCount)] out string aAttrs);
+    readonly attribute ACString attributes;
 
     /** 
      * This method formats an error condition into an nsIAutoCompleteItem
      * for display to the user.  Specifically, the state that the session
      * was in when the error occured (aState) is formatted into a general
      * error message which is put in the value attribute of the item,
      * and the specific error (aErrorCode) is formatted into another message
      * which is put in an nsISupportsString in the param attribute of the
--- a/mailnews/addrbook/src/nsAbLDAPAttributeMap.js
+++ b/mailnews/addrbook/src/nsAbLDAPAttributeMap.js
@@ -118,32 +118,31 @@ nsAbLDAPAttributeMap.prototype = {
 
     if (!(aAttribute in this.mAttrMap)) {
       return null;
     }
 
     return this.mAttrMap[aAttribute];
   },
 
-  getAllCardAttributes: function getAllCardAttributes(aCount) {
+  getAllCardAttributes: function getAllCardAttributes() {
     var attrs = [];
     for each (var attrArray in this.mPropertyMap) {
       attrs = attrs.concat(attrArray);
     }
 
     if (!attrs.length) {
       throw Components.results.NS_ERROR_FAILURE;
     }
 
-    aCount.value = attrs.length;
-    return attrs;
+    return attrs.join(",");
   },
-  
+
   getAllCardProperties: function getAllCardProperties(aCount) {
-    
+
     var props = [];
     for (var prop in this.mPropertyMap) {
       props.push(prop);
     }
 
     aCount.value = props.length;
     return props;
   },
@@ -185,17 +184,17 @@ nsAbLDAPAttributeMap.prototype = {
 
       // go through the list of possible attrs in precedence order
       for each (var attr in this.mPropertyMap[prop]) {
 
         attr = attr.toLowerCase();
 
         // find the first attr that exists in this message
         if (msgAttrs.indexOf(attr) != -1) {
-          
+
           try {
             var values = aMessage.getValues(attr, {});
             // strip out the optional label from the labeledURI
             if (attr == "labeleduri" && values[0]) {
               var index = values[0].indexOf(" ");
               if (index != -1)
                 values[0] = values[0].substring(0, index);
             }
@@ -213,17 +212,17 @@ nsAbLDAPAttributeMap.prototype = {
     if (!cardValueWasSet) {
       throw Components.results.NS_ERROR_FAILURE;
     }
 
     return;
   },
 
   checkState: function checkState() {
-    
+
     var attrsSeen = [];
 
     for each (var attrArray in this.mPropertyMap) {
 
       for each (var attr in attrArray) {
 
         // multiple attributes that mapped to the empty string are permitted
         if (!attr.length) {
@@ -249,17 +248,17 @@ nsAbLDAPAttributeMap.prototype = {
 
 function nsAbLDAPAttributeMapService() {
 }
 
 nsAbLDAPAttributeMapService.prototype = {
 
   classID: NS_ABLDAPATTRIBUTEMAPSERVICE_CID,
 
-  mAttrMaps: {}, 
+  mAttrMaps: {},
 
   getMapForPrefBranch: function getMapForPrefBranch(aPrefBranchName) {
 
     // if we've already got this map, return it
     if (aPrefBranchName in this.mAttrMaps) {
       return this.mAttrMaps[aPrefBranchName];
     }
 
--- a/mailnews/addrbook/src/nsAbLDAPAutoCompFormatter.cpp
+++ b/mailnews/addrbook/src/nsAbLDAPAutoCompFormatter.cpp
@@ -352,22 +352,18 @@ nsAbLDAPAutoCompFormatter::FormatExcepti
 
     // all done; return the item
     //
     NS_IF_ADDREF(*aItem = item);
     return NS_OK;
 }
 
 NS_IMETHODIMP
-nsAbLDAPAutoCompFormatter::GetAttributes(PRUint32 *aCount, char ** *aAttrs)
+nsAbLDAPAutoCompFormatter::GetAttributes(nsACString &aResult)
 {
-    if (!aCount || !aAttrs) {
-        return NS_ERROR_INVALID_POINTER;
-    }
-
     nsCStringArray mSearchAttrs;
     nsresult rv = ProcessFormat(mNameFormat, 0, 0, &mSearchAttrs);
     if (NS_FAILED(rv)) {
         NS_WARNING("nsAbLDAPAutoCompFormatter::SetNameFormat(): "
                    "ProcessFormat() failed");
         return rv;
     }
     rv = ProcessFormat(mAddressFormat, 0, 0, &mSearchAttrs);
@@ -388,47 +384,27 @@ nsAbLDAPAutoCompFormatter::GetAttributes
     PRUint32 count = mSearchAttrs.Count();   // size of XPCOM array we'll need
     if (!count) {
         NS_ERROR("nsAbLDAPAutoCompFormatter::GetAttributes(): "
                  "current output format (if set) requires no search "
                  "attributes");
         return NS_ERROR_NOT_INITIALIZED;
     }
 
-    // build up the raw XPCOM array to return
-    //
-    PRUint32 rawSearchAttrsSize = 0;        // grown as XPCOM array is built
-    char **rawSearchAttrs = 
-        static_cast<char **>(nsMemory::Alloc(count * sizeof(char *)));
-    if (!rawSearchAttrs) {
-        NS_ERROR("nsAbLDAPAutoCompFormatter::GetAttributes(): out of "
-		 "memory");
-        return NS_ERROR_OUT_OF_MEMORY;
+    aResult = *mSearchAttrs.CStringAt(0);
+
+    for (PRUint32 i = 1; i < count; i++)
+    {
+      aResult.Append(',');
+      aResult.Append(*mSearchAttrs.CStringAt(i));
     }
 
-    // Loop through the string array, and build up the C-array.
-    //
-    while (rawSearchAttrsSize < count) {
-        if (!(rawSearchAttrs[rawSearchAttrsSize] = 
-              ToNewCString(*(mSearchAttrs.CStringAt(rawSearchAttrsSize))))) {
-            NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(rawSearchAttrsSize, 
-                                                  rawSearchAttrs);
-            NS_ERROR("nsAbLDAPAutoCompFormatter::GetAttributes(): out "
-		     "of memory");
-            return NS_ERROR_OUT_OF_MEMORY;
-        }
-        rawSearchAttrsSize++;
-    }
+    return NS_OK;
+}
 
-    *aCount = rawSearchAttrsSize;
-    *aAttrs = rawSearchAttrs;
-
-    return NS_OK;
-
-}
 // parse and process a formatting attribute.  If aStringArray is
 // non-NULL, return a list of the attributes from mNameFormat in
 // aStringArray.  Otherwise, generate an autocomplete value from the
 // information in aMessage and append it to aValue.  Any errors
 // (including failure to find a required attribute while building up aValue) 
 // return an NS_ERROR_* up the stack so that the caller doesn't try and
 // generate an nsIAutoCompleteItem from this.
 //
--- a/mailnews/addrbook/src/nsAbLDAPDirectoryQuery.cpp
+++ b/mailnews/addrbook/src/nsAbLDAPDirectoryQuery.cpp
@@ -262,29 +262,27 @@ nsresult nsAbQueryLDAPMessageListener::D
   PRInt32 scope;
   rv = mSearchUrl->GetScope(&scope);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCAutoString filter;
   rv = mSearchUrl->GetFilter(filter);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  CharPtrArrayGuard attributes;
-  rv = mSearchUrl->GetAttributes(attributes.GetSizeAddr(),
-                                 attributes.GetArrayAddr());
+  nsCAutoString attributes;
+  rv = mSearchUrl->GetAttributes(attributes);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = mOperation->SetServerControls(mServerSearchControls);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = mOperation->SetClientControls(mClientSearchControls);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  return mOperation->SearchExt(dn, scope, filter, attributes.GetSize(),
-                               attributes.GetArray(), mTimeOut,
+  return mOperation->SearchExt(dn, scope, filter, attributes, mTimeOut,
                                mResultLimit);
 }
 
 void nsAbQueryLDAPMessageListener::InitFailed(PRBool aCancelled)
 {
   if (!mResultListener)
     return;
 
@@ -453,31 +451,27 @@ NS_IMETHODIMP nsAbLDAPDirectoryQuery::Do
   nsCOMPtr<nsISupports> iSupportsMap;
   rv = aArguments->GetTypeSpecificArg(getter_AddRefs(iSupportsMap));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIAbLDAPAttributeMap> map = do_QueryInterface(iSupportsMap, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Require all attributes that are mapped to card properties
-  PRUint32 returnAttrsCount;
-  char** returnAttrsArray;
-  rv = map->GetAllCardAttributes(&returnAttrsCount, &returnAttrsArray);
+  nsCAutoString returnAttributes;
+  rv = map->GetAllCardAttributes(returnAttributes);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = url->SetAttributes(returnAttrsCount,
-                          const_cast<const char**>(returnAttrsArray));
-  // First free the array
-  NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(returnAttrsCount, returnAttrsArray);
+  rv = url->SetAttributes(returnAttributes);
   // Now do the error check
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Also require the objectClass attribute, it is used by
   // nsAbLDAPCard::SetMetaProperties
-  rv = url->AddAttribute("objectClass");
+  rv = url->AddAttribute(NS_LITERAL_CSTRING("objectClass"));
 
   // Get the filter
   nsCOMPtr<nsISupports> supportsExpression;
   rv = aArguments->GetExpression (getter_AddRefs (supportsExpression));
   NS_ENSURE_SUCCESS(rv, rv);
   
   nsCOMPtr<nsIAbBooleanExpression> expression (do_QueryInterface (supportsExpression, &rv));
   nsCAutoString filter;
--- a/mailnews/addrbook/src/nsAbLDAPReplicationData.cpp
+++ b/mailnews/addrbook/src/nsAbLDAPReplicationData.cpp
@@ -242,31 +242,28 @@ nsresult nsAbLDAPProcessReplicationData:
   if (dn.IsEmpty())
     return NS_ERROR_UNEXPECTED;
 
   PRInt32 scope;
   rv = mDirectoryUrl->GetScope(&scope);
   if (NS_FAILED(rv))
     return rv;
 
-  CharPtrArrayGuard attributes;
-  rv = mDirectoryUrl->GetAttributes(attributes.GetSizeAddr(),
-                                    attributes.GetArrayAddr());
+  nsCAutoString attributes;
+  rv = mDirectoryUrl->GetAttributes(attributes);
   if (NS_FAILED(rv))
     return rv;
 
   mState = kReplicatingAll;
 
   if (mListener && NS_SUCCEEDED(rv))
     mListener->OnStateChange(nsnull, nsnull,
                              nsIWebProgressListener::STATE_START, PR_TRUE);
 
-  return mOperation->SearchExt(dn, scope, urlFilter,
-                               attributes.GetSize(), attributes.GetArray(),
-                               0, 0);
+  return mOperation->SearchExt(dn, scope, urlFilter, attributes, 0, 0);
 }
 
 void nsAbLDAPProcessReplicationData::InitFailed(PRBool aCancelled)
 {
   // Just call Done() which will ensure everything is tidied up nicely.
   Done(PR_FALSE);
 }
 
--- a/mailnews/addrbook/src/nsLDAPAutoCompleteSession.cpp
+++ b/mailnews/addrbook/src/nsLDAPAutoCompleteSession.cpp
@@ -69,25 +69,22 @@ static PRLogModuleInfo *sLDAPAutoComplet
 NS_IMPL_THREADSAFE_ISUPPORTS3(nsLDAPAutoCompleteSession, 
                               nsIAutoCompleteSession, nsILDAPMessageListener,
                               nsILDAPAutoCompleteSession)
 
 nsLDAPAutoCompleteSession::nsLDAPAutoCompleteSession() :
     mState(UNBOUND), 
     mFilterTemplate("(|(cn=%v1*%v2-*)(mail=%v1*%v2-*)(sn=%v1*%v2-*))"),
     mMaxHits(100), mMinStringLength(2), mCjkMinStringLength(0), 
-    mSearchAttrs(0), mSearchAttrsSize(0), mVersion(nsILDAPConnection::VERSION3)
+    mVersion(nsILDAPConnection::VERSION3)
 {
 }
 
 nsLDAPAutoCompleteSession::~nsLDAPAutoCompleteSession()
 {
-    if (mSearchAttrs) {
-        NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(mSearchAttrsSize, mSearchAttrs);
-    }
     NS_IF_RELEASE(mConnection);
 }
 
 #define IS_CJK_CHAR_FOR_LDAP(u)  (0x2e80 <= (u) && (u) <= 0xd7ff)
 
 /* void onStartLookup (in wstring searchString, in nsIAutoCompleteResults previousSearchResult, in nsIAutoCompleteListener listener); */
 NS_IMETHODIMP 
 nsLDAPAutoCompleteSession::OnStartLookup(const PRUnichar *searchString, 
@@ -752,19 +749,18 @@ nsLDAPAutoCompleteSession::DoTask()
                                  BOUND);
         return NS_ERROR_UNEXPECTED;
     }
 
     // Time to kick off the search.
     //
     // XXXdmose what about timeouts? 
     //
-    rv = mOperation->SearchExt(dn, scope, searchFilter, mSearchAttrsSize,
-                               const_cast<const char **>(mSearchAttrs),
-                               0, mMaxHits);
+    rv = mOperation->SearchExt(dn, scope, searchFilter, mSearchAttrs, 0,
+                               mMaxHits);
     if (NS_FAILED(rv)) {
         switch(rv) {
 
         case NS_ERROR_LDAP_ENCODING_ERROR:
             PR_LOG(sLDAPAutoCompleteLogModule, PR_LOG_DEBUG, 
                    ("nsLDAPAutoCompleteSession::DoTask(): SearchExt "
                     "returned NS_ERROR_LDAP_ENCODING_ERROR"));
             FinishAutoCompleteLookup(nsIAutoCompleteStatus::failureItems, rv, 
@@ -1211,24 +1207,18 @@ nsLDAPAutoCompleteSession::SetFormatter(
     }
 
     NS_ASSERTION(mState == UNBOUND || mState == BOUND, 
                  "nsLDAPAutoCompleteSession::SetFormatter was called when "
                  "mState was set to something other than BOUND or UNBOUND");
 
     mFormatter = aFormatter;
 
-    // Ensure any old data is freed if necessary.
-    if (mSearchAttrs) {
-        NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(mSearchAttrsSize, mSearchAttrs);
-        mSearchAttrs = nsnull;
-    }
-
     // Get and cache the attributes that will be used to do lookups.
-    nsresult rv = mFormatter->GetAttributes(&mSearchAttrsSize, &mSearchAttrs);
+    nsresult rv = mFormatter->GetAttributes(mSearchAttrs);
     if (NS_FAILED(rv)) {
         NS_ERROR("nsLDAPAutoCompleteSession::SetFormatter(): "
                  " mFormatter->GetAttributes failed");
         return NS_ERROR_FAILURE;
     }
     
     return NS_OK;
 }
--- a/mailnews/addrbook/src/nsLDAPAutoCompleteSession.h
+++ b/mailnews/addrbook/src/nsLDAPAutoCompleteSession.h
@@ -88,18 +88,17 @@ protected:
     nsCOMPtr<nsIAutoCompleteListener> mListener;  // callback 
     nsCOMPtr<nsIAutoCompleteResults> mResults;    // being built up
     nsCOMPtr<nsISupportsArray> mResultsArray;     // cached, to avoid re-gets
     nsString mSearchString;                       // autocomplete this string
     nsCString mFilterTemplate;                    // search filter template
     PRInt32 mMaxHits;                       // return at most this many entries
     PRUint32 mMinStringLength;              // strings < this size are ignored
     PRUint32 mCjkMinStringLength;           // ignore CJK strings < this size
-    char **mSearchAttrs;        // outputFormat search attrs for SearchExt call
-    PRUint32 mSearchAttrsSize;              // size of above array
+    nsCString mSearchAttrs;     // outputFormat search attrs for SearchExt call
     PRUint32 mVersion;                      // version of LDAP to use
 
     // used to format the ldap message into an nsIAutoCompleteItem
     //
     nsCOMPtr<nsILDAPAutoCompFormatter> mFormatter;
 
     // stopgap until nsLDAPService works
     nsresult InitConnection();