httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Kew <n...@webthing.com>
Subject Re: svn commit: r592951 - in /httpd/httpd/trunk: ./ docs/manual/mod/ include/ modules/filters/ modules/http/ server/
Date Sat, 01 Dec 2007 17:50:55 GMT
On Wed, 07 Nov 2007 23:31:10 -0000
minfrin@apache.org wrote:

> Author: minfrin
> Date: Wed Nov  7 15:31:03 2007
> New Revision: 592951
> 
> URL: http://svn.apache.org/viewvc?rev=592951&view=rev
> Log:
> core: Add the option to keep aside a request body up to a certain
> size that would otherwise be discarded, to be consumed by filters
> such as mod_include. When enabled for a directory, POST requests
> to shtml files can be passed through to embedded scripts as POST
> requests, rather being downgraded to GET requests.
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/docs/manual/mod/core.xml
>     httpd/httpd/trunk/docs/manual/mod/mod_include.xml
>     httpd/httpd/trunk/include/ap_mmn.h
>     httpd/httpd/trunk/include/httpd.h
>     httpd/httpd/trunk/modules/filters/mod_include.c
>     httpd/httpd/trunk/modules/http/http_core.c
>     httpd/httpd/trunk/modules/http/http_filters.c
>     httpd/httpd/trunk/modules/http/mod_core.h
>     httpd/httpd/trunk/server/request.c

Um, right.

You created a new kept_body input filter, which seems exactly right
to me.  But you're storing the kept body on the request_rec itself,
which looks like extra complexity.

Why not make the new filter into its own module, and keep the kept_body
brigade on the filter's own config?  That way the complexity of dealing
with top-level vs other requests is kept in one place.  It's not clear
to me from the changes in http_filter.h that this patch deals with
that distinction (or is kept_body supposed to be able to originate
in non-origin requests)?

> Modified: httpd/httpd/trunk/include/httpd.h
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=592951&r1=592950&r2=592951&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/httpd.h (original) +++
> httpd/httpd/trunk/include/httpd.h Wed Nov  7 15:31:03 2007 @@ -988,6
> +988,9 @@ /** A flag to determine if the eos bucket has been sent yet
> */ int eos_sent;
>  
> +    /** The optional kept body of the request. */
> +    apr_bucket_brigade *kept_body;
> +
>  /* Things placed at the end of the record to avoid breaking binary
>   * compatibility.  It would be nice to remember to reorder the entire
>   * record to improve 64bit alignment the next time we need to break

On principle, we should be reducing the request_rec, not adding to it!

Modified: httpd/httpd/trunk/server/request.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=592951&r1=592950&r2=592951&view=diff
==============================================================================
--- httpd/httpd/trunk/server/request.c (original) +++
httpd/httpd/trunk/server/request.c Wed Nov  7 15:31:03 2007 @@ -1566,6
+1566,16 @@
      * until some module interjects and changes the value.
      */
     rnew->used_path_info = AP_REQ_DEFAULT_PATH_INFO;
+    
+    /* Pass on the kept body (if any) into the new request. */
+    rnew->kept_body = r->kept_body;

Shouldn't this happen inside the filter, with logic something like
   if (not an origin request) {
      assign kept_body from origin;
   }

-- 
Nick Kew

Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/

Mime
View raw message