nginx tempfiles

[Maxim Dounin]

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/