On Fri, Nov 14, 2014 at 11:25 PM, Jeff Trawick <[email protected]> wrote: > On Fri, Nov 14, 2014 at 1:18 PM, <[email protected]> wrote: [] >> -static int handle_headers(request_rec *r, >> - int *state, >> - char *readbuf) >> +static int handle_headers(request_rec *r, int *state, >> + char *readbuf, apr_size_t readlen) >> { >> const char *itr = readbuf; >> >> - while (*itr) { >> + while (readlen) { >> if (*itr == '\r') { >> switch (*state) { >> case HDR_STATE_GOT_CRLF: >> @@ -443,13 +442,17 @@ static int handle_headers(request_rec *r >> break; >> } >> } >> - else { >> + else if (*itr == '\t' || !apr_iscntrl(*itr)) { >> *state = HDR_STATE_READING_HEADERS; >> } >> + else { >> + return -1; >> + } > [] > I guess I don't understand the importance of this code which returns an > error on non-tab control characters. Why isn't the character checking the > same as in ap_scan_script_header_err_brigade_ex() since that is what will > finally parse the buffer?
Well, the previous code was stopping on '\0' (hopefully a '\n' was reached before), and I wanted to preserve this behaviour. While at it, I chose to also stop on any control char since they are not valid in HTTP headers (but '\t'). getsfunc_BRIGADE() does not seem to care about those, and is not supposed to return anything but -1 (timeout) as error... Isn't handle_headers() handling HTTP headers? If not I'm afraid I overdid that fix.
