On 03/05/2015 10:01 AM, Yann Ylavic wrote: > On Thu, Mar 5, 2015 at 9:38 AM, Ruediger Pluem <[email protected]> wrote: >> >> Don't we need to have the following in addition to avoid a crash in another >> path? >> >> >> Index: protocol.c >> =================================================================== >> --- protocol.c (revision 1664261) >> +++ protocol.c (working copy) >> @@ -674,6 +674,8 @@ >> ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02418) >> "Invalid protocol '%s'", r->protocol); >> if (enforce_strict) { >> + r->protocol = apr_pstrdup(r->pool, "HTTP/1.0"); >> + r->proto_num = HTTP_VERSION(1,0); >> r->status = HTTP_BAD_REQUEST; >> return 0; >> } >> > > r->protocol is initialized above in any case (with an invalid value > but it is), so it won't crash. > But r->proto_num is still 0 (which, if I read output filter chain > correctly, will lead to a HTTP/1.1 response), though. > So we probably should keep the original value in r->protocol (for > logging), but force proto_num = HTTP_VERSION(1,0).
I saw that r->protocol is initialized in this case, but I wanted to keep it consistent with r->proto_num in this case. Hence the reset to "HTTP/1.0". If we are interested in logging we might should increase the severity of the error message above from DEBUG to something more visible e.g. INFO at least in the enforce_strict case. Regards Rüdiger
