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

diff --git a/src/gateway/apachetools.c b/src/gateway/apachetools.c
index b2cddbd..fdc4fc1 100644 (file)
--- 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;
 }