[PATCH] fix Rasmus' chunking error

[Dean Gaudet <dg...@arctic.org>]

On Tue, 16 Dec 1997, Roy T. Fielding wrote:

> >When I load up the port 81 page a dialog pops up which says:
> >
> >  "Internet Explorer cannot open the Internet Site http://www.ler....
> >   Could not complete operation due to error 800c0008"
> >
> >and it shows me about 3/4 of the page.  The port 80 URL comes up fine.
> >Both servers are set up with the same document root for the
> >www.lerdorf.on.ca host-header virtual host, so it is the exact same file
> >being served up in both cases.
> 
> The last chunk is screwed up....
> 
>    positive input during the initial development of the interpreter.
>    ^M
>    1^M
>    <^M
>    /font>
>    </body></html>
>    0^M
>    ^M
> 
> which means that the module is doing something screwy with the buff.c
> calls or there is some borderline case that is being tickled.  You
> could output the same contents as a CGI (without content-length)
> and see if that is the case, but I suspect it is a problem with
> mixing buffers.  Try a systrace (truss) to see if you can pick up on
> a weird call pattern, or try stepping through it with gdb.
> 
> This is definitely not a bug in IE4.

Right, it's our bug.  In fact there's two buglets happening.  The first is
that bputc() does not understand chunking... it's a macro defined in
buff.h and I guess I missed it.  When it fills the buffer it calls
bflsbuf() to flush and buffer a single character, but it doesn't call
start_chunk() before starting the new buffer. Rather than slow down/expand
the macro any I chose to fix this problem by doing a start_chunk() in the
bflsbuf() routine.  This bug exists in 1.2. 

The second buglet is that when using bputc(), it is way easy to trigger
the large_write() code.  What was happening above was that it would
trigger large_write(), do a 4 element writev() (the buffer, chunked, plus
a chunk header, the 1 byte to write, and a chunk footer).  A workaround is
to not consider 1 byte writes for large_write().  large_write isn't in 1.2
so this bug isn't either. 

Dean

Index: main/buff.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/buff.c,v
retrieving revision 1.51
diff -u -r1.51 buff.c
--- buff.c	1997/11/13 20:37:57	1.51
+++ buff.c	1997/12/20 22:20:30
@@ -807,9 +807,16 @@
 API_EXPORT(int) bflsbuf(int c, BUFF *fb)
 {
     char ss[1];
+    int rc;
 
     ss[0] = c;
-    return bwrite(fb, ss, 1);
+    rc = bwrite(fb, ss, 1);
+    /* We do start_chunk() here so that the bputc macro can be smaller
+     * and faster
+     */
+    if (rc == 1 && (fb->flags & B_CHUNK))
+	start_chunk(fb);
+    return rc;
 }
 
 /*
@@ -1059,9 +1080,12 @@
 #ifndef NO_WRITEV
 /*
  * Detect case where we're asked to write a large buffer, and combine our
- * current buffer with it in a single writev()
+ * current buffer with it in a single writev().  Note we don't consider
+ * the case nbyte == 1 because modules which use rputc() loops will cause
+ * us to use writev() too frequently.  In those cases we really should just
+ * start a new buffer.
  */
-    if (fb->outcnt > 0 && nbyte + fb->outcnt >= fb->bufsiz) {
+    if (fb->outcnt > 0 && nbyte > 1 && nbyte + fb->outcnt >= fb->bufsiz) {
 	return large_write(fb, buf, nbyte);
     }
 #endif