[prev in list] [next in list] [prev in thread] [next in thread] 

List:       squid-dev
Subject:    squid-3 icap related patch
From:       "Tsantilas Christos" <chtsanti () users ! sourceforge ! net>
Date:       2006-09-28 9:18:11
Message-ID: 53270.127.0.0.1.1159435091.squirrel () www ! chtsanti ! net
[Download RAW message or body]

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;



["icap-patch.txt" (text/plain)]

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);
 }

[prev in list] [next in list] [prev in thread] [next in thread]