Re: /bzr/squid3/trunk/ r9845: Correct header limit checks. 64KB max is REQUIRED.

On 07/24/2009 02:31 PM, Henrik Nordstrom wrote:
> fre 2009-07-24 klockan 14:24 -0600 skrev Alex Rousskov:
>>> - if (hdr_len > Config.maxReplyHeaderSize || (hdr_len <= 0 && (size_t)buf->contentSize() > Config.maxReplyHeaderSize)) {
>>> + if (hdr_len >= Config.maxReplyHeaderSize || (hdr_len <= 0 && (size_t)buf->contentSize() > Config.maxReplyHeaderSize)) {

>> Does the comparison need to be ">="? On the surface, it seems like this
>> change contradicts expected maxReplyHeaderSize semantics and
>> documentation because the maximum header size should still be allowed.

> It's because we do not want to go above 64KB-1 at this time.

Should not the maximum allowed Config.maxReplyHeaderSize value be
changed to (64KB-1) instead, then?

> Not sure anyone is going to care or even notice that 1 byte difference.

I do care, and I would not be surprised if the check becomes "wrong"
(i.e., correct!) again during some future code polishing.

At the very least there should be a comment there saying that this has
to be that way, but I think enforcing a lower limit on the
Config.maxReplyHeaderSize setting is the right solution.

Thank you,

Alex.
Received on Fri Jul 24 2009 - 20:45:06 MDT