[nazard@xxxxxxxxxxxxxxx]

On 31 Mar, Guy Harris wrote:
>> As a side note, the ASN1 call that is failing is a string_decode. Apparently
>> one of the above products uses a string type that fails in this test here
> 
> No, we were incorrectly dissecting the "present" CHOICE value for the
> Filter type.
> 
> I checked in fixes to a number of bugs I ran across, most if not all of
> which are probably my fault, not Doug's.  (I also checked in the
> beginnings of some checks for ASN.1 dissection errors, which I'd put in
> to try to keep various loops from just running on forever when I was
> debugging the problem.)

Damn. Went to post the fix and just received your message. <sigh>

Oh well, while working on this, I noticed that the full dissector ran
for each packet even while loading. Based on the code before, I had
assumed that tree would be null during the first pass (while populating
the packet list) and only be set during population of the protocol tree
allowing me to short circuit the dissector. 

If this is by design, is there something else I can use?

-- 
Doug Nazar
Dragon Computer Consultants Inc.
Tel: (416) 708-1578     Fax: (416) 708-8081

Index: packet-ldap.c
===================================================================
RCS file: /cvsroot/ethereal/packet-ldap.c,v
retrieving revision 1.4
diff -u -w -r1.4 packet-ldap.c
--- packet-ldap.c	2000/03/29 09:25:20	1.4
+++ packet-ldap.c	2000/03/31 10:25:13
@@ -262,7 +262,7 @@
   return read_string_value(a, tree, hf_id, new_tree, s, start, length);
 }
 
-static void parse_filter_strings(ASN1_SCK *a, char **filter, guint *filter_length, const guchar *operation)
+static int parse_filter_strings(ASN1_SCK *a, char **filter, guint *filter_length, const guchar *operation)
 {
   guchar *string;
   guchar *string2;
@@ -270,13 +270,18 @@
   gint string2_length;
   guint string_bytes;
 
-  asn1_octet_string_decode(a, &string, &string_length, &string_bytes);
-  asn1_octet_string_decode(a, &string2, &string2_length, &string_bytes);
+  if (asn1_octet_string_decode(a, &string, &string_length, &string_bytes))
+    return 1;
+  if (asn1_octet_string_decode(a, &string2, &string2_length, &string_bytes))
+    return 1;
+
   *filter_length += 2 + strlen(operation) + string_length + string2_length;
   *filter = g_realloc(*filter, *filter_length);
   sprintf(*filter + strlen(*filter), "(%.*s%s%.*s)", string_length, string, operation, string2_length, string2);
   g_free(string);
   g_free(string2);
+  
+  return 0;
 }
 
 static gboolean parse_filter(ASN1_SCK *a, char **filter, guint *filter_length, const guchar **end)
@@ -286,7 +291,8 @@
   gboolean def;
 
   /* XXX - what if this returns an error? */
-  asn1_header_decode(a, &cls, &con, &tag, &def, &length);
+  if (asn1_header_decode(a, &cls, &con, &tag, &def, &length))
+    return TRUE;
   
   if (*end == 0)
   {
@@ -366,13 +372,12 @@
      case LDAP_FILTER_PRESENT:
       {
         guchar *string;
-        gint string_length;
-        guint string_bytes;
     
-        asn1_octet_string_decode(a, &string, &string_length, &string_bytes);
-        *filter_length += 3 + string_length;
+        if (asn1_octet_string_value_decode(a, length, &string))
+	  return TRUE;
+        *filter_length += 3 + length;
         *filter = g_realloc(*filter, *filter_length);
-        sprintf(*filter + strlen(*filter), "(%.*s=*)", string_length, string);
+        sprintf(*filter + strlen(*filter), "(%.*s=*)", length, string);
         g_free(string);
       }
       break;
@@ -399,6 +404,12 @@
   while (!parse_filter(a, &filter, &filter_length, &end))
     continue;
   
+  if (end != a->pointer)
+  {
+    printf("Ethereal: packet-ldap: premature exit from parse_filter\n");
+    a->pointer = end;
+  }
+   
   if (tree)
     proto_tree_add_item(tree, hf_id, start-a->begin, a->pointer-start, filter);
 
@@ -680,15 +691,15 @@
     message_start = a.pointer - a.begin;
     if (read_sequence(&a, &messageLength))
     {
-      if (tree)
-        proto_tree_add_text(tree, offset, 1, "Invalid LDAP packet");
+      if (ldap_tree)
+        proto_tree_add_text(ldap_tree, offset, 1, "Invalid LDAP packet");
       break;
     }
 
     if (messageLength > (a.end - a.pointer))
     {
-      if (tree)
-        proto_tree_add_text(tree, message_start, END_OF_FRAME, "Short message! (expected: %u, actual: %u)",
+      if (ldap_tree)
+        proto_tree_add_text(ldap_tree, message_start, END_OF_FRAME, "Short message! (expected: %u, actual: %u)",
 			    messageLength, a.end - a.pointer);
       break;
     }
@@ -713,9 +724,11 @@
         col_add_fstr(fd, COL_INFO, "MsgId=%u MsgType=%s",
 		     messageId, typestr);
       first_time = 0;
+      if (!tree)
+	return;
     }
 
-    if (tree) 
+    if (ldap_tree) 
     {
       proto_tree_add_item_hidden(ldap_tree, hf_ldap_message_id, message_id_start, message_id_length, messageId);
       proto_tree_add_item_hidden(ldap_tree, hf_ldap_message_type,