Re: [RFC] HTTP problems caused by broken ICAP servers

On 01/29/2013 03:50 AM, Amos Jeffries wrote:

> The issue outlined very clearly in this squid-users mail:
> http://www.squid-cache.org/mail-archive/squid-users/201301/0358.html
> is that ICAP server was broken and producing a stupid response.

For the record, I do not see anything broken in that ICAP server
behavior, but I am biased since I wrote that server. The ICAP response
in question appears to be valid from the ICAP point of view.

As for the "stupidity" of the encapsulated HTTP response constructed by
the ICAP service adapter, it is difficult for me to asses it since
"stupid" is not an HTTP term. RFC 2616 allows GET requests with bodies.
Needless to say that using such requests is not a good idea in general,
especially when the body is actually empty!

Based on the squid-users email, it sounds like the addition of the empty
body was not intentional in this particular case. The ICAP service
adapter probably added an empty body but should not have.

> Can we add some better protocol idiocy detection on the ICAP responses
> please?

Personally, I am not a big fan of general-purpose proxies policing HTTP
messages. As you probably know, I am more annoyed by thousands of "OMG,
invalid HTTP request character detected! Wear a sweater on Tuesday!"
warnings in cache.log than by thousands of those invalid characters :-).

ICAP is a somewhat special case because ICAP responses are often
generated by software under Squid admin control, but we should tread
carefully because most ICAP responses are just an echo of outside HTTP
messages (where ICAP 204 was not possible) and because some ICAP
services are not under admin controls.

> I know Squid is faithfully replicating what ICAP tol it to produce. But
> we need something to flag when ICAP is telling Squid to produce a risky
> HTTP syntax combination. Or something the Squid admin has configured (or
> defaulted) to not be possible.

I agree that it may be useful to warn or even err when the from-ICAP
HTTP message violates Squid's own message admission policies. Based on
your discussion with Michael, clientIsContentLengthValid() needs to be
polished and moved outside client side code so that ICAP can reuse it.

I suspect there are more similar validity checks that should be
aggregated and placed in src/http/ and used by both Squid sides and
ICAP/eCAP. Can you compile a list of checks in the existing code that
you think should be shared that way?

I cannot promise speedy delivery of these changes, but I can accept your
request to implement those changes as time permits. Is that what you are
asking for?

Thank you,

Alex.
Received on Tue Jan 29 2013 - 16:24:56 MST