On Thu, Mar 5, 2015 at 10:09 AM, Ruediger Pluem <[email protected]> wrote: > > > 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.
I meant *custom* logging of the original request fields, but r->the_request will do the job in this case. So it's indeed probably better to keep things consistent internally.
