dgaudet 98/02/07 02:12:23
Modified: src buff.c buff.h
Log:
Chunking sucks. I wrote a stress testing module and found two more bugs:
- start_chunk() called bflush() called start_chunk() caused chaos
- if we ended up in the tail of bwrite() where a memcpy happens to copy
the remainder, in certain boundary cases with chunking we would
go past the end of the buffer
Just generally clean up chunking a bit more. This would be a lot easier
if chunking were just a layered I/O handler.
Revision Changes Path
1.28 +48 -36 apache-1.2/src/buff.c
Index: buff.c
===================================================================
RCS file: /export/home/cvs/apache-1.2/src/buff.c,v
retrieving revision 1.27
retrieving revision 1.28
diff -u -r1.27 -r1.28
--- buff.c 1998/01/30 09:13:47 1.27
+++ buff.c 1998/02/07 10:12:18 1.28
@@ -71,6 +71,10 @@
#endif
#define DEFAULT_BUFSIZE (4096)
+/* This must be enough to represent (DEFAULT_BUFSIZE - 3) in hex,
+ * plus two extra characters.
+ */
+#define CHUNK_HEADER_SIZE (5)
/*
* Buffered I/O routines.
@@ -186,6 +190,9 @@
}
}
+
+static int bflush_core(BUFF *fb);
+
/*
* Start chunked encoding.
*
@@ -198,9 +205,6 @@
static void
start_chunk( BUFF *fb )
{
- char chunksize[16]; /* Big enough for practically anything */
- int chunk_header_size;
-
if (fb->outchunk != -1) {
/* already chunking */
return;
@@ -210,21 +214,16 @@
return;
}
- /* we know that the chunk header is going to take at least 3 bytes... */
- chunk_header_size = ap_snprintf( chunksize, sizeof(chunksize),
- "%x\015\012", fb->bufsiz - fb->outcnt - 3 );
/* we need at least the header_len + at least 1 data byte
* remember that we've overallocated fb->outbase so that we can always
* fit the two byte CRLF trailer
*/
- if( fb->bufsiz - fb->outcnt < chunk_header_size + 1 ) {
- bflush(fb);
+ if( fb->bufsiz - fb->outcnt < CHUNK_HEADER_SIZE + 1 ) {
+ bflush_core(fb);
}
/* assume there's enough space now */
- memcpy( &fb->outbase[fb->outcnt], chunksize, chunk_header_size );
fb->outchunk = fb->outcnt;
- fb->outcnt += chunk_header_size;
- fb->outchunk_header_size = chunk_header_size;
+ fb->outcnt += CHUNK_HEADER_SIZE;
}
@@ -235,13 +234,14 @@
end_chunk( BUFF *fb )
{
int i;
+ char *strp;
if( fb->outchunk == -1 ) {
/* not chunking */
return;
}
- if( fb->outchunk + fb->outchunk_header_size == fb->outcnt ) {
+ if( fb->outchunk + CHUNK_HEADER_SIZE == fb->outcnt ) {
/* nothing was written into this chunk, and we can't write a 0 size
* chunk because that signifies EOF, so just erase it
*/
@@ -252,22 +252,21 @@
/* we know this will fit because of how we wrote it in start_chunk() */
i = ap_snprintf( (char *)&fb->outbase[fb->outchunk],
- fb->outchunk_header_size,
- "%x", fb->outcnt - fb->outchunk - fb->outchunk_header_size );
+ CHUNK_HEADER_SIZE,
+ "%x", fb->outcnt - fb->outchunk - CHUNK_HEADER_SIZE );
/* we may have to tack some trailing spaces onto the number we just wrote
* in case it was smaller than our estimated size. We've also written
* a \0 into the buffer with ap_snprintf so we might have to put a
* \r back in.
*/
- i += fb->outchunk;
- while( fb->outbase[i] != '\015' && fb->outbase[i] != '\012' ) {
- fb->outbase[i++] = ' ';
- }
- if( fb->outbase[i] == '\012' ) {
- /* we overwrote the \r, so put it back */
- fb->outbase[i-1] = '\015';
+ strp = &fb->outbase[fb->outchunk + i];
+ while (i < CHUNK_HEADER_SIZE - 2) {
+ *strp++ = ' ';
+ ++i;
}
+ *strp++ = '\015';
+ *strp = '\012';
/* tack on the trailing CRLF, we've reserved room for this */
fb->outbase[fb->outcnt++] = '\015';
@@ -747,7 +746,7 @@
int
bwrite(BUFF *fb, const void *buf, int nbyte)
{
- int i, nwr;
+ int i, nwr, useable_bufsiz;
if (fb->flags & (B_WRERR|B_EOUT)) return -1;
if (nbyte == 0) return 0;
@@ -833,8 +832,12 @@
* original buffer until there is less than bufsiz left. Note that we
* use bcwrite() to do this for us, it will do the chunking so that
* we don't have to dink around building a chunk in our own buffer.
+ * Remember we may not be able to use the entire buffer if we're
+ * chunking.
*/
- while (nbyte >= fb->bufsiz)
+ useable_bufsiz = fb->bufsiz;
+ if (fb->flags & B_CHUNK) useable_bufsiz -= CHUNK_HEADER_SIZE;
+ while (nbyte >= useable_bufsiz)
{
i = bcwrite(fb, buf, nbyte);
if (i <= 0) {
@@ -861,21 +864,10 @@
return nwr;
}
-/*
- * Flushes the buffered stream.
- * Returns 0 on success or -1 on error
- */
-int
-bflush(BUFF *fb)
+static int bflush_core(BUFF *fb)
{
int i;
- if (!(fb->flags & B_WR) || (fb->flags & B_EOUT)) return 0;
-
- if (fb->flags & B_WRERR) return -1;
-
- if (fb->flags & B_CHUNK) end_chunk(fb);
-
while (fb->outcnt > 0)
{
do {
@@ -913,8 +905,28 @@
return -1;
}
- if (fb->flags & B_CHUNK)
+ return 0;
+}
+
+/*
+ * Flushes the buffered stream.
+ * Returns 0 on success or -1 on error
+ */
+int bflush(BUFF *fb)
+{
+ int ret;
+
+ if (!(fb->flags & B_WR) || (fb->flags & B_EOUT)) return 0;
+
+ if (fb->flags & B_WRERR) return -1;
+
+ if (fb->flags & B_CHUNK) end_chunk(fb);
+
+ ret = bflush_core(fb);
+
+ if (ret == 0 && (fb->flags & B_CHUNK)) {
start_chunk(fb);
+ }
return 0;
}
1.14 +0 -1 apache-1.2/src/buff.h
Index: buff.h
===================================================================
RCS file: /export/home/cvs/apache-1.2/src/buff.h,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -r1.13 -r1.14
--- buff.h 1998/01/30 09:13:48 1.13
+++ buff.h 1998/02/07 10:12:20 1.14
@@ -83,7 +83,6 @@
int incnt; /* number of bytes left to read from input buffer;
* always 0 if had a read error */
int outchunk; /* location of chunk header when chunking */
- int outchunk_header_size; /* how long the header is */
int outcnt; /* number of byte put in output buffer */
unsigned char *inbase;
unsigned char *outbase;
