Re: [Evolution-hackers] known bugs in gpg support?

[Jeffrey Stedfast <fejj ximian com>]

As per the discussion, new patch is attached.

Michael: okay for me to commit (to both branches)?

Jeff

On Thu, 2003-02-27 at 00:34, Jeffrey Stedfast wrote:
> On Wed, 2003-02-26 at 23:52, Not Zed wrote:
> > On Thu, 2003-02-27 at 10:16, Jeffrey Stedfast wrote:
> > > did some digging and it seems that my first impression of the problem
> > > was *almost right*.  I had been thinking that we were passing the last
> > > \n before the boundary (which really belongs to the boundary, so we
> > > should not be doing that) but it actually turns out the canon filter
> > > code would always just add a \r\n to the end of the stream even if the
> > > higher-level code did provided a stream that did not end with \n
> > > 
> > > however, that was not the only problem... I discovered in my testing
> > > that the code was in fact dropping the entire last line when sending it
> > > to gpg because the last line did not end with a \n, and so it was saved
> > > in the backup buffer of the canon filter waiting for a
> > > filter_complete().
> > > 
> > > the filter_complete(), however, would never come because:
> > > 1) all of the data had been read from the src stream
> > > 2) filter_stream->priv->filteredlen == 0, meaning that the filter_stream
> > > did not have any data in its own buffer.
> > 
> > > to fix this, I just fixed camel-stream-filter.c:do_eos() to check for
> > > buffered data in any of the filters in the chain. if there was data
> > > there, then eos == FALSE.
> > 
> > I'm not sure this fix is correct, but it might be, i'll verify.
> > 
> > Another simpler way might be to do
> > 
> > eos = filteredlen == 0
> > 	&& complete has been called
> > 
> >  and set the 'complete called' when its been called, since filters MUST
> > flush everything when that is called.
> 
> that'd be fine... 
> 
> > 
> > > attached are patches for 1.2 and 1.3 - it seems when I backported a
> > > patch from 1.3 to 1.2 (or vise versa?) and extra blank line made it in
> > > there and so the 1.3 patch will not apply cleanly to the 1-2-branch, so
> > > I made 2 patches. both are identical except for that minor difference.
> > > 
> > > Michael (NotZed): The only change I'm not 100% sure about is the removal
> > > of the code in camel-mime-filter-canon.c:filter_complete() that added
> > > \r\n to the stream. As far as I can tell, it should not be needed since
> > > filter_filter() works on a line-by-line basis and so any remaining
> > > buffered data *should* be an incomplete line and thus never contain a
> > > \n, and so I *think* my removal of that code is fine.
> > 
> > Its a fallback for repairing truncated data ... its there for a reason. 
> > It might not be the right approach for canonicalising pgp data though.
> > 
> > Just #ifdef it out for now.  If somethign else needs it to work like
> > that it can be done as a different flag, or if not, removed later.
> 
> ah, okay... will do.
> 
> Jeff
-- 
Jeffrey Stedfast
Evolution Hacker - Ximian, Inc.
fejj ximian com  - www.ximian.com

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/camel/ChangeLog,v
retrieving revision 1.1681.2.9
diff -u -r1.1681.2.9 ChangeLog
--- ChangeLog	24 Feb 2003 03:48:29 -0000	1.1681.2.9
+++ ChangeLog	27 Feb 2003 18:51:19 -0000
@@ -1,3 +1,19 @@
+2003-02-27  Jeffrey Stedfast  <fejj ximian com>
+
+	* camel-stream-filter.c: Add a 'flushed' state variable to the
+	private struct.
+	(do_read): Set p->flushed to TRUE after we call
+	camel_mime_filter_complete() on all the filters.
+	(do_reset): Set p->flushed to FALSE.
+	(do_eos): Make sure the filters have been flushed before returning
+	that the stream is at EOS.
+
+	* camel-mime-filter-canon.c (complete): Don't add a eol -
+	otherwise we will fail to verify some mutt signatures that do not
+	have a blank line before the boundary line (and note that the last
+	\n before the boundary really belongs to the boundary anyway) so
+	#if 0 this code out for now.
+
 2003-02-22  Jeffrey Stedfast  <fejj ximian com>
 
 	* providers/sendmail/camel-sendmail-transport.c
Index: camel-mime-filter-canon.c
===================================================================
RCS file: /cvs/gnome/evolution/camel/camel-mime-filter-canon.c,v
retrieving revision 1.2.8.1
diff -u -r1.2.8.1 camel-mime-filter-canon.c
--- camel-mime-filter-canon.c	21 Jan 2003 03:07:05 -0000	1.2.8.1
+++ camel-mime-filter-canon.c	27 Feb 2003 18:51:19 -0000
@@ -195,6 +195,8 @@
 			while (o>starto && (o[-1] == ' ' || o[-1] == '\t' || o[-1]=='\r'))
 				o--;
 		}
+		
+#if 0
 		/* check end of line canonicalisation */
 		if (o>starto) {
 			if (flags & CAMEL_MIME_FILTER_CANON_CRLF) {
@@ -208,7 +210,8 @@
 
 		/* and always finish with an eol */
 		*o++ = '\n';
-
+#endif
+		
 		*outlen = o - *out;
 		
 		f->backlen = 0;
Index: camel-stream-filter.c
===================================================================
RCS file: /cvs/gnome/evolution/camel/camel-stream-filter.c,v
retrieving revision 1.24
diff -u -r1.24 camel-stream-filter.c
--- camel-stream-filter.c	19 Jul 2002 11:25:02 -0000	1.24
+++ camel-stream-filter.c	27 Feb 2003 18:51:19 -0000
@@ -19,11 +19,16 @@
  * Boston, MA 02111-1307, USA.
  */
 
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
 #include <string.h>
 #include "camel-stream-filter.h"
 
 #define d(x)
-/*#include <stdio.h>*/
 
 /* use my malloc debugger? */
 /*extern void g_check(void *mp);*/
@@ -45,7 +50,8 @@
 	char *filtered;		/* the filtered data */
 	size_t filteredlen;
 
-	int last_was_read;	/* was the last op read or write? */
+	int last_was_read:1;	/* was the last op read or write? */
+	int flushed:1           /* were the filters flushed? */
 };
 
 #define READ_PAD (128)		/* bytes padded before buffer */
@@ -90,6 +96,7 @@
 	p->realbuffer = g_malloc(READ_SIZE + READ_PAD);
 	p->buffer = p->realbuffer + READ_PAD;
 	p->last_was_read = TRUE;
+	p->flushed = FALSE;
 }
 
 static void
@@ -234,6 +241,7 @@
 					f = f->next;
 				}
 				size = p->filteredlen;
+				p->flushed = TRUE;
 			}
 			if (size <= 0)
 				return size;
@@ -371,7 +379,10 @@
 
 	if (p->filteredlen > 0)
 		return FALSE;
-
+	
+	if (!p->flushed)
+		return FALSE;
+	
 	return camel_stream_eos(filter->source);
 }
 
@@ -383,7 +394,8 @@
 	struct _filter *f;
 
 	p->filteredlen = 0;
-
+	p->flushed = FALSE;
+	
 	/* and reset filters */
 	f = p->filters;
 	while (f) {