Hello! On Mon, Sep 01, 2014 at 11:34:07AM +0100, Nick Kew wrote:
> On Fri, 2014-08-29 at 17:26 +0400, Maxim Dounin wrote: > > > In either case, you may want to consider using request body > > filters instead, as recently discussed here: > > > > http://mailman.nginx.org/pipermail/nginx-devel/2014-August/005781.html > > If I'm to use such a feature when it's uncommitted because you're > not sure it's a good way to implement input filters, I'd like at > least to make it easily switchable. > > Would you consider adding a #define to detect the patch > and simplify compile-time selection of code? > Something like: > > $ diff -u configure* > --- configure 2014-08-05 12:13:05.000000000 +0100 > +++ configure.patched 2014-09-01 11:25:21.128430978 +0100 > @@ -112,4 +112,6 @@ > have=NGX_BUILD value="\"$NGX_BUILD\"" . auto/define > fi > > +have=NGX_INPUT_FILTERS value=1358156187 . auto/define > + > . auto/summary There is no real difference with just committing it as is. Once committed, you can test nginx_version as it's usually done with all API changes/new features. Further work in this area is expected to happen in near future (at least I hope so). > (That's against the 1.7.4 tarball: I didn't find anything looking > like a dev repo). Repository is linked from http://nginx.org/en/download.html. See http://nginx.org/en/docs/contributing_changes.html for tips. > > Reading and then writing temorary files will be suboptimal. And > > Yes of course. I was just guessing that the alternative of buffering > it all in memory would be worse; else nginx wouldn't've used a tempfile > in the first place! > > I can also see a minor advantage to all-at-once filtering, in that > it means the filter can take responsibility for administrivia like > setting a correct Content-Length for the possibly-changed payload. This should be possible with request body filters, too. E.g., chunked request body filter currently set Content-Length. > > replacing r->request_body->temp_file, even if you'll be able to do > > it properly, will likely result in your module being broken during > > further nginx development. > > That's a risk with any patch, and surely applies even to your own > uncommitted input-filtering patch? The problem is that what you are trying to do with r->request_body->temp_file is certainly out of the scope of what is considered to be nginx modules API. And such a code has much higher chances of being broken. -- Maxim Dounin http://nginx.org/ _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
