> > Did that patch fix the bug for everybody? If so, I want to commit it. > > I have a three hour meeting now, so I'm not going to have time to > > though. Can somebody else commit it this afternoon if it works? > > Thanks Ryan, with your patch we longer trap in the SSI testcase but > I have a question. In make_sub_request where the filter pointers > are copied, why this: > > rnew->proto_output_filters = r->connection->output_filters;
That would be a bug, and a pretty big one at that. How in the world did that slip through all this time. :-( > rnew->output_filters is pointing to the remainder of the > main chain (next_filter) and rnew->proto_output_filters is > not pointing to the proto part of it. Is this just a > placeholder in case some subrequest code tries to add > a proto filter? Ah, this is how it slipped through. Because r->output_filters is setup correctly, and r->proto_output_filters should never be modified by a sub-request, this will work. Of course, it is incorrect, and should be fixed. I'll fix it along with the other patch. Thanks for catching this. > My only other observation is that any filters added in the subreq > must only be added to the top of the rnew->output_filters > chain or must be cleanly removed before the subreq returns > otherwise the main chain will get corrupted. I haven't seen > this happen so far, so maybe this is just a hypothetical > concern. It isn't hypothetical, but it also isn't a bug we want to protect against. The thing is that the filters added in the subrequest by definition must be resource filters, which means that they must be added above protocol and connection filters. If you are adding either a protocol or connection filter in a sub request, then you are doing something incredibly wrong, and we want the server to fail. Ryan
