Re: [PATCH] Truncate HTTP response bodies to match clen

On 06/27/2009 06:32 AM, Amos Jeffries wrote:
> Alex Rousskov wrote:
>> Truncate too-long HTTP response bodies to match their Content-Length
>> header.
>>
>> Sometimes a broken server sends more than Content-Length bytes in the
>> response. For example, a 302 redirect message with "Content-Length: 0"
>> header may include an HTML body. Squid used to send "everything" it read
>> to the client, even if it read more than the Content-Length bytes. That
>> may have helped in some cases, but I think we should be more
>> conservative when dealing with broken servers to combat message
>> smuggling attacks and other bad side-effects for clients.
>>
>> We now do not forward more than the advertised content length and
>> declare the connection with a broken server non-persistent.
>>
>> Chunked responses (that Squid should not receive and that must not have
>> a Content-Length header) are not truncated because RFC 2616 says we MUST
>> ignore their Content-Length header.
>>
>> TODO: simply truncating read content would not work for pipelined
>> responses. We should preserve extra content for the next transaction on
>> a pconn.
>>
>> ---------------
>>
>> The attached patch is against Squid 3.0 and has been tested in
>> production. More testing is welcomed. I will port to trunk if needed if
>> the change is accepted.
>>
>> Thank you,
>>
>> Alex.
>>
>
> Trouble...
>
> I was of the understanding that Squid already was doing these truncation
> + non-pconn flagging :( I've written off many bugs in the last few
> years to this behavior...
>
>
> Is this some side case you found internal to Squid? or are the current
> Squid-3 actually not protecting people from message-appending?

The problem was triggered at the production site running Squid 3p0-plus
that was accelerating a broken server application. The application was
including bodies in 302 redirects that had Content-Length set to zero.
That, in turn, broke some clients. Neither the application nor the
clients could be fixed, of course.

Besides, it is probably an HTTP violation to forward trailing garbage
anyway, unless we switch to some kind of "tunneling mode" and become a
TCP proxy, but then you get problems with message smuggling and such.

Do you think this change should go in?

Thank you,

Alex.
Received on Sat Jun 27 2009 - 14:39:55 MDT