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, except s/6/64/ and no special configuration is probably needed: reply_header_max_size is 64KB by default (and so is String::SizeMax_). > I am attaching a patch using SBuf instead of String for Via > accumulating, as you previously suggested. I am not > sure this is a much better solution since we have to do an > extra copy into SBuf. Will this new patch assert in String::setBuffer(?) when the final Via value exceeds String::SizeMax_ and putStr() is called with a 64KB+1 or longer value string? >> The older bbuf code >> cropping at 1KB was nasty but would not crash Squid. 1KB cropping of the additional Via component before strListAdd() is pretty much irrelevant to String overflows discussed here -- the size of the added value is usually not the problem (the resulting total size is). The reason strListAdd() is better than String::append() when assembling the Via header is because the former throws and the latter asserts. > I don't see the code preventing Squid from String overflow > there. The bbuf you are talking about was appended to via string > by strListAdd() without any checks... strListAdd() has Must(str->canGrowBy(itemSize)). This protection is far from perfect or even good, but better than an assert. See trunk r14552 for details. Wondering how much more time will be wasted on what used to be a simple and useful code de-duplication patch, Alex. _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
