Re: [squid-dev] [PATCH] VIA creation code duplication

2017-03-22 Thread Alex Rousskov
On 03/22/2017 07:20 AM, Amos Jeffries wrote: > On 17/03/2017 6:17 a.m., Alex Rousskov wrote: >> On 03/16/2017 05:15 AM, Amos Jeffries wrote: >> >> >>> Any objections to applying this with this added: >>> >>> // XXX: putStr() still has String 64KB limits >>> Must(strVia.length() < 64*1024); >>

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-03-16 Thread Alex Rousskov
On 03/16/2017 05:15 AM, Amos Jeffries wrote: > Any objections to applying this with this added: > > // XXX: putStr() still has String 64KB limits > Must(strVia.length() < 64*1024); No objections from me if you replace the magic constant with a new inlined String::MaxSizeXXX() method. The

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-03-16 Thread Eduard Bagdasaryan
I would avoid using magic numbers like 64*1024. If you are sure that this check is much better/informative, consider making String::SizeMax_ public and use it instead. Eduard. On 16.03.2017 14:15, Amos Jeffries wrote: On 16/03/2017 11:03 p.m., Eduard Bagdasaryan wrote: If throwing when

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-03-16 Thread Amos Jeffries
On 16/03/2017 11:03 p.m., Eduard Bagdasaryan wrote: > If throwing when String exceeds its 64K limit is > acceptable then why not just use my previous t2 patch? > HttpHeader::addVia() calls strListAdd() similarly as the old > did: it throws if str->canGrowBy(itemSize) returns false. > I was

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-03-16 Thread Eduard Bagdasaryan
If throwing when String exceeds its 64K limit is acceptable then why not just use my previous t2 patch? HttpHeader::addVia() calls strListAdd() similarly as the old did: it throws if str->canGrowBy(itemSize) returns false. Eduard. On 16.03.2017 10:38, Amos Jeffries wrote: On 14/03/2017 5:24

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-03-16 Thread Amos Jeffries
On 14/03/2017 5:24 a.m., Alex Rousskov wrote: > On 03/13/2017 08:25 AM, Eduard Bagdasaryan wrote: >> On 14.02.2017 04:22, Amos Jeffries wrote: >>> The problem is with proxy where the admin has configured large headers >>> to be allowed, and receives a Via just under the 6KB liit. Our append >>>

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-03-13 Thread Alex Rousskov
On 03/13/2017 08:25 AM, Eduard Bagdasaryan wrote: > On 14.02.2017 04:22, Amos Jeffries wrote: >> The problem is with proxy where the admin has configured large headers >> to be allowed, and receives a Via just under the 6KB liit. Our append >> pushing it over by even one byte would assert. Yes,

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-03-13 Thread Eduard Bagdasaryan
On 14.02.2017 04:22, Amos Jeffries wrote: The problem is with proxy where the admin has configured large headers to be allowed, and receives a Via just under the 6KB liit. Our append pushing it over by even one byte would assert. I am attaching a patch using SBuf instead of String for Via

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-02-13 Thread Amos Jeffries
On 14/02/2017 2:06 a.m., Eduard Bagdasaryan wrote: > I see that String::append asserts when String is unable to "grow": > String has hardcoded ~64Kb limit for that. It is hardly possible since > most of web servers have header length limit less than this value. > Theoretically a buggy upstream

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-02-13 Thread Eduard Bagdasaryan
I see that String::append asserts when String is unable to "grow": String has hardcoded ~64Kb limit for that. It is hardly possible since most of web servers have header length limit less than this value. Theoretically a buggy upstream server could generate such huge Via. However any other header

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-02-12 Thread Amos Jeffries
Thank you. But if we are going to keep the copy+append behaviour we have to consider the assertion in String::append which goes off if the client has supplied a large header already. We need to either assemble in the SBuf, or to check the strVia length before appending the new parts. Amos

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-02-11 Thread Eduard Bagdasaryan
On 09.02.2017 20:19, Amos Jeffries wrote: > Since Via is a list header we should be able to just append a new Via > header to the header list with putStr. No need to use getList, String, > delById to inject on the end of existing Via string. Doing so will change the Via generation way currently

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-02-10 Thread Alex Rousskov
On 02/10/2017 04:17 PM, Amos Jeffries wrote: > This patch is "polishing a turd" as the saying goes and a bit premature. > There are some obvious and easy to fix bugs that should be attended > first, then polishing afterwards. I agree that there are many problems related to Via adding code and

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-02-10 Thread Amos Jeffries
On 11/02/2017 5:04 a.m., Alex Rousskov wrote: > On 02/09/2017 10:19 AM, Amos Jeffries wrote: >> On 3/02/2017 4:02 a.m., Eduard Bagdasaryan wrote: >>> This patch fixes VIA appending code duplication, moving common >>> code into a separate method. > >> Since Via is a list header we should be able

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-02-10 Thread Alex Rousskov
On 02/09/2017 10:19 AM, Amos Jeffries wrote: > On 3/02/2017 4:02 a.m., Eduard Bagdasaryan wrote: >> This patch fixes VIA appending code duplication, moving common >> code into a separate method. > Since Via is a list header we should be able to just append a new Via > header to the header list

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-02-09 Thread Amos Jeffries
On 3/02/2017 4:02 a.m., Eduard Bagdasaryan wrote: > Hello, > > This patch fixes VIA appending code duplication, moving common > code into a separate method. > Since Via is a list header we should be able to just append a new Via header to the header list with putStr. No need to use getList,

[squid-dev] [PATCH] VIA creation code duplication

2017-02-02 Thread Eduard Bagdasaryan
Hello, This patch fixes VIA appending code duplication, moving common code into a separate method. Regards, Eduard. Fixed appending Http::HdrType::VIA code duplication. === modified file 'src/HttpHeader.cc' --- src/HttpHeader.cc 2017-01-25 22:29:03 + +++ src/HttpHeader.cc 2017-02-02