On Sun, Jan 20, 2002 at 10:36:47PM -0500, Cliff Woolley wrote:
>
> Can someone please remind me why APR_BRIGADE_NORMALIZE is absolutely
> necessary? It's only being used in one place AFAICT (in core.c), and as
> far as I know, that just means there's a bug we're patching around rather
> than fixing. I was grudgingly okay with it until today, when it occurred
> to me that removing all zero-length buckets would nuke any third-party
> metadata buckets that might be in the brigade, meaning that it's a really
> bad idea.
IIRC, there are two cases where a zero-length bucket can be inserted
into a brigade in core_input_filter (aka CORE_IN):
- We read the end-of-the-socket via apr_bucket_read(). Remember that
buckets may get "silently" morphed to another type. We want to
ensure that we don't get stuck into an infinite loop. We prevent
this by checking for APR_BRIGADE_EMPTY and if that's the case, we
return APR_EOF. Remember that core_input_filter doesn't insert
an EOS bucket - that is the job of HTTP_IN in Apache - all CORE_IN
knows is when we have exhausted our socket bucket (as indicated
when there are no buckets left).
Actually, in our current code when we have exhausted the socket
bucket, we'd call core_input_filter with an 0-length brigade,
check for EMPTY, then normalize it, and continue on. So, we
have one extra iteration before CORE_IN returns APR_EOF. Oh,
well.
The way to resolve this is to either stop morphing the bucket
to a 0-length immortal bucket (and somehow remove that bucket
from its brigade) or rewrite CORE_IN to not use the socket
bucket abstractions. I *really* like using the socket bucket
abstraction in CORE_IN - just call apr_bucket_read and you don't
have to deal with the socket code directly. I also don't know
how that would interfere with any "socket-compatible" layers -
like FirstBill's fast-SSL sockets (which I believe was the
rationale for Ryan's MPM rewrite a few months ago).
The other alternative would be to somehow resolve the situation
where the bucket is exhausted and remove that bucket rather than
placing a 0-length bucket in its place.
- We split a bucket at the length of the bucket. We will end up
with a bucket with the exact length as before and a 0-length
bucket. I proposed fixing the split semantics so that if the
split occurred at the length, it would be a no-op. Both
you (Cliff) and Ryan had objections to that. I don't recall
what your precise rationale was, but I dropped it and left the
NORMALIZE call in with that "This is bad" comment.
I believe in practice, the second issue happens more often than
the first. Also, since this is the "lowest" level filter - I'm
not sure that any metadata would be stripped here. Calling
normalize another place may be troublesome however - and the
issues listed here are just as pertinent there. -- justin