On 1/25/2017 9:02 PM, Daniel Ruggeri wrote: > On 1/25/2017 6:53 PM, Daniel Ruggeri wrote: >> I'd say that not returning until ctx->bb has enough information to >> determine if the header is present or not would be sufficient. Isn't >> this already done in the potentially repeated calls to ap_get_brigade on >> line no 1056 inside the loop verifying that ctx->done is not set? If >> we're not intentionally holding onto the data until we know it's safe, >> then I think there's a structural problem in this module that would >> require us to start doing so. > Sorry - I neglected to notice the obvious check that the brigade is not > empty JUST before this line. Nevermind this silly comment... > > Indeed, if the brigade runs dry before the minimum number of bytes > needed has been read, the data should not be deleted. > I'm thinking apr_bucket_setaside(b, f->c->pool) in place of > apr_bucket_delete(b) would be good here... I will also experiment with > having the filter ask for less data than it needs to verify these > multiple calls through the filter work as expected. >
Changes to set aside the bucket data are in r1780725. I'll await updating the 2.4 backport patch with this and the compiler warning fix when we're good on how to proceed regarding the other email I just sent. To verify behavior, I forced the filter to receive only a few bytes at a time until it "built up" enough data to evaluate the header type and pass all data set aside in bb_out when RemoteIPProxyProtocol is set to Optional. Thanks for the pointers. -- Daniel Ruggeri
