Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-27 Thread Amos Jeffries
On 26/08/2016 5:43 a.m., Alex Rousskov wrote: > On 08/25/2016 10:26 AM, Amos Jeffries wrote: >> About >> the only further optimization we can do there is make the >> "CharacterSet::SP" that it outputs in the sensitive path be a local >> static *within* DelimiterCharacters() itself and return a

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-25 Thread Alex Rousskov
On 08/25/2016 10:26 AM, Amos Jeffries wrote: > About > the only further optimization we can do there is make the > "CharacterSet::SP" that it outputs in the sensitive path be a local > static *within* DelimiterCharacters() itself and return a reference to > that instead of constructing a new

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-25 Thread Alex Rousskov
On 08/25/2016 10:26 AM, Amos Jeffries wrote: > 2016-08-23 17:50 GMT+03:00 Alex Rousskov: >> I wonder whether we should make this variable static to avoid repeated >> function calls on a performance-sensitive code path. > The output of DelimiterCharacters() cannot be stored in a static because >

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-25 Thread Amos Jeffries
On 25/08/2016 11:31 a.m., Eduard Bagdasaryan wrote: > 2016-08-23 17:50 GMT+03:00 Alex Rousskov: > >> s/request-line/request-line: URI/ for consistency and clarity sake. > >> I wonder whether we should make this variable static to avoid repeated >> function calls on a performance-sensitive code

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-24 Thread Eduard Bagdasaryan
2016-08-23 17:50 GMT+03:00 Alex Rousskov : > s/request-line/request-line: URI/ for consistency and clarity sake. > I wonder whether we should make this variable static to avoid repeated > function calls on a performance-sensitive code path. Same for the old >

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-24 Thread Alex Rousskov
On 08/24/2016 08:30 AM, Amos Jeffries wrote: > On 25/08/2016 12:36 a.m., Eduard Bagdasaryan wrote: >> 2016-08-23 18:01 GMT+03:00 Alex Rousskov: >> >>> invalid request-line: missing delimiter before "HTTP/1" >> >> In order to generate "where" with such detalization (i.e. the specific >> protocol

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-24 Thread Amos Jeffries
On 25/08/2016 12:36 a.m., Eduard Bagdasaryan wrote: > 2016-08-23 18:01 GMT+03:00 Alex Rousskov: > >> invalid request-line: missing delimiter before "HTTP/1" > > In order to generate "where" with such detalization (i.e. the specific > protocol version or method) we would need to pass

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-24 Thread Alex Rousskov
On 08/24/2016 06:36 AM, Eduard Bagdasaryan wrote: > 2016-08-23 18:01 GMT+03:00 Alex Rousskov > : > >> invalid request-line: missing delimiter before "HTTP/1" > > In order to generate "where" with such detalization (i.e. the specific > protocol version or method)

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-24 Thread Eduard Bagdasaryan
2016-08-23 18:01 GMT+03:00 Alex Rousskov : > invalid request-line: missing delimiter before "HTTP/1" In order to generate "where" with such detalization (i.e. the specific protocol version or method) we would need to pass skipDelimiter() the parsed

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-23 Thread Alex Rousskov
On 08/23/2016 08:08 AM, Amos Jeffries wrote: > A followup patch can be done to give skipDelimiter a 'const char* which' > parameter that takes a description/name for the delimiter to improve > that debug output. > > so: > skipDelimiter(blah, "method") > > produces: > invalid request-line:

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-23 Thread Alex Rousskov
On 08/23/2016 03:26 AM, Eduard Bagdasaryan wrote: > 2016-08-23 3:08 GMT+03:00 Alex Rousskov: >> I do not understand why you decided to use maxMethodLength in >> parseRequestFirstLine(). AFAICT, parseMethodField() already does >> everything we need: It logs an error message and sets parseStatusCode

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-23 Thread Amos Jeffries
On 23/08/2016 9:26 p.m., Eduard Bagdasaryan wrote: > 2016-08-23 3:08 GMT+03:00 Alex Rousskov: > >> I do not understand why you decided to use maxMethodLength in >> parseRequestFirstLine(). AFAICT, parseMethodField() already does >> everything we need: It logs an error message and sets

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-23 Thread Eduard Bagdasaryan
2016-08-23 3:08 GMT+03:00 Alex Rousskov : > I do not understand why you decided to use maxMethodLength in > parseRequestFirstLine(). AFAICT, parseMethodField() already does > everything we need: It logs an error message and sets parseStatusCode > accordingly.

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-22 Thread Alex Rousskov
On 08/22/2016 04:24 PM, Eduard Bagdasaryan wrote: > -// Limit to 32 characters to prevent overly long sequences of non-HTTP > -// being sucked in before mismatch is detected. 32 is itself annoyingly > -// big but there are methods registered by IANA that reach 17 bytes: > -//

[squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-22 Thread Eduard Bagdasaryan
Hello, This patch makes Squid respond with 414 (URI Too Long) when request target exceeds limits. Before the fix, Squid simply closed client connection after receiving a huge URI (or a huge request-line), violating the RFC 7230 MUST. This happened because a high-level Must(have buffer space)