mod_layout - what is wrong?

Thu Jan 7 02:39:14 MSK 2010

Hello!

On Wed, Jan 06, 2010 at 04:09:56PM +0100, witekfl Gazeta.pl wrote:

> Hi,
> http://rkd.republika.pl/ngx_http_layout_filter_module.c
> 
> I want to write the mod_layout.
> The idea is: write all chains to temp file, at the end read it, insert the
> header after <body> and the footer before </body>.
> Sometimes it fails. There is only the header and the footer. Why?
> Could you review it?

Some random comments in random order:

1. It's probably good idea to polish style a bit before posting 
code for review.

2. The whole idea of buffering to temporary file looks wrong.  
There is no need to buffer more than several bytes here.  And the 
only case where you need buffering is when you find something like 
"</b" at the end of buffer, and need next buffer to find out if 
it's "</body>" or "</b>".  It requires a bit more complicated 
matching code, but saves lots of resources.  See sub filter
and ssi filter for examples.

3. After writing chain to temp file you have to mark buffers in 
this chain as sent by updating buf->pos.  This will allow upper 
layers to reuse this buffers.  Note well: you probably want to 
test your code with something like

   output_buffers 1 64;

to see what happens if upper layers are out of buffers since you 
don't mark them as sent.  Hint: request will just hang awaiting 
some buffers to become free after first 64 bytes.

4. layout_ignore_uris setting doesn't fit into nginx model of 
doing things.  One should write correct configuration using 
"location" directives instead of doing some fancy filtering at 
module level.

5. You assume buffers are in memory, but do not require it.  You 
should set r->filter_need_in_memory in header filter handler.

Maxim Dounin