Re: Squid-2.5 ICAP client

Geetha Manjunath wrote:

> > Hmm.. immediately there is one additional question on clientReadRequest
> > when called from icapReqModReadReply. How does it handle the case where
> > the full request could not be read immediately? To me it looks like the
> > connection gets stuck in such case with clientReadRequest installed as a
> > read handler for the ICAP connection, but nobody taking care of kicking
> > the requests going again..
>
> I am not sure I understand your question fully.. please explain... If
> you mean "continue after being defered", then the fact that
> commSetDefer() is not used on icap_fd before calling clientReadRequest
> from icapReqModReadReply unlike a similar call from httpAccept()
> probably does the trick.

No, I am not talking about deferred reads, I am talking about the
situation where the modified HTTP request returned from the ICAP server
is fragmented into multiple packets and cannot be immediately read in
full by the call from

The way icapReqModReadReply() calls clientReadRequest() seems to assume
clientReadRequest will be able to immediately read and process the
request, but there is no guarantee the full modified request is yet
available.

If the full modified request is not yet available clientReadRequest()
installs itself as a read handler for the filedescriptor (well, it
always does this), and returns without having a request attached to the
connState. icapReqModReadReply() then continues and finds no request,
only destroying the icap state.. (not sure if this also closes the ICAP
filedescriptor).

Lets assume this does not close the ICAP filedescriptor. comm_poll()
then finds that more data is now available and calls clientReadRequest()
to have it read. clientReadRequest() reads the request and attaches it
to the connState, but there is nothing in clientReadRequest() which
initiates continued processing of the request. You only took away the
call to clientAccessCheck(), you did not add another call to notify the
icap code.

The correct design would be icapReqModReadReply reading the ICAP
response headers, then hand off to clientReadRequest for reading the
modified request, and finally have clientReadRequest call back into the
icap code to continue processing of the request instead of calling
clientAccessCheck().

Alternatively you can have icapReqModReadReply verifying that the
modified HTTP request is available in the TCP buffer before calling
clientReadRequest() but such design is not clean and will cause problems
if the modified request is larger than the TCP receive window/buffer.

In detail the case I am worring about is

   icapReqModReadReply() called with ICAP response headers + part of the
modified HTTP request received
   
   icapReqModReadReply reads the ICAP response headers, and makes call
to clientReadRequest for reading the modified HTTP request.

   clientReadRequest finds that the request is partial and does not
process it yet, only reads what is available, and tells comm_poll to
wait for more data to arrive on the filedescriptor and then call
clientReadRequest() again. This is the purpose of the commSetSelect()
call in clientReadRequest().

   control returns to icapReqModReadReply before the full modified
request has been received by clientReadRequest.

Regards
Henrik
Received on Fri Mar 28 2003 - 05:40:01 MST