From 3a5aa0275ab298a90be42647482de0477ba8c9a0 Mon Sep 17 00:00:00 2001 From: erickson Date: Thu, 15 May 2008 13:36:32 +0000 Subject: [PATCH 1/1] Patch from Scott McKellar: This patch is mostly a performance tweak, but also tidies up a few things. In apacheParseParms() we load POST data and GET data into the same buffer, with the GET data coming first. However the old code loads the POST data first. If there is also some GET data, we juggle some buffers in order to get the GET and POST data into the right order. The new code loads the GET data first, and then appends the POST data onto it. Besides being simpler, the new code avoids a layer of copying, as well as and two round trips through malloc() and free(). Other details: 1. I rearranged the declarations of the variables sarray, buffer, key, and val, so as to narrow their scope. In the case of sarray this rearrangement avoids a potential memory leak in the case of an error exit (where the POST data is excessive). 2. I append a terminal null to the input buffer instead of using memset() to fill the entire buffer. 3. Since the bread variable is a long rather than an int, I corrected the format specifier accordingly in one of the debug messages. 4. I eliminated some redundant casts in a couple of calls to ap_unescape(), since the variables so cast are already the right types. 5. The final debug message was being issued only when sarray was not NULL. However at this spot sarray cannot be NULL anyway, so I eliminated the test of sarray. git-svn-id: svn://svn.open-ils.org/OpenSRF/trunk@1320 9efc2488-bf62-4759-914b-345cdb29e865 --- src/gateway/apachetools.c | 66 ++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 36 deletions(-) diff --git a/src/gateway/apachetools.c b/src/gateway/apachetools.c index b2cddbd..fdc4fc1 100644 --- a/src/gateway/apachetools.c +++ b/src/gateway/apachetools.c @@ -4,15 +4,8 @@ osrfStringArray* apacheParseParms(request_rec* r) { if( r == NULL ) return NULL; - char* arg = r->args; /* url query string */ + char* arg = NULL; apr_pool_t *p = r->pool; /* memory pool */ - osrfStringArray* sarray = osrfNewStringArray(12); /* method parameters */ - - growing_buffer* buffer = NULL; /* POST data */ - growing_buffer* tmp_buf = NULL; /* temp buffer */ - - char* key = NULL; /* query item name */ - char* val = NULL; /* query item value */ /* gather the post args and append them to the url query string */ if( !strcmp(r->method,"POST") ) { @@ -23,15 +16,21 @@ osrfStringArray* apacheParseParms(request_rec* r) { if(ap_should_client_block(r)) { - char body[1025]; - memset(body,0,sizeof(body)); - buffer = buffer_init(1025); + growing_buffer* buffer = buffer_init(1025); + /* Start with url query string, if any */ + + if(r->args && r->args[0]) + buffer_add(buffer, r->args); + + char body[1025]; osrfLogDebug(OSRF_LOG_MARK, "gateway client has post data, reading..."); - + + /* Append POST data */ + long bread; - while( (bread = ap_get_client_block(r, body, 1024)) ) { + while( (bread = ap_get_client_block(r, body, sizeof(body) - 1)) ) { if(bread < 0) { osrfLogInfo(OSRF_LOG_MARK, @@ -39,13 +38,11 @@ osrfStringArray* apacheParseParms(request_rec* r) { break; } + body[bread] = '\0'; buffer_add( buffer, body ); - memset(body,0,sizeof(body)); osrfLogDebug(OSRF_LOG_MARK, - "gateway read %d bytes: %d bytes of data so far", bread, buffer->n_used); - - if(buffer->n_used == 0) break; + "gateway read %ld bytes: %d bytes of data so far", bread, buffer->n_used); if(buffer->n_used > APACHE_TOOLS_MAX_POST_SIZE) { osrfLogError(OSRF_LOG_MARK, "gateway received POST larger " @@ -57,20 +54,10 @@ osrfStringArray* apacheParseParms(request_rec* r) { osrfLogDebug(OSRF_LOG_MARK, "gateway done reading post data"); - if(arg && arg[0]) { - - tmp_buf = buffer_init(1024); - buffer_add(tmp_buf,arg); - buffer_add(tmp_buf,buffer->buf); - arg = (char*) apr_pstrdup(p, tmp_buf->buf); - buffer_free(tmp_buf); - - } else if(buffer->n_used > 0){ - arg = (char*) apr_pstrdup(p, buffer->buf); - - } else { + if(buffer->n_used > 0) + arg = apr_pstrdup(p, buffer->buf); + else arg = NULL; - } buffer_free(buffer); } @@ -83,17 +70,25 @@ osrfStringArray* apacheParseParms(request_rec* r) { return NULL; } - osrfLogDebug(OSRF_LOG_MARK, "parsing URL params from post/get request data: %s", arg); + + osrfStringArray* sarray = osrfNewStringArray(12); /* method parameters */ int sanity = 0; + char* key = NULL; /* query item name */ + char* val = NULL; /* query item value */ + + /* Parse the post/get request data into a series of name/value pairs. */ + /* Load each name into an even-numbered slot of an osrfStringArray, and */ + /* the corresponding value into the following odd-numbered slot. */ + while( arg && (val = ap_getword(p, (const char**) &arg, '&'))) { key = ap_getword(r->pool, (const char**) &val, '='); if(!key || !key[0]) break; - ap_unescape_url((char*)key); - ap_unescape_url((char*)val); + ap_unescape_url(key); + ap_unescape_url(val); osrfLogDebug(OSRF_LOG_MARK, "parsed URL params %s=%s", key, val); @@ -109,9 +104,8 @@ osrfStringArray* apacheParseParms(request_rec* r) { } - if(sarray) - osrfLogDebug(OSRF_LOG_MARK, - "Apache tools parsed %d params key/values", sarray->size / 2 ); + osrfLogDebug(OSRF_LOG_MARK, + "Apache tools parsed %d params key/values", sarray->size / 2 ); return sarray; } -- 2.43.2