[squid-dev] Terminating ICAP requests for aborted HTTP requests

[Alex Rousskov]

On 07/11/2018 07:54 AM, Steve Hill wrote:

> the HTTP client had made a request which has been forwarded onto
> the web server, the web server has started responding, Squid is sending
> the response body to the RESPMOD ICAP service and is forwarding the
> modified body to the client.  Part way through the download, the client
> (or maybe even the server?) drops the TCP connection.  What should Squid
> do?

As far as ICAP is concerned, Squid should give the service everything
Squid has. Whether Squid continues to download the object is subject to
various factors such as object cachability and quick_abort_pct settings.
After giving everything, Squid should indicate a premature message
termination (as discussed below).


> My testing shows that the response body that is being sent to the ICAP
> server is cut short by Squid, by terminating it with a zero-length
> chunk.

Using last-chunk for aborted body termination is a bug. If Squid
terminates the body, it should not use the standard last-chunk. Like you
suggested below, Squid should either (half-)close the connection or
negotiate the use of a chunk extension, both indicating a premature body
termination.


> This is also how fully successful ICAP requests are terminated
> and as the request has been terminated "normally", the ICAP connection
> can then be used for a new, unrelated, ICAP request.

Yes, that is the advantage of sending a standard last-chunk, but that
performance advantage is not an excuse for lying to the ICAP service.


> The solutions that spring to my mind are:
> 
> 1. Squid could just terminate the ICAP connection if the HTTP request is
> aborted.  The ICAP server would then be able to see that the connection
> was chopped before it received the terminating zero length chunk.  There
> may be a performance impact since Squid would need to establish a new
> connection for the next request.

Agreed. Half-closing the connection would allow Squid to receive the
ICAP response which may be useful for logging/annotations, but I am not
sure many ICAP services and Squid itself support that without changes.


> 2. Squid could add an "aborted" extension to the terminating chunk
> header.  i.e. in the same way that "0; ieof" is allowed, "0; aborted"
> could be used.  However, this is not part of the ICAP RFC and would
> therefore be non-standard behaviour that could break clients that have
> less robust parsers.

Using non-negotiated chunk extensions will break many ICAP servers. The
new feature should be negotiated using the ICAP Allow mechanism. We have
used that mechanism for negotiating ICAP trailer support, for example:
https://tools.ietf.org/html/draft-rousskov-icap-trailers-01#section-5

Are you in a position to modify your ICAP service to support a new chunk
extension? If yes, I would recommend teaching Squid to use that
extension (when supported by the service) or terminate the ICAP
connection (otherwise).

It would be important to document the exact effects of that new
extension. For example, should the ICAP client expect an ICAP response?
I suspect the answer is yes because, in part, it would be difficult to
prevent an early response that may be already in-flight. Can the
extension be sent by ICAP servers? I suspect the answer is yes because
ICAP servers may face transaction termination conditions as well.

FWIW, I would probably reuse "ieof" instead of "abort". Its meaning
matches the intent and its reuse would allow implementations to reuse
the existing parsing code.


Cheers,

Alex.