Re: Squid-2.5 ICAP client

Hello Henrik,

You are right! Actually you have hit on a bug here... I will fix it..
Thanks!

It so happens that a modified request is usually small and was written
out by the icap server in one shot in all our tests ... Hence this bug
has not shown up so far.

Yes ofcourse! how can I surpass the "continuation" mechanism of squid??
I should have put a callback on clientReadRequest... We have done that
for RESPMOD though.

Thanks again for pointing this out!
 
PS: Sorry for my delayed answer - I was out of office...

thanks and regards
Geetha

Henrik Nordstrom wrote:
>
> 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 Tue Apr 01 2003 - 06:23:28 MST