Right. That's why I'm saying we should change core_output_filter as I described (so that it returns APR_ECONNABORTED in this case). (Don't think we're disagreeing on that.)The only filter I changed was the content-length filter. But yes I would agree that in a commit which fixes our handlers as you mentioned we also fix the content-length filter to not worry about c->aborted.Note that I changed the mod_cgi handler to look at c->aborted as well as rv after ap_pass_brigade(). In all honesty I don't know who gets to set c->aborted and whether rv absolutely must be non-APR_SUCCESS if aborted is set. But I suspect that mod_cgi wouldn't have to check both rv and c->aborted to see if an error occurred talking to the client.
Well, he ain't here no more. Just us folks. Do you (or anyone else) see a reason why we shouldn't change this?I don't buy that logic at all. Why should we be returning APR_SUCCESS here? We want to signal an error, so that the filters stop what they are doing and exit.Talk to Ryan :)
How do we differentiate between a 200 that all the data was sent for and a 200 where the client aborted? If someone looked at the access logs, they would see that the transmitted length wasn't correct in the aborted case. Perhaps that was Ryan's initial motivation - did he want the length to be what we wanted to send not what we actually sent? (mod_logio is bringing up some of these concerns as well.)I'm kinda stuck thinking that the philosophy should be that we log whatever was written to the client in the status line, but there are some implementation details with that one.
I guess I'd like to see some notation in access log that indicates, "Hey, we know we didn't send everything, but that's because they left in the middle." Yes, it also might be noted in the error log, but I gotta believe it's goodness to have tools looking at just the access logs know that the client aborted.
Makes sense. So, core_output_filter should be calling ap_log_rerror. Hmm. Does it have access to the request_rec? Didn't we change that? Or, was I vetoed on that? -- justinOne issue with that is that the filter that encounters the error really should do the logging so that the most specific information is available. If some other code also logs less-specific info, then it is all for naught.
