mdounin at mdounin.ru
Mon Sep 1 12:38:08 UTC 2014
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
> +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.
More information about the nginx-devel