Hi all, I am sending a small patch for icap client. With this patch squid-icap works for me (using 1-2 clients). I will test it more under load, using different scenarios.
At the following lines I am trying to explain my changes: 1) File HttpReply.cc in function HttpReply::reset pstate need to initialized again to psReadyToParseStartLine 2) Files http.c and ftp.c functions HttpStateData::doneAdapting, HttpStateData::abortAdapting, FtpStateData::doneAdapting and FtpStateData::abortAdapting I comment out cbdataReferenceDone function call. I did not found any cbdataReferenceValid call or something similar so I believe I am correct here. 3) File ICAP/ICAPModXact.cc function ICAPModXact::handleCommWroteHeaders() Sometimes, we have read the headers but not any part of the body yet so in this case we must not call writeMore() function. - writeMore(); + if(virgin->data->body && virgin->data->body->contentSize()) + writeMore(); 4) File ICAP/ICAPModXact.cc function ICAPModXact::moveRequestChunk The preview.wrote(chunkSize, state.doneReceiving) statement is not correct, becouse we can have all the data but we did not send all the data yet. My patch is: + const bool done = state.doneReceiving && claimSize(virginWriteClaim) <= 0; if (state.writing == State::writingPreview) - preview.wrote(chunkSize, state.doneReceiving); // even if wrote nothing + preview.wrote(chunkSize, done); // even if wrote nothing 5) File ICAP/ICAPModXact.cc function ICAPModXact::addLastRequestChunk The last chunk was not correct ("0\r\n; ieof\r\n" instead of "0; ieof\r\n\r\n" ). 6) File ICAP/ICAPModXact.cc function ICAPModXact::handle100Continue This function called when we read "100 Continue" responce from icap server. After that we expect to read the real ICAP responce (ICAP headers followed by http headers and http body). In current implementation squid ICAP client expects to read the http headers. The solution is: - state.parsing = State::psHttpHeader; // eventually + state.parsing = State::psIcapHeader; + icapReply->reset(); 7) File ICAP/ICAPModXact.cc function ICAPModXact::estimateVirginBody if function header->expectingBody(method, size) returns size==0 then we do not expect a body. 8) File ICAP/ICAPModXact.cc function ICAPPreview::enable The theAd is the expected preview size so if theAd==0 do not enable preview mode. - theState = stWriting; + if(theAd >0) + theState = stWriting; + else + theState = stDisabled;
Index: HttpReply.cc =================================================================== RCS file: /squid/squid3/src/HttpReply.cc,v retrieving revision 1.89 diff -u -r1.89 HttpReply.cc --- HttpReply.cc 7 Jun 2006 22:39:33 -0000 1.89 +++ HttpReply.cc 27 Sep 2006 12:25:29 -0000 @@ -105,6 +105,7 @@ // are allowed with MEMPROXY_CLASS() and whether some cbdata void* // conversions are not going to kill virtual tables const String pfx = protoPrefix; + pstate=psReadyToParseStartLine; clean(); init(); protoPrefix = pfx; Index: ftp.cc =================================================================== RCS file: /squid/squid3/src/ftp.cc,v retrieving revision 1.407 diff -u -r1.407 ftp.cc --- ftp.cc 19 Sep 2006 07:56:57 -0000 1.407 +++ ftp.cc 27 Sep 2006 12:25:43 -0000 @@ -3439,7 +3439,7 @@ */ delete icap; - cbdataReferenceDone(icap); +// cbdataReferenceDone(icap); if (ctrl.fd >= 0) comm_close(ctrl.fd); @@ -3456,7 +3456,7 @@ * ICAP has given up, we're done with it too */ delete icap; - cbdataReferenceDone(icap); +// cbdataReferenceDone(icap); if (entry->isEmpty()) { ErrorState *err; Index: http.cc =================================================================== RCS file: /squid/squid3/src/http.cc,v retrieving revision 1.507 diff -u -r1.507 http.cc --- http.cc 20 Sep 2006 06:29:10 -0000 1.507 +++ http.cc 27 Sep 2006 12:25:47 -0000 @@ -2077,8 +2077,8 @@ * ICAP is done, so we don't need this any more. */ delete icap; - - cbdataReferenceDone(icap); + icap = NULL; +// cbdataReferenceDone(icap); if (fd >= 0) comm_close(fd); @@ -2095,7 +2095,8 @@ * ICAP has given up, we're done with it too */ delete icap; - cbdataReferenceDone(icap); + icap = NULL; +// cbdataReferenceDone(icap); if (entry->isEmpty()) { ErrorState *err; Index: ICAP/ICAPModXact.cc =================================================================== RCS file: /squid/squid3/src/ICAP/ICAPModXact.cc,v retrieving revision 1.24 diff -u -r1.24 ICAPModXact.cc --- ICAP/ICAPModXact.cc 8 May 2006 23:38:34 -0000 1.24 +++ ICAP/ICAPModXact.cc 27 Sep 2006 12:25:48 -0000 @@ -177,7 +177,8 @@ state.writing = preview.enabled() ? State::writingPreview : State::writingPrime; virginWriteClaim.protectAll(); - writeMore(); + if(virgin->data->body && virgin->data->body->contentSize()) + writeMore(); } else { stopWriting(); } @@ -303,14 +304,17 @@ virginConsume(); } + const bool done = state.doneReceiving && claimSize(virginWriteClaim) <= 0; if (state.writing == State::writingPreview) - preview.wrote(chunkSize, state.doneReceiving); // even if wrote nothing + preview.wrote(chunkSize, done); // even if wrote nothing } void ICAPModXact::addLastRequestChunk(MemBuf &buf) { - openChunk(buf, 0); - closeChunk(buf, state.writing == State::writingPreview && preview.ieof()); + if(state.writing == State::writingPreview && preview.ieof()) + buf.append("0; ieof\r\n\r\n", 11); + else + buf.append("0\r\n\r\n", 5); } void ICAPModXact::openChunk(MemBuf &buf, size_t chunkSize) @@ -653,8 +657,10 @@ if (virginSendClaim.limited()) // preview only stopBackup(); - state.parsing = State::psHttpHeader; // eventually - +// state.parsing = State::psHttpHeader; // eventually + state.parsing = State::psIcapHeader; + icapReply->reset(); + state.writing = State::writingPrime; writeMore(); @@ -1143,8 +1149,8 @@ else return; - ssize_t size; - if (virgin->data->header->expectingBody(method, size)) { + ssize_t size=0; + if (virgin->data->header->expectingBody(method, size) && size ) { virginBody.expect(size) ; debugs(93, 6, "ICAPModXact expects virgin body; size: " << size); @@ -1243,7 +1249,10 @@ Must(anAd >= 0); Must(!enabled()); theAd = anAd; - theState = stWriting; + if(theAd >0) + theState = stWriting; + else + theState = stDisabled; } bool ICAPPreview::enabled() const @@ -1282,7 +1291,7 @@ if (theWritten >= theAd) theState = stDone; // sawEof is irrelevant - else +// else if (sawEof) theState = stIeof; } Index: ICAP/ICAPXaction.cc =================================================================== RCS file: /squid/squid3/src/ICAP/ICAPXaction.cc,v retrieving revision 1.8 diff -u -r1.8 ICAPXaction.cc --- ICAP/ICAPXaction.cc 19 Sep 2006 17:17:52 -0000 1.8 +++ ICAP/ICAPXaction.cc 27 Sep 2006 12:25:48 -0000 @@ -46,7 +46,7 @@ } static -void ICAPXaction_noteCommWrote(int, char *, size_t size, comm_err_t status, void *data) +void ICAPXaction_noteCommWrote(int, char *, size_t size, comm_err_t status, int xerrno, void *data) { ICAPXaction_fromData(data).noteCommWrote(status, size); }