Here's another patch for libapreq that should help keep
Apache::Request from hogging memory during large file uploads.
The 2 main problems with the multipart_buffer code are
1) the loop in multipart_buffer_fill needs "optimizing".
The my_join call (line 142 in multipart_buffer.c) mallocs
a brand new buffer with each pass of the loop. This is BAD.
2) multipart_buffer_read (line 225 in multipart_buffer.c) mallocs
with each function call.
A large file upload calls this code repeatedly
(427-430 inapache_request.c), so you wind up shoving another
copy of the entire file into r->pool.
Here's the patch. I parsed a 50M file with it using Apache::Request,
but that's about the full extent to which it's been tested.
diff -ur libapreq-0.31/c/multipart_buffer.c libapreq-0.31-new/c/multipart_buffer.c
--- libapreq-0.31/c/multipart_buffer.c Sat May 1 02:44:28 1999
+++ libapreq-0.31-new/c/multipart_buffer.c Thu Jun 29 01:58:43 2000
@@ -55,111 +55,112 @@
*
*/
+/* JS: 6/29/2000
+ * This should fix the memory allocation issues.
+ */
+
+
#include "multipart_buffer.h"
-#define FILLUNIT (1024 * 5)
#ifndef CRLF
#define CRLF "\015\012"
#endif
#define CRLF_CRLF "\015\012\015\012"
+#define min(A, B) ((A) < (B) ? (A) : (B))
+
+#define DEBUG 0 /* 0 = off, 1 = noisy */
static char *my_join(pool *p, char *one, int lenone, char *two, int lentwo)
{
- char *res;
- int len = lenone+lentwo;
- res = (char *)ap_palloc(p, len + 1);
- memcpy(res, one, lenone);
+ char *res;
+ res = (char *)ap_palloc(p, lenone + lentwo + 1);
+ memcpy(res, one, lenone);
memcpy(res+lenone, two, lentwo);
+ *(res+lenone+lentwo+1) = '\0';
return res;
}
-static char *
-my_ninstr(register char *big, register char *bigend, char *little, char *lend)
-{
- register char *s, *x;
- register int first = *little;
- register char *littleend = lend;
-
- if (!first && little >= littleend) {
- return big;
- }
- if (bigend - big < littleend - little) {
- return NULL;
- }
- bigend -= littleend - little++;
- while (big <= bigend) {
- if (*big++ != first) {
- continue;
- }
- for (x=big,s=little; s < littleend; /**/ ) {
- if (*s++ != *x++) {
- s--;
- break;
- }
- }
- if (s >= littleend) {
- return big-1;
- }
+void multipart_buffer_flush(multipart_buffer *self)
+{
+ /* shifts all unread data in the buffer to the beginning of the buffer */
+
+ char *ptr = self->buffer;
+ while ( ptr < self->buffer + self->buffer_len) {
+ *ptr = *(ptr + self->buffer_off);
+ ++ptr;
}
- return NULL;
-}
+
+ self->buffer_off = 0;
+}
+
/*
* This fills up our internal buffer in such a way that the
* boundary is never split between reads
*/
-void multipart_buffer_fill(multipart_buffer *self, long bytes)
+
+int multipart_buffer_fill(multipart_buffer *self, long bytes)
{
- int len_read, length, bytes_to_read;
- int buffer_len = self->buffer_len;
+
+ int len_read=0, length, bytes_to_read, total=0;
if (!self->length) {
return;
}
- length = (bytes - buffer_len) + self->boundary_length + 2;
- if (self->length < length) {
- length = self->length;
+
+ bytes_to_read = min( (BUFSIZE) - self->buffer_len, min( bytes, self->length) );
+
+ if ( bytes_to_read + self->buffer_off + self->buffer_len > BUFSIZE )
+ { /* overflow condition- need to flush stale data */
+ multipart_buffer_flush(self);
}
- bytes_to_read = length;
+
+#if DEBUG == 1
+ ap_log_rerror(MPB_ERROR,
+ "[libapreq]: BUFFER_FILL: SIZE=%d, OFF=%d, LENGTH=%d",
+ bytes_to_read, self->buffer_off,
+ self->buffer_len);
+#endif /* DEBUG */
ap_hard_timeout("multipart_buffer_fill", self->r);
+
+ total = bytes_to_read;
+
while (bytes_to_read > 0) {
- /*XXX: we can optimize this loop*/
- char *buff = (char *)ap_pcalloc(self->subp,
- sizeof(char) * bytes_to_read + 1);
- len_read = ap_get_client_block(self->r, buff, bytes_to_read);
+
+ char *buff = self->buffer + self->buffer_len + self->buffer_off;
+ len_read = ap_get_client_block(self->r,
+ buff,
+ bytes_to_read);
if (len_read < 0) {
ap_log_rerror(MPB_ERROR,
"[libapreq] client dropped connection during read");
self->length = 0;
- self->buffer = NULL;
self->buffer_len = 0;
- return;
+ multipart_buff_flush(self);
+ return 0;
}
- self->buffer = self->buffer ?
- my_join(self->r->pool,
- self->buffer, self->buffer_len,
- buff, len_read) :
- ap_pstrndup(self->r->pool, buff, len_read);
-
- self->total += len_read;
self->buffer_len += len_read;
- self->length -= len_read;
- bytes_to_read -= len_read;
+ bytes_to_read -= len_read;
ap_reset_timeout(self->r);
}
+
+ self->total += total;
+ self->length -= total;
+
ap_kill_timeout(self->r);
- ap_clear_pool(self->subp);
+
+ return total;
}
char *multipart_buffer_read(multipart_buffer *self, long bytes, int *blen)
{
int start = -1;
- char *retval, *str;
-
+ char *mem, *retval;
+
/* default number of bytes to read */
if (!bytes) {
bytes = FILLUNIT;
@@ -169,18 +170,32 @@
* Fill up our internal buffer in such a way that the boundary
* is never split between reads.
*/
- multipart_buffer_fill(self, bytes);
- /* Find the boundary in the buffer (it may not be there). */
- if (self->buffer) {
- if ((str = my_ninstr(self->buffer,
- self->buffer+self->buffer_len,
- self->boundary,
- self->boundary+self->boundary_length)))
- {
- start = str - self->buffer;
+ while ( self->buffer_len < self->boundary_length + 2 ) {
+ if ( multipart_buffer_fill(self, bytes) == 0 ) { /* paranoid test */
+ multipart_buffer_flush(self);
}
- }
+ }
+
+
+#if DEBUG == 1
+ ap_log_rerror(MPB_ERROR,
+ "[libapreq]: BUFFER_READ: SIZE=%d, OFF=%d, LENGTH=%d\n",
+ self->length, self->buffer_off,
+ self->buffer_len, self->buffer + self->buffer_off);
+#endif /* DEBUG */
+
+
+
+ if (self->buffer_len == 0) { return NULL; }
+
+ /* Find the boundary in the buffer (it may not be there). */
+
+ if (mem = memmem(self->buffer + self->buffer_off, self->buffer_len,
+ self->boundary, self->boundary_length) )
+ {
+ start = mem - ( self->buffer + self->buffer_off );
+ }
/* protect against malformed multipart POST operations */
if (!(start >= 0 || self->length > 0)) {
@@ -195,39 +210,53 @@
* and return NULL. The +2 here is a fiendish plot to
* remove the CR/LF pair at the end of the boundary.
*/
+
if (start == 0) {
+
+ if ( self->buffer_len > self->boundary_length + 2 &&
+ memcmp(self->buffer+self->buffer_off,
+ self->boundary_end,
+ self->buffer_len) >=0
+ )
+ {
/* clear us out completely if we've hit the last boundary. */
- if (strEQ(self->buffer, self->boundary_end)) {
- self->buffer = NULL;
- self->buffer_len = 0;
+
+ ap_clear_pool(self->subp);
self->length = 0;
+ self->buffer_len = 0;
return NULL;
}
/* otherwise just remove the boundary. */
- self->buffer += (self->boundary_length + 2);
+ self->buffer_off += (self->boundary_length + 2);
self->buffer_len -= (self->boundary_length + 2);
return NULL;
}
if (start > 0) { /* read up to the boundary */
- *blen = start > bytes ? bytes : start;
- }
- else { /* read the requested number of bytes */
+ *blen = min(start, bytes);
+
+ } else {
+ /* read the requested number of bytes */
/*
* leave enough bytes in the buffer to allow us to read
* the boundary. Thanks to Kevin Hendrick for finding
* this one.
*/
- *blen = bytes - (self->boundary_length + 1);
+ *blen = self->buffer_len - (self->boundary_length + 1);
+ }
+
+ retval = self->buffer + self->buffer_off;
+ self->buffer_len -= *blen;
+
+ if (!self->buffer_len) {
+ self->buffer_off = 0; /* no data left, so we reset buffer */
+ } else {
+ self->buffer_off += *blen;
}
-
- retval = ap_pstrndup(self->r->pool, self->buffer, *blen);
-
- self->buffer += *blen;
- self->buffer_len -= *blen;
/* If we hit the boundary, remove the CRLF from the end. */
+
if (start > 0) {
*blen -= 2;
retval[*blen] = '\0';
@@ -238,51 +267,78 @@
table *multipart_buffer_headers(multipart_buffer *self)
{
- int end=0, ok=0, bad=0;
+ int end=-1, length=0;
table *tab;
char *header;
+ char *entry;
+
+
+ do {
+
+ char *mem;
- do {
- char *str;
multipart_buffer_fill(self, FILLUNIT);
- if ((str = strstr(self->buffer, CRLF_CRLF))) {
- ++ok;
- end = str - self->buffer;
- }
- if (self->buffer == NULL || *self->buffer == '\0') {
- ++ok;
- }
- if (!ok && self->length <= 0) {
- ++bad;
+
+ if (mem = memmem(self->buffer + self->buffer_off, self->buffer_len,
+ CRLF_CRLF, 4))
+ {
+ /* have final header 'fragment' in the buffer */
+
+ end = mem - ( self->buffer + self->buffer_off );
+
+ header = my_join( self->subp,
+ header, length,
+ self->buffer + self->buffer_off, end+2 );
+
+ length += end+2;
+ self->buffer_off += end+4;
+ self->buffer_len -= end+4;
+
+ } else if (self->buffer_len > 3) {
+ /* fetch fragment */
+
+ header = my_join( self->subp,
+ header, length,
+ self->buffer + self->buffer_off, self->buffer_len-3);
+
+ length += self->buffer_len - 3;
+ self->buffer_off += self->buffer_len - 3;
+ self->buffer_len = 3;
+
+ } else if (self->length <= 0) {
+ /* bad header read, check out early*/
+
+ ap_log_rerror( MPB_ERROR, "[libapreq]: Bad header read near EOF." );
+ return NULL;
+
}
- } while (!ok && !bad);
- if (bad) {
- return NULL;
- }
+ } while (end < 0);
- header = ap_pstrndup(self->r->pool, self->buffer, end+2);
- /*XXX: need to merge continuation lines here?*/
+
+#if DEBUG == 1
+ ap_log_rerror(MPB_ERROR,
+ "[libapreq]: DONE READING HEADER: OFF=%d, LENGTH=%d\n%s",
+ self->buffer_off,
+ self->buffer_len, self->buffer + self->buffer_off);
+#endif /* DEBUG */
tab = ap_make_table(self->r->pool, 10);
- self->buffer += end+4;
- self->buffer_len -= end+4;
- {
- char *entry;
- while ((entry = ap_getword_nc(self->r->pool, &header, '\r')) && *header) {
- char *key;
- key = ap_getword_nc(self->r->pool, &entry, ':');
- while (ap_isspace(*entry)) {
- ++entry;
- }
- if (*header == '\n') {
- ++header;
- }
- ap_table_add(tab, key, entry);
+ while ((entry = ap_getword_nc(self->subp, &header, '\r')) && *header) {
+ char *key;
+ key = ap_getword_nc(self->subp, &entry, ':');
+ while (ap_isspace(*entry)) {
+ ++entry;
}
+ if (*header == '\n') {
+ ++header;
+ }
+ ap_table_add(tab, key, entry); /* copies key & entry strings to tab */
}
+ ap_clear_pool(self->subp); /* OK to clean up here */
+
return tab;
}
@@ -293,11 +349,14 @@
while ((data = multipart_buffer_read(self, 0, &blen))) {
retval = retval ?
- my_join(self->r->pool, retval, old_len, data, blen) :
- ap_pstrndup(self->r->pool, data, blen);
+ my_join(self->subp, retval, old_len, data, blen) :
+ ap_pstrndup(self->subp, data, blen);
old_len = blen;
}
+/* can't clear subp w/o destroying retval
+ * ap_clear_pool(self->subp);
+ */
return retval;
}
@@ -307,14 +366,14 @@
multipart_buffer *self = (multipart_buffer *)
ap_pcalloc(r->pool, sizeof(multipart_buffer));
+ self->subp = ap_make_sub_pool(r->pool);
self->r = r;
self->length = length;
self->boundary = ap_pstrcat(r->pool, "--", boundary, NULL);
self->boundary_length = strlen(self->boundary);
self->boundary_end = ap_pstrcat(r->pool, self->boundary, "--", NULL);
- self->buffer = NULL;
- self->buffer_len = 0;
- self->subp = ap_make_sub_pool(self->r->pool);
+ self->buffer_off = 0;
+ self->buffer_len = 0;
/* Read the preamble and the topmost (boundary) line plus the CRLF. */
(void)multipart_buffer_read(self, 0, &blen);
diff -ur libapreq-0.31/c/multipart_buffer.h libapreq-0.31-new/c/multipart_buffer.h
--- libapreq-0.31/c/multipart_buffer.h Sat May 1 02:44:28 1999
+++ libapreq-0.31-new/c/multipart_buffer.h Thu Jun 29 01:58:44 2000
@@ -1,24 +1,29 @@
-#include "apache_request.h"
+#include "apache_request.h" /* JS: 6/29/2000 - littered with comments */
+
+#define BUFSIZE (1024 * 8) /* >> maximum possible boundary_length */
+#define FILLUNIT (1024 * 4) /* < BUFSIZE (default byte read) */
typedef struct {
request_rec *r;
pool *subp;
long length;
- long total;
- long boundary_length;
- char *boundary;
+ long total;
+ long boundary_length; /* ~ 40 ; see BUFSIZE above*/
+ char *boundary;
char *boundary_end;
- char *buffer;
- long buffer_len;
+ char buffer[BUFSIZE];
+ long buffer_len; /* size of new data in buff[BUFSIZE] */
+ long buffer_off; /* NEW: starting point of new data in buff[BUFSIZE] */
} multipart_buffer;
#define multipart_buffer_eof(self) \
-(((self->buffer == NULL) || (*self->buffer == '\0')) && (self->length <= 0))
-
+(((self->buffer_len == 0 ) || (self->buffer + self->buffer_off == '\0')) && (self->length <= 0))
char *multipart_buffer_read_body(multipart_buffer *self);
table *multipart_buffer_headers(multipart_buffer *self);
-void multipart_buffer_fill(multipart_buffer *self, long bytes);
+void multipart_buffer_flush(multipart_buffer *self); /* new */
+int multipart_buffer_fill(multipart_buffer *self, long bytes);
char *multipart_buffer_read(multipart_buffer *self, long bytes, int *blen);
multipart_buffer *multipart_buffer_new(char *boundary, long length, request_rec *r);
-
#define MPB_ERROR APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, self->r
+
+
HTH.
--
Joe Schaefer
