On 10/11/2016 7:29 a.m., Alex Rousskov wrote: > On 11/09/2016 08:16 AM, Amos Jeffries wrote: >> On 9/11/2016 3:05 a.m., Eduard Bagdasaryan wrote: >>> Also simplified and fixed headers isolating code while dealing with >>> empty (i.e. zero header fields) headers. Old httpMsgIsolateHeaders() >>> tried to re-implement header end detection/processing logic that is >>> actually covered by headersEnd(). Old httpMsgIsolateHeaders() could >>> return success for some garbage input (e.g., a buffer of several CRs) >>> even if no end of headers was found. > > >> Careful there. Since HttpMsg is actually HTTP parse logic a buffer >> containing several CR's can be a valid mime block (just empty). CR is >> part of BWS/OWS in HTTP. > > Amos, > > Do you agree that a buffer consisting of several CRs and nothing > else must not result in httpMsgIsolateHeaders() returning 1 (i.e. > "success")?
Only if all the callers of this function are handling the case where it produces non-1 and adding a terminator LF explicitly for a re-parse attempt. IIRC we made the HTTP code do that, but I'm not sure of the ICAP or HTCP code, or the Store loading code (which parses into StoreEntry/MemObject or something). If this functions output in this case is going to change then all those callers need to be checked that they cope with this change correctly. > > The above preamble text is trying to say that the old > httpMsgIsolateHeaders() could return 1 in that invalid-input case. The > text should be rephrased for clarity if that is not how you interpreted > it. Suggestions welcomed. > > > In this context, the "HTTP header(s)" is defined as > > *( header-field CR?LF ) > CR?LF > > where CR?LF is either CRLF or just LF. > > Old httpMsgIsolateHeaders() and new HttpHeader::Isolate() must extract > the mime block (i.e., the *(header-field CR?LF) part) but only if > headersEnd() finds the end of the HTTP header first. The old > httpMsgIsolateHeaders was buggy. If you believe HttpHeader::Isolate() is > buggy, please speak up! I have not seen any bugs, just the above needs doing. Amos _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
