You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2008/04/09 13:39:59 UTC

svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Author: minfrin
Date: Wed Apr  9 04:39:58 2008
New Revision: 646281

URL: http://svn.apache.org/viewvc?rev=646281&view=rev
Log:
Add a function to the http filters that is able to parse an HTML
form request with the type of application/x-www-form-urlencoded.

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/include/http_protocol.h
    httpd/httpd/trunk/modules/http/http_filters.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=646281&r1=646280&r2=646281&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Wed Apr  9 04:39:58 2008
@@ -2,6 +2,10 @@
 Changes with Apache 2.3.0
 [ When backported to 2.2.x, remove entry from this file ]
 
+  *) Add a function to the http filters that is able to parse an HTML
+     form request with the type of application/x-www-form-urlencoded.
+     [Graham Leggett]
+
   *) mod_session_crypto: Initialise SSL in the post config hook.
      [Ruediger Pluem, Graham Leggett]
 

Modified: httpd/httpd/trunk/include/http_protocol.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_protocol.h?rev=646281&r1=646280&r2=646281&view=diff
==============================================================================
--- httpd/httpd/trunk/include/http_protocol.h (original)
+++ httpd/httpd/trunk/include/http_protocol.h Wed Apr  9 04:39:58 2008
@@ -669,7 +669,55 @@
  * @param send_headers Whether to send&clear headers in r->headers_out
  */
 AP_DECLARE(void) ap_send_interim_response(request_rec *r, int send_headers);
-                                                                                
+
+/**
+ * Structure to store the contents of an HTTP form of the type
+ * application/x-www-form-urlencoded.
+ * 
+ * Currently it contains the name as a char* of maximum length
+ * HUGE_STRING_LEN, and a value in the form of a bucket brigade
+ * of arbitrary length.
+ */
+typedef struct {
+    const char *name;
+    apr_bucket_brigade *value;
+} ap_form_pair_t;
+
+/**
+ * Read the body and parse any form found, which must be of the
+ * type application/x-www-form-urlencoded.
+ *
+ * Name/value pairs are returned in an array, with the names as
+ * strings with a maximum length of HUGE_STRING_LEN, and the
+ * values as bucket brigades. This allows values to be arbitrarily
+ * large.
+ *
+ * All url-encoding is removed from both the names and the values
+ * on the fly. The names are interpreted as strings, while the
+ * values are interpreted as blocks of binary data, that may
+ * contain the 0 character.
+ *
+ * In order to ensure that resource limits are not exceeded, a
+ * maximum size must be provided. If the sum of the lengths of
+ * the names and the values exceed this size, this function
+ * will return HTTP_REQUEST_ENTITY_TOO_LARGE.
+ *
+ * An optional number of parameters can be provided, if the number
+ * of parameters provided exceeds this amount, this function will
+ * return HTTP_REQUEST_ENTITY_TOO_LARGE. If this value is negative,
+ * no limit is imposed, and the number of parameters is in turn
+ * constrained by the size parameter above.
+ * 
+ * This function honours any kept_body configuration, and the
+ * original raw request body will be saved to the kept_body brigade
+ * if so configured, just as ap_discard_request_body does.
+ * 
+ * NOTE: File upload is not yet supported, but can be without change
+ * to the function call.
+ */
+AP_DECLARE(int) ap_parse_request_form(request_rec * r, apr_array_header_t ** ptr,
+                                      apr_size_t num, apr_size_t size);
+
 #ifdef __cplusplus
 }
 #endif

Modified: httpd/httpd/trunk/modules/http/http_filters.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=646281&r1=646280&r2=646281&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_filters.c (original)
+++ httpd/httpd/trunk/modules/http/http_filters.c Wed Apr  9 04:39:58 2008
@@ -1724,3 +1724,240 @@
     return APR_SUCCESS;
 
 }
+
+/* form parsing stuff */
+typedef enum {
+    FORM_NORMAL,
+    FORM_AMP,
+    FORM_NAME,
+    FORM_VALUE,
+    FORM_PERCENTA,
+    FORM_PERCENTB,
+    FORM_ABORT
+} ap_form_type_t;
+
+/**
+ * Read the body and parse any form found, which must be of the
+ * type application/x-www-form-urlencoded.
+ *
+ * Name/value pairs are returned in an array, with the names as
+ * strings with a maximum length of HUGE_STRING_LEN, and the
+ * values as bucket brigades. This allows values to be arbitrarily
+ * large.
+ *
+ * All url-encoding is removed from both the names and the values
+ * on the fly. The names are interpreted as strings, while the
+ * values are interpreted as blocks of binary data, that may
+ * contain the 0 character.
+ *
+ * In order to ensure that resource limits are not exceeded, a
+ * maximum size must be provided. If the sum of the lengths of
+ * the names and the values exceed this size, this function
+ * will return HTTP_REQUEST_ENTITY_TOO_LARGE.
+ *
+ * An optional number of parameters can be provided, if the number
+ * of parameters provided exceeds this amount, this function will
+ * return HTTP_REQUEST_ENTITY_TOO_LARGE. If this value is negative,
+ * no limit is imposed, and the number of parameters is in turn
+ * constrained by the size parameter above.
+ * 
+ * This function honours any kept_body configuration, and the
+ * original raw request body will be saved to the kept_body brigade
+ * if so configured, just as ap_discard_request_body does.
+ * 
+ * NOTE: File upload is not yet supported, but can be without change
+ * to the function call.
+ */
+AP_DECLARE(int) ap_parse_request_form(request_rec * r, apr_array_header_t ** ptr,
+                                      apr_size_t num, apr_size_t size)
+{
+    core_dir_conf *dconf;
+    apr_off_t left = 0;
+    apr_bucket_brigade *bb = NULL, *kept_body = NULL;
+    apr_bucket *e;
+    int seen_eos = 0;
+    char buffer[HUGE_STRING_LEN + 1];
+    const char *ct;
+    apr_size_t offset = 0;
+    ap_form_type_t state = FORM_NAME, percent = FORM_NORMAL;
+    ap_form_pair_t *pair = NULL;
+    apr_array_header_t *pairs = apr_array_make(r->pool, 4, sizeof(ap_form_pair_t));
+
+    char hi = 0;
+    char low = 0;
+
+    *ptr = pairs;
+
+    /* sanity check - we only support forms for now */
+    ct = apr_table_get(r->headers_in, "Content-Type");
+    if (!ct || strcmp("application/x-www-form-urlencoded", ct)) {
+        return ap_discard_request_body(r);
+    }
+
+    dconf = ap_get_module_config(r->per_dir_config,
+                                     &http_module);
+    if (dconf->keep_body > 0) {
+        left = dconf->keep_body;
+        kept_body = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+    }
+
+    bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+    do {
+        apr_bucket *bucket = NULL, *last = NULL;
+
+        int rv = ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
+                                APR_BLOCK_READ, HUGE_STRING_LEN);
+        if (rv != APR_SUCCESS) {
+            apr_brigade_destroy(bb);
+            return (rv == AP_FILTER_ERROR) ? rv : HTTP_BAD_REQUEST;
+        }
+
+        for (bucket = APR_BRIGADE_FIRST(bb);
+             bucket != APR_BRIGADE_SENTINEL(bb);
+             last = bucket, bucket = APR_BUCKET_NEXT(bucket)) {
+            const char *data;
+            apr_size_t len, slide;
+
+            if (last) {
+                apr_bucket_delete(last);
+            }
+            if (APR_BUCKET_IS_EOS(bucket)) {
+                seen_eos = 1;
+                break;
+            }
+            if (bucket->length == 0) {
+                continue;
+            }
+
+            rv = apr_bucket_read(bucket, &data, &len, APR_BLOCK_READ);
+            if (rv != APR_SUCCESS) {
+                apr_brigade_destroy(bb);
+                return HTTP_BAD_REQUEST;
+            }
+
+            slide = len;
+            while (state != FORM_ABORT && slide-- > 0 && size >= 0 && num != 0) {
+                char c = *data++;
+                if ('+' == c) {
+                    c = ' ';
+                }
+                else if ('&' == c) {
+                    state = FORM_AMP;
+                }
+                if ('%' == c) {
+                    percent = FORM_PERCENTA;
+                    continue;
+                }
+                if (FORM_PERCENTA == percent) {
+                    if (c >= 'a') {
+                        hi = c - 'a' + 10;
+                    }
+                    else if (c >= 'A') {
+                        hi = c - 'A' + 10;
+                    }
+                    else if (c >= '0') {
+                        hi = c - '0';
+                    }
+                    hi = hi << 4;
+                    percent = FORM_PERCENTB;
+                    continue;
+                }
+                if (FORM_PERCENTB == percent) {
+                    if (c >= 'a') {
+                        low = c - 'a' + 10;
+                    }
+                    else if (c >= 'A') {
+                        low = c - 'A' + 10;
+                    }
+                    else if (c >= '0') {
+                        low = c - '0';
+                    }
+                    c = low ^ hi;
+                    percent = FORM_NORMAL;
+                }
+                switch (state) {
+                case FORM_AMP:
+                    if (pair) {
+                        const char *tmp = apr_pmemdup(r->pool, buffer, offset);
+                        apr_bucket *b = apr_bucket_pool_create(tmp, offset, r->pool, r->connection->bucket_alloc);
+                        APR_BRIGADE_INSERT_TAIL(pair->value, b);
+                    }
+                    state = FORM_NAME;
+                    pair = NULL;
+                    offset = 0;
+                    num--;
+                    break;
+                case FORM_NAME:
+                    if (offset < HUGE_STRING_LEN) {
+                        if ('=' == c) {
+                            buffer[offset] = 0;
+                            offset = 0;
+                            pair = (ap_form_pair_t *) apr_array_push(pairs);
+                            pair->name = apr_pstrdup(r->pool, buffer);
+                            pair->value = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+                            state = FORM_VALUE;
+                        }
+                        else {
+                            buffer[offset++] = c;
+                            size--;
+                        }
+                    }
+                    else {
+                        state = FORM_ABORT;
+                    }
+                    break;
+                case FORM_VALUE:
+                    if (offset >= HUGE_STRING_LEN) {
+                        const char *tmp = apr_pmemdup(r->pool, buffer, offset);
+                        apr_bucket *b = apr_bucket_pool_create(tmp, offset, r->pool, r->connection->bucket_alloc);
+                        APR_BRIGADE_INSERT_TAIL(pair->value, b);
+                        offset = 0;
+                    }
+                    buffer[offset++] = c;
+                    size--;
+                    break;
+                default:
+                    break;
+                }
+            }
+
+            /* If we have been asked to, keep the data up until the
+             * configured limit. If the limit is exceeded, we return an
+             * HTTP_REQUEST_ENTITY_TOO_LARGE response so the caller is
+             * clear the server couldn't handle their request.
+             */
+            if (kept_body) {
+                if (len <= left) {
+                    apr_bucket_copy(bucket, &e);
+                    APR_BRIGADE_INSERT_TAIL(kept_body, e);
+                    left -= len;
+                }
+                else {
+                    apr_brigade_destroy(bb);
+                    apr_brigade_destroy(kept_body);
+                    return HTTP_REQUEST_ENTITY_TOO_LARGE;
+                }
+            }
+
+        }
+
+        apr_brigade_cleanup(bb);
+    } while (!seen_eos);
+
+    if (FORM_ABORT == state || size < 0 || num == 0) {
+        return HTTP_REQUEST_ENTITY_TOO_LARGE;
+    }
+    else if (FORM_VALUE == state && pair && offset > 0) {
+        const char *tmp = apr_pmemdup(r->pool, buffer, offset);
+        apr_bucket *b = apr_bucket_pool_create(tmp, offset, r->pool, r->connection->bucket_alloc);
+        APR_BRIGADE_INSERT_TAIL(pair->value, b);
+    }
+
+    if (kept_body) {
+        r->kept_body = kept_body;
+    }
+
+    return OK;
+
+}
+



Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Apr 11, 2008, at 8:46 PM, Graham Leggett wrote:
> Roy T. Fielding wrote:
>
>> New features belong in modules so that they don't get compiled into
>> the server by those of us who don't want that feature.  I don't  
>> recall
>> why or how KeptBodySize made it into http_filter (where it does  
>> not belong),
>> but I am quite certain that the core will not be parsing HTTP body  
>> content
>> any time soon.  So, either find another way or remove the feature  
>> as well.
>
> Done (r647263).

Thanks Graham,

....Roy


Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by Graham Leggett <mi...@sharp.fm>.
Roy T. Fielding wrote:

> New features belong in modules so that they don't get compiled into
> the server by those of us who don't want that feature.  I don't recall
> why or how KeptBodySize made it into http_filter (where it does not 
> belong),
> but I am quite certain that the core will not be parsing HTTP body content
> any time soon.  So, either find another way or remove the feature as well.

Done (r647263).

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by Graham Leggett <mi...@sharp.fm>.
Roy T. Fielding wrote:

> New features belong in modules so that they don't get compiled into
> the server by those of us who don't want that feature.  I don't recall
> why or how KeptBodySize made it into http_filter (where it does not 
> belong),
> but I am quite certain that the core will not be parsing HTTP body content
> any time soon.  So, either find another way or remove the feature as well.

Before I waste another few weeks of my time finding a way only to have 
it shot down right at the end, I would appreciate some help with finding 
out a way to do this in advance.

util_filter.c is actually http_module, which is not the core, or do you 
disagree, yes?

If I can find a way to introduce another module into modules/http to 
provide HTTP support services like the KeptBody stuff and the ability to 
parse a form, which an admin can choose to not enable, will this be 
sufficient?

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Apr 9, 2008, at 3:10 PM, Graham Leggett wrote:
> Roy T. Fielding wrote:
>
>> -1.  Bloat like this belongs in a module.
>
> This piece of code depends on the KeptBodySize directive, which is  
> part of the http_filter, and sits alongside ap_discard_request_body().
>
> I can move it into another module, but then that just gives the  
> administrator one extra thing to configure incorrectly.
>
> Putting it in mod_auth_form meant that mod_auth_form would be  
> delving into the internals of the http_filter module, which seems  
> wrong to me. Rather let http_filter export a formal function to the  
> world, than having external modules poking around inside it.

New features belong in modules so that they don't get compiled into
the server by those of us who don't want that feature.  I don't recall
why or how KeptBodySize made it into http_filter (where it does not  
belong),
but I am quite certain that the core will not be parsing HTTP body  
content
any time soon.  So, either find another way or remove the feature as  
well.

....Roy


Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by Graham Leggett <mi...@sharp.fm>.
Akins, Brian wrote:

>> This kind of stuff belongs in a
>> common place.
> 
> Doesn't libapreq do this??

Do what exactly?

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by "Akins, Brian" <Br...@turner.com>.
On 4/10/08 7:05 AM, "Graham Leggett" <mi...@sharp.fm> wrote:

> This kind of stuff belongs in a
> common place.

Doesn't libapreq do this??

-- 
Brian Akins
Chief Operations Engineer
Turner Digital Media Technologies


Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by Graham Leggett <mi...@sharp.fm>.
Nick Kew wrote:

> c.f. my mod_form (at apache.webthing.com), which does just that,
> providing optional functions for other modules to access the
> form data.

This is upside down - modules should not have to do anything at all to 
get a possibly inserted request body or form data. What happens when a 
potential mod_auth_openid comes along, now modules have to look in two 
potential places for the body, not one? This kind of stuff belongs in a 
common place.

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by Nick Kew <ni...@webthing.com>.
On 9 Apr 2008, at 17:33, Joe Orton wrote:
> On Wed, Apr 09, 2008 at 05:07:33PM +0200, Graham Leggett wrote:
>> Joe Orton wrote:
>>
>>> I don't understand why *that* stuff needed to be in the core.  It is
>>> certainly possible to consume then reinject the request body,  
>>> without
>>> changing one line of core filters; it's done in mod_ssl, see
>>> ssl_io_filter_buffer/ssl_io_buffer_fill.
>>
>> What about modules that call ap_discard_request_body? Once the  
>> body has
>> been discarded, the body is discarded, there is no way to get it  
>> back.
>
> The entire request body passes through the input filter stack when  
> it is
> being discarded.  So an input filter inserted by a module can capture
> and buffer it somewhere very easily; and can provide access to that
> buffered copy however it likes, without needing to modify request_rec.

c.f. my mod_form (at apache.webthing.com), which does just that,
providing optional functions for other modules to access the
form data.

mod_form isn't (as it stands) clean enough to propose for
inclusion in httpd.  But I think it would be a pretty trivial job
either to fix or duplicate it.  And I'm happy for a module derived
from it to be ASL-licensed if it's being dropped into httpd.

-- 
Nick Kew



Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by Paul Querna <ch...@force-elite.com>.
Graham Leggett wrote:
> Joe Orton wrote:
> 
>> The entire request body passes through the input filter stack when it 
>> is being discarded.  So an input filter inserted by a module can 
>> capture and buffer it somewhere very easily; and can provide access to 
>> that buffered copy however it likes, without needing to modify 
>> request_rec.
> 
> In practise it is not that simple[1].
> 
> If mod_include had its own kept_body filter, it would not take into 
> account the magic that mod_auth_form performs to keep the contents of 
> the end user's form from being lost.
> 
> In other words, modules need to cooperate with one another to make this 
> work effectively, and that means common code.
> 
> So where does this common code go?

apreq handles this.

import apreq to turnk.

IMO.

-Paul

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by Graham Leggett <mi...@sharp.fm>.
Joe Orton wrote:

> The entire request body passes through the input filter stack when it is 
> being discarded.  So an input filter inserted by a module can capture 
> and buffer it somewhere very easily; and can provide access to that 
> buffered copy however it likes, without needing to modify request_rec.

In practise it is not that simple[1].

If mod_include had its own kept_body filter, it would not take into 
account the magic that mod_auth_form performs to keep the contents of 
the end user's form from being lost.

In other words, modules need to cooperate with one another to make this 
work effectively, and that means common code.

So where does this common code go?

HTTP request bodies being most closely associated with the HTTP 
protocol, the http_module seemed the most logical place for this 
functionality.

Others may disagree that the http_module is the best place for this, and 
that is perfectly fine by me. I am open to suggestions for alternatives.

Just to be clear: the current intention is to add it to the HTTP module, 
*not* the core.

The fact that the HTTP module still is half melted into a core is a 
separate problem.

[1] Making KeptBodySize work was like getting blood out of a stone.

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Apr 09, 2008 at 05:07:33PM +0200, Graham Leggett wrote:
> Joe Orton wrote:
>
>> I don't understand why *that* stuff needed to be in the core.  It is 
>> certainly possible to consume then reinject the request body, without 
>> changing one line of core filters; it's done in mod_ssl, see 
>> ssl_io_filter_buffer/ssl_io_buffer_fill.
>
> What about modules that call ap_discard_request_body? Once the body has 
> been discarded, the body is discarded, there is no way to get it back.

The entire request body passes through the input filter stack when it is 
being discarded.  So an input filter inserted by a module can capture 
and buffer it somewhere very easily; and can provide access to that 
buffered copy however it likes, without needing to modify request_rec.

joe

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by Graham Leggett <mi...@sharp.fm>.
Joe Orton wrote:

> I don't understand why *that* stuff needed to be in the core.  It is 
> certainly possible to consume then reinject the request body, without 
> changing one line of core filters; it's done in mod_ssl, see 
> ssl_io_filter_buffer/ssl_io_buffer_fill.

What about modules that call ap_discard_request_body? Once the body has 
been discarded, the body is discarded, there is no way to get it back.

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Apr 09, 2008 at 03:10:25PM +0200, Graham Leggett wrote:
> Roy T. Fielding wrote:
>
>> -1.  Bloat like this belongs in a module.
>
> This piece of code depends on the KeptBodySize directive, which is part of 
> the http_filter, and sits alongside ap_discard_request_body().

I don't understand why *that* stuff needed to be in the core.  It is 
certainly possible to consume then reinject the request body, without 
changing one line of core filters; it's done in mod_ssl, see 
ssl_io_filter_buffer/ssl_io_buffer_fill.

joe

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by Graham Leggett <mi...@sharp.fm>.
Roy T. Fielding wrote:

> -1.  Bloat like this belongs in a module.

This piece of code depends on the KeptBodySize directive, which is part 
of the http_filter, and sits alongside ap_discard_request_body().

I can move it into another module, but then that just gives the 
administrator one extra thing to configure incorrectly.

Putting it in mod_auth_form meant that mod_auth_form would be delving 
into the internals of the http_filter module, which seems wrong to me. 
Rather let http_filter export a formal function to the world, than 
having external modules poking around inside it.

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
-1.  Bloat like this belongs in a module.

....Roy