aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--addressbook/ChangeLog29
-rw-r--r--addressbook/backend/pas/pas-backend-ldap.c201
2 files changed, 156 insertions, 74 deletions
diff --git a/addressbook/ChangeLog b/addressbook/ChangeLog
index bfccc93641..df209424be 100644
--- a/addressbook/ChangeLog
+++ b/addressbook/ChangeLog
@@ -1,3 +1,32 @@
+2002-08-26 Chris Toshok <toshok@ximian.com>
+
+ [ Fixes (almost certainly) #24649, #25494, #27351, and other LDAP search crashes ]
+ * backend/pas/pas-backend-ldap.c (view_destroy): use an EList
+ instead of a GList to store the book_view's so we don't have weird
+ issues with modifying the list while it's being traversed.
+ (find_book_view): same.
+ (create_card_handler): same.
+ (remove_card_handler): same.
+ (modify_card_modify_handler): same.
+ (poll_ldap): same, and also ref the book_view before calling
+ ldap_search_op_timeout (and therefore send_pending_adds).
+ (ldap_search_handler): same.
+ (ldap_op_add): warn about conflicting ldap msgid's (shouldn't ever
+ happen..)
+ (homephone_populate): make this a bit more robust (if values[0] ==
+ NULL, values[1] won't be valid).
+ (business_populate): same.
+ (build_card_from_entry): break out of the prop_info loop when we
+ get a match, and only set the simple field if the value != NULL.
+ (ldap_search_dtor): free all the pending adds stuff.
+ (pas_backend_ldap_process_get_book_view): g_list_prepend =>
+ e_list_append.
+ (pas_backend_ldap_remove_client): simplify the removing of the
+ book (use g_list_remove instead of searching and then using
+ g_list_remove_link.)
+ (pas_backend_ldap_destroy): unref the book_views list.
+ (pas_backend_ldap_init): initialize the EList for book_views.
+
2002-08-25 Mike Kestner <mkestner@ximian.com>
* gui/widgets/e-addressbook-view.c (remove_book_view): stop the
diff --git a/addressbook/backend/pas/pas-backend-ldap.c b/addressbook/backend/pas/pas-backend-ldap.c
index 2d30ef508c..bf8646fec5 100644
--- a/addressbook/backend/pas/pas-backend-ldap.c
+++ b/addressbook/backend/pas/pas-backend-ldap.c
@@ -107,7 +107,7 @@ struct _PASBackendLDAPPrivate {
was not built into openldap. */
PASBackendLDAPUseTLS use_tls;
- GList *book_views;
+ EList *book_views;
LDAP *ldap;
@@ -291,44 +291,59 @@ remove_view (int msgid, LDAPOp *op, PASBookView *view)
static void
view_destroy(GtkObject *object, gpointer data)
{
- CORBA_Environment ev;
- GNOME_Evolution_Addressbook_Book corba_book;
PASBook *book = (PASBook *)data;
PASBackendLDAP *bl;
- GList *list;
+ EIterator *iter;
bl = PAS_BACKEND_LDAP(pas_book_get_backend(book));
- for (list = bl->priv->book_views; list; list = g_list_next(list)) {
- PASBackendLDAPBookView *view = list->data;
+
+ iter = e_list_get_iterator (bl->priv->book_views);
+
+ while (e_iterator_is_valid (iter)) {
+ PASBackendLDAPBookView *view = (PASBackendLDAPBookView*)e_iterator_get (iter);
+
if (view->book_view == PAS_BOOK_VIEW(object)) {
+ GNOME_Evolution_Addressbook_Book corba_book;
+ CORBA_Environment ev;
+
/* if we have an active search, interrupt it */
- if (view->search_op)
+ if (view->search_op) {
ldap_op_finished (view->search_op);
+ }
/* and remove us as the view for any other
operations that might be using us to spew
status messages to the gui */
g_hash_table_foreach (bl->priv->id_to_op, (GHFunc)remove_view, view->book_view);
+
+ /* free up the view structure */
g_free (view->search);
gtk_object_unref (GTK_OBJECT (view->card_sexp));
g_free (view);
- bl->priv->book_views = g_list_remove_link(bl->priv->book_views, list);
- g_list_free_1(list);
- break;
- }
- }
- corba_book = bonobo_object_corba_objref(BONOBO_OBJECT(book));
+ /* and remove it from our list */
+ e_iterator_delete (iter);
- CORBA_exception_init(&ev);
+ /* unref the book now */
+ corba_book = bonobo_object_corba_objref(BONOBO_OBJECT(book));
+
+ CORBA_exception_init(&ev);
- GNOME_Evolution_Addressbook_Book_unref(corba_book, &ev);
+ GNOME_Evolution_Addressbook_Book_unref(corba_book, &ev);
- if (ev._major != CORBA_NO_EXCEPTION) {
- g_warning("view_destroy: Exception unreffing "
- "corba book.\n");
+ if (ev._major != CORBA_NO_EXCEPTION) {
+ g_warning("view_destroy: Exception unreffing "
+ "corba book.\n");
+ }
+
+ CORBA_exception_free(&ev);
+ break;
+ }
+
+ e_iterator_next (iter);
}
- CORBA_exception_free(&ev);
+ gtk_object_unref (GTK_OBJECT (iter));
+
}
static void
@@ -342,14 +357,19 @@ book_view_notify_status (PASBookView *view, const char *status)
static PASBookView*
find_book_view (PASBackendLDAP *bl)
{
- /* just always use the first book view */
- if (bl->priv->book_views) {
- PASBackendLDAPBookView *v = bl->priv->book_views->data;
- return v->book_view;
- }
- else {
- return NULL;
+ EIterator *iter = e_list_get_iterator (bl->priv->book_views);
+ PASBookView *rv = NULL;
+
+ if (e_iterator_is_valid (iter)) {
+ /* just always use the first book view */
+ PASBackendLDAPBookView *v = (PASBackendLDAPBookView*)e_iterator_get(iter);
+ if (v)
+ rv = v->book_view;
}
+
+ gtk_object_unref (GTK_OBJECT (iter));
+
+ return rv;
}
static void
@@ -688,6 +708,10 @@ ldap_op_add (LDAPOp *op, PASBackend *backend,
op->handler = handler;
op->dtor = dtor;
+ if (g_hash_table_lookup (bl->priv->id_to_op, &op->id)) {
+ g_warning ("conflicting ldap msgid's");
+ }
+
g_hash_table_insert (bl->priv->id_to_op,
&op->id, op);
@@ -713,6 +737,7 @@ ldap_op_finished (LDAPOp *op)
op->dtor (op);
bl->priv->active_ops--;
+
if (bl->priv->active_ops == 0) {
if (bl->priv->poll_timeout != -1)
g_source_remove (bl->priv->poll_timeout);
@@ -1040,11 +1065,13 @@ create_card_handler (LDAPOp *op, LDAPMessage *res)
if (ldap_error == LDAP_SUCCESS) {
/* the card was created, let's let the views know about it */
- GList *l;
- for (l = bl->priv->book_views; l; l = l->next) {
+ EIterator *iter;
+
+ iter = e_list_get_iterator (bl->priv->book_views);
+ while (e_iterator_is_valid (iter)) {
CORBA_Environment ev;
gboolean match;
- PASBackendLDAPBookView *view = l->data;
+ PASBackendLDAPBookView *view = (PASBackendLDAPBookView*)e_iterator_get (iter);
char *new_vcard;
CORBA_exception_init(&ev);
@@ -1064,7 +1091,10 @@ create_card_handler (LDAPOp *op, LDAPMessage *res)
g_free (new_vcard);
bonobo_object_release_unref(bonobo_object_corba_objref(BONOBO_OBJECT(view->book_view)), &ev);
+
+ e_iterator_next (iter);
}
+ gtk_object_unref (GTK_OBJECT (iter));
}
else {
ldap_perror (ldap, "create_card");
@@ -1237,10 +1267,11 @@ remove_card_handler (LDAPOp *op, LDAPMessage *res)
if (ldap_error == LDAP_SUCCESS) {
/* the card was removed, let's let the views know about it */
- GList *l;
- for (l = bl->priv->book_views; l; l = l->next) {
+ EIterator *iter = e_list_get_iterator (bl->priv->book_views);
+
+ while (e_iterator_is_valid (iter)) {
CORBA_Environment ev;
- PASBackendLDAPBookView *view = l->data;
+ PASBackendLDAPBookView *view = (PASBackendLDAPBookView*)e_iterator_get (iter);
CORBA_exception_init(&ev);
@@ -1249,7 +1280,10 @@ remove_card_handler (LDAPOp *op, LDAPMessage *res)
pas_book_view_notify_remove (view->book_view, remove_op->id);
bonobo_object_release_unref(bonobo_object_corba_objref(BONOBO_OBJECT(view->book_view)), &ev);
+
+ e_iterator_next (iter);
}
+ gtk_object_unref (GTK_OBJECT (iter));
}
else {
ldap_perror (bl->priv->ldap, "remove_card");
@@ -1351,11 +1385,11 @@ modify_card_modify_handler (LDAPOp *op, LDAPMessage *res)
if (ldap_error == LDAP_SUCCESS) {
/* the card was modified, let's let the views know about it */
- GList *l;
- for (l = bl->priv->book_views; l; l = l->next) {
+ EIterator *iter = e_list_get_iterator (bl->priv->book_views);
+ while (e_iterator_is_valid (iter)) {
CORBA_Environment ev;
gboolean old_match, new_match;
- PASBackendLDAPBookView *view = l->data;
+ PASBackendLDAPBookView *view = (PASBackendLDAPBookView*)e_iterator_get (iter);
CORBA_exception_init(&ev);
@@ -1374,7 +1408,10 @@ modify_card_modify_handler (LDAPOp *op, LDAPMessage *res)
pas_book_view_notify_complete (view->book_view, GNOME_Evolution_Addressbook_BookViewListener_Success);
bonobo_object_release_unref(bonobo_object_corba_objref(BONOBO_OBJECT(view->book_view)), &ev);
+
+ e_iterator_next (iter);
}
+ gtk_object_unref (GTK_OBJECT (iter));
}
else {
ldap_perror (ldap, "ldap_modify_s");
@@ -1902,10 +1939,11 @@ email_compare (ECardSimple *ecard1, ECardSimple *ecard2)
static void
homephone_populate(ECardSimple *card, char **values)
{
- if (values[0])
+ if (values[0]) {
e_card_simple_set (card, E_CARD_SIMPLE_FIELD_PHONE_HOME, values[0]);
- if (values[1])
- e_card_simple_set (card, E_CARD_SIMPLE_FIELD_PHONE_HOME_2, values[1]);
+ if (values[1])
+ e_card_simple_set (card, E_CARD_SIMPLE_FIELD_PHONE_HOME_2, values[1]);
+ }
}
struct berval**
@@ -1969,10 +2007,11 @@ homephone_compare (ECardSimple *ecard1, ECardSimple *ecard2)
static void
business_populate(ECardSimple *card, char **values)
{
- if (values[0])
+ if (values[0]) {
e_card_simple_set (card, E_CARD_SIMPLE_FIELD_PHONE_BUSINESS, values[0]);
- if (values[1])
- e_card_simple_set (card, E_CARD_SIMPLE_FIELD_PHONE_BUSINESS_2, values[1]);
+ if (values[1])
+ e_card_simple_set (card, E_CARD_SIMPLE_FIELD_PHONE_BUSINESS_2, values[1]);
+ }
}
struct berval**
@@ -2568,7 +2607,7 @@ typedef struct {
static ECardSimple *
build_card_from_entry (LDAP *ldap, LDAPMessage *e, GList **existing_objectclasses)
{
- ECard *ecard = E_CARD(gtk_type_new(e_card_get_type()));
+ ECard *ecard = e_card_new ("");
ECardSimple *card = e_card_simple_new (ecard);
char *dn;
char *attr;
@@ -2593,8 +2632,10 @@ build_card_from_entry (LDAP *ldap, LDAPMessage *e, GList **existing_objectclasse
}
else {
for (i = 0; i < num_prop_infos; i ++)
- if (!g_strcasecmp (attr, prop_info[i].ldap_attr))
+ if (!g_strcasecmp (attr, prop_info[i].ldap_attr)) {
info = &prop_info[i];
+ break;
+ }
if (info) {
values = ldap_get_values (ldap, e, attr);
@@ -2602,7 +2643,8 @@ build_card_from_entry (LDAP *ldap, LDAPMessage *e, GList **existing_objectclasse
if (values) {
if (info->prop_type & PROP_TYPE_STRING) {
/* if it's a normal property just set the string */
- e_card_simple_set (card, info->field_id, values[0]);
+ if (values[0])
+ e_card_simple_set (card, info->field_id, values[0]);
}
else if (info->prop_type & PROP_TYPE_COMPLEX) {
@@ -2620,11 +2662,6 @@ build_card_from_entry (LDAP *ldap, LDAPMessage *e, GList **existing_objectclasse
ldap_memfree (attr);
}
- /* XXX the openldap man page for
- ldap_first_attribute/ldap_next_attribute says that the ber
- is freed if there are no more attributes
- (ldap_next_attribute returns NULL), but this seems to not
- be the case (as of openldap-2.0.23), so free it here. */
if (ber)
ber_free (ber, 0);
@@ -2638,13 +2675,18 @@ build_card_from_entry (LDAP *ldap, LDAPMessage *e, GList **existing_objectclasse
static gboolean
poll_ldap (PASBackendLDAP *bl)
{
- GList *list;
LDAP *ldap = bl->priv->ldap;
int rc;
LDAPMessage *res;
GTimeVal cur_time;
glong cur_millis;
struct timeval timeout;
+ EIterator *iter;
+
+ if (!bl->priv->active_ops) {
+ g_warning ("poll_ldap being called for backend with no active operations");
+ return FALSE;
+ }
timeout.tv_sec = 0;
timeout.tv_usec = LDAP_RESULT_TIMEOUT_MILLIS * 1000;
@@ -2666,10 +2708,11 @@ poll_ldap (PASBackendLDAP *bl)
LDAPOp *op;
op = g_hash_table_lookup (bl->priv->id_to_op, &msgid);
- if (!op)
- g_error ("unknown operation, msgid = %d", msgid);
- op->handler (op, res);
+ if (op)
+ op->handler (op, res);
+ else
+ g_warning ("unknown operation, msgid = %d", msgid);
ldap_msgfree(res);
}
@@ -2678,11 +2721,19 @@ poll_ldap (PASBackendLDAP *bl)
g_get_current_time (&cur_time);
cur_millis = TV_TO_MILLIS (cur_time);
- for (list = bl->priv->book_views; list; list = g_list_next(list)) {
- PASBackendLDAPBookView *view = list->data;
- if (view->search_op)
+ iter = e_list_get_iterator (bl->priv->book_views);
+ while (e_iterator_is_valid (iter)) {
+ PASBackendLDAPBookView *view = (PASBackendLDAPBookView *)e_iterator_get (iter);
+ if (view->search_op) {
+ bonobo_object_dup_ref(bonobo_object_corba_objref(BONOBO_OBJECT(view->book_view)), NULL);
+
ldap_search_op_timeout (view->search_op, cur_millis);
+
+ bonobo_object_release_unref(bonobo_object_corba_objref(BONOBO_OBJECT(view->book_view)), NULL);
+ }
+ e_iterator_next (iter);
}
+ gtk_object_unref (GTK_OBJECT (iter));
return TRUE;
}
@@ -2734,11 +2785,14 @@ static void
ldap_search_handler (LDAPOp *op, LDAPMessage *res)
{
LDAPSearchOp *search_op = (LDAPSearchOp*)op;
+ PASBackendLDAPBookView *view = search_op->view;
PASBackendLDAP *bl = PAS_BACKEND_LDAP (op->backend);
LDAP *ldap = bl->priv->ldap;
LDAPMessage *e;
int msg_type;
+ bonobo_object_dup_ref(bonobo_object_corba_objref(BONOBO_OBJECT(view->book_view)), NULL);
+
if (!search_op->notified_receiving_results) {
search_op->notified_receiving_results = TRUE;
book_view_notify_status (op->view, _("Receiving LDAP search results..."));
@@ -2790,6 +2844,9 @@ ldap_search_handler (LDAPOp *op, LDAPMessage *res)
pas_book_view_notify_complete (search_op->op.view, GNOME_Evolution_Addressbook_BookViewListener_OtherError);
ldap_op_finished (op);
}
+
+
+ bonobo_object_release_unref(bonobo_object_corba_objref(BONOBO_OBJECT(view->book_view)), NULL);
}
static void
@@ -2801,6 +2858,11 @@ ldap_search_dtor (LDAPOp *op)
if (search_op->view)
search_op->view->search_op = NULL;
+ g_list_foreach (search_op->pending_adds, (GFunc)g_free, NULL);
+ g_list_free (search_op->pending_adds);
+ search_op->pending_adds = NULL;
+ search_op->num_pending_adds = 0;
+
g_free (search_op);
}
@@ -2900,7 +2962,7 @@ pas_backend_ldap_process_get_book_view (PASBackend *backend,
view->limit = bl->priv->ldap_limit;
}
- bl->priv->book_views = g_list_prepend(bl->priv->book_views, view);
+ e_list_append(bl->priv->book_views, view);
pas_book_respond_get_book_view (book,
(book_view != NULL
@@ -3210,8 +3272,6 @@ pas_backend_ldap_remove_client (PASBackend *backend,
PASBook *book)
{
PASBackendLDAP *bl;
- GList *l;
- PASBook *lbook;
g_return_if_fail (backend != NULL);
g_return_if_fail (PAS_IS_BACKEND_LDAP (backend));
@@ -3220,21 +3280,8 @@ pas_backend_ldap_remove_client (PASBackend *backend,
bl = PAS_BACKEND_LDAP (backend);
- /* Find the book in the list of clients */
-
- for (l = bl->priv->clients; l; l = l->next) {
- lbook = PAS_BOOK (l->data);
-
- if (lbook == book)
- break;
- }
-
- g_assert (l != NULL);
-
/* Disconnect */
-
- bl->priv->clients = g_list_remove_link (bl->priv->clients, l);
- g_list_free_1 (l);
+ bl->priv->clients = g_list_remove (bl->priv->clients, book);
/* When all clients go away, notify the parent factory about it so that
* it may decide whether to kill the backend or not.
@@ -3300,8 +3347,12 @@ pas_backend_ldap_destroy (GtkObject *object)
g_hash_table_foreach_remove (bl->priv->id_to_op, (GHRFunc)call_dtor, NULL);
g_hash_table_destroy (bl->priv->id_to_op);
- if (bl->priv->poll_timeout != -1)
+ if (bl->priv->poll_timeout != -1) {
+ printf ("removing timeout\n");
g_source_remove (bl->priv->poll_timeout);
+ }
+
+ gtk_object_unref (GTK_OBJECT (bl->priv->book_views));
if (bl->priv->supported_fields)
gtk_object_unref (GTK_OBJECT (bl->priv->supported_fields));
@@ -3345,6 +3396,8 @@ pas_backend_ldap_init (PASBackendLDAP *backend)
priv->ldap_limit = 100;
priv->id_to_op = g_hash_table_new (g_int_hash, g_int_equal);
priv->poll_timeout = -1;
+ priv->book_views = e_list_new (NULL, NULL, NULL);
+
backend->priv = priv;
}