On Tue, Apr 02, 2002 at 01:50:59PM -0800, Ryan Bloom wrote:
>...
> > chain.  Bill S. stuck his head in here and said something about a rule
> > that if old_write is ever used, it has to always be used.
> 
> If you read through mod_include, you will see that whenever we create a
> sub-request, we send the output from the current filter to the next
> filter.  This is required to make sure that things are output in the
> correct order.  If you are a handler and are generating data through
> ap_r* functions, you must flush the filter stack so that you are sure
> that OLD_WRITE isn't buffering for you.

That's not true. OLD_WRITE was designed to allow a mix/match of writing to
the stack *and* using the ap_r* functions.

The design is that ap_r* implies a write into the filter stack. That write
is optimized to use a brigade for buffering (so every ap_rputc() doesn't
traverse the filter stack). If OLD_WRITE isn't at the top, then it doesn't
do the buffering (because there are filters between the top and the
OLD_WRITE buffers).

If somebody writes directly into the filter stack, then it prepends any
buffered content to whatever is written, and then passes it all down the
stack.

[ just reviewed the code; it seems to still match the original design ]

It all looks pretty good, but there might be a problem where:

*) ap_r* is used, so OLD_WRITE is inserted, and the content is stored into
   its buffer.

*) another filter is inserted "above" OLD_WRITE, so it is not the top. ap_r*
   content should now go into this new filter.

*) ap_r* is called again, but it thinks it cannot buffer. it passes the new
   data directly into the filter stack.
   
   BUG: the previously-buffered content is not prepended.


So... the bug might appear because of the increased dynamicity of the filter
chain nowadays. Back when OLD_WRITE was written, it was perceived that the
chain would be quite static by the time the first write occurred.

Note that OLD_WRITE tries quite hard to keep its filter on "top" (its filter
type is RESOURCE-10). If something broke that, then it is possible to hit
the above bug.

The fix to the (potential) bug is to change line 1370 in server/protocol.c
to concat the brigades, similar to ap_old_write_filter() (defined just
above, at line 1316).

Another question is whether anybody defines their filter type as less than
RESOURCE-10 and ends up ahead of OLD_WRITE.

And lastly: I just realized that if a filter gets in front of OLD_WRITE,
then the branch at line 1366 will get called for each ap_r* call. If that is
a series of 1000's of ap_rputc() calls, then you'll end up creating
thousands of brigades for those transient buckets(!).

[ a similar brigade over-creation can occur if you alternate ap_r* and
  writing to the filter stack; ap_old_write_filter() will forget about the
  brigade it created, so the next ap_r* needs to recreate one ]

The answer might be to test for brigade-empty rather than ctx->bb == NULL.
Then the branch at 1366 can store and reuse a brigade in ctx->bb.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Reply via email to