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. _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
