To clarify, the issues with handling of the buckets and the fact that buckets are not being set aside came from the original code. The question about what to do regarding f->next versus passing the data along some other way is the only one that came from the code I added to make the header optional.
Being honest, I don't fully grok all the details of the filter stack and what kinds of buckets to expect emanating from core as the request for data propagates up through the input filters, but what Rudiger has pointed out seems like a structural problem with the module that could bite users under certain circumstances. What I'm particularly unclear about is what those circumstances would be. I'll try to reply to the other thread to provide more clarity. -- Daniel Ruggeri On 1/24/2017 8:36 AM, Jim Jagielski wrote: > ++1. I know that Daniel is out of pocket for a little bit so I'l > give it a coupla more days before I "restore" to the original filter > code... >> On Jan 24, 2017, at 3:46 AM, Ruediger Pluem <rpl...@apache.org> wrote: >> >> >> >> On 01/17/2017 02:48 PM, Jim Jagielski wrote: >>>> On Jan 16, 2017, at 6:57 PM, Daniel Ruggeri <drugg...@primary.net> wrote: >>>> >>>> For the most part, yes except the portions that make the header presence >>>> optional (the HDR_MISSING case). Those were added as it came into the >>>> code base to handle a use case I was working on. I've added some >>>> comments inline since I won't have time to poke around myself for a >>>> while yet. >>>> >>>> >>>> For convenience, here's a link to the original code >>>> >>>> https://github.com/roadrunner2/mod-proxy-protocol/blob/master/mod_proxy_protocol.c >>>> >>> Would it make sense to have the "stable" version available >>> for backport, and keep in the WIP in trunk? >>> >> This would be an option, but apart from this I would like to see the WIP in >> trunk >> somehow fixed. Otherwise it is a perfect candidate for falling through the >> cracks >> and giving yet another surprise once we branch "whatever we name it" from >> trunk. >> IMHO easier to fix that now or even revert that part for the time being >> while people >> remember what is going on then later digging through it while hitting the >> issues. >> >> Regards >> >> Rüdiger