Re: APR_BRIGADE_NORMALIZE
i thought it was added as a workaround during one of the mod_ssl filter rewrites. during the last one i tried removing APR_BRIGADE_NORMALIZE from core.c and all tests in httpd-test passed except for protocol/echo and protocol/nntp_like (which are the same code in the place where the problem shows up). so you might want to try debugging with one of those tests to find the culprit.
Re: APR_BRIGADE_NORMALIZE
On Mon, 21 Jan 2002, Justin Erenkrantz wrote: > 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. The reason for normalize was to make the code easier to read and understand. Every filter must be able to deal with 0 length buckets, because nobody knows everytime that they will be inserted. Remember that just because we have written a core_input_filter, that doesn't mean it will always be used, it is easy enough for anybody to replace it. The easiest way to deal with 0 length buckets is to remove them from the brigade. However, you are correct that there are a lot of problems with just removing them, so we should fix the code instead of hacking around it. Ryan
Re: APR_BRIGADE_NORMALIZE
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
Re: APR_BRIGADE_NORMALIZE
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. There was a bug somewhere with zero length buckets. I don't remember, and I bet it doesn't exist now. I say torch the damned thing. You're quite right: removing zero length buckets is Bad Juju. Cheers, -g -- Greg Stein, http://www.lyra.org/
APR_BRIGADE_NORMALIZE
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. No? --Cliff -- Cliff Woolley [EMAIL PROTECTED] Charlottesville, VA
