Re: squid-2.5 / ICAP patch

This patch do not work for me.

I tested for an icap 204 responce with preview enable. Maybe works for
an "204 No Content" responce outside preview. This patch when
reads a 204 responce from the icap server try to read more data from the
server witch is illegal and connection blocks here.(file icap_respmod.c,
line 419).

Moreover when icap_server responds with "100 Continue" the squid
frequently dies with message "icapSendRespMod: oops, write_pending=1".
This message exist at file icap_respmod.c line 202. In this block the
variables theEnd is 0 and len is non zero (look assert here).

The following patch for the icap_respmod.c file operates well for me. I
test it only for "204 No Content" and "100 Continue" responces inside
preview. Please test it before apply it.

Regards,
         Christos

> I've finally committed your patch to the sourceforge CVS. They
> look good to me, but I have not tested them.
>
>
> Thanks a lot!
>
>
> Duane W.
>

--- /root/icap_respmod.c 2004-08-18 10:50:33.000000000 -0700
+++ icap_respmod.c 2004-08-19 17:24:03.000000000 -0700
@@ -188,7 +188,7 @@
     if (theEnd) {
         if (icap->respmod.res_body_sz)
             icap->flags.send_zero_chunk = 1;
- else
+ /*else*/ /*Why this "else" here? theEnd means http_server_eof=1!!! (Tsantilas) .... */
             icap->flags.http_server_eof = 1;
     }
     /*
@@ -289,11 +289,13 @@
             memBufAppend(&icap->respmod.buffer, buf, len);
         /* do not send any data now while waiting for preview response */
         /* but prepare for read more data on the HTTP connection */
- if (!icap->flags.http_server_eof) {
- debug(81, 3) ("icapSendRespMod: FD %d: commSetSelect on read icapRespModReadReply waiting for preview response.\n", icap->icap_fd);
- commSetSelect(icap->icap_fd, COMM_SELECT_READ, icapRespModReadReply,
- icap, 0);
- }
+
+ /*Here is not the right position for the commSetSelect. Better called in icapRespModReadReply. (Tsantilas) */
+ /* if (!icap->flags.http_server_eof) {*/
+ /* debug(81, 3) ("icapSendRespMod: FD %d: commSetSelect on read icapRespModReadReply waiting for preview response.\n", icap->icap_fd); */
+ /* commSetSelect(icap->icap_fd, COMM_SELECT_READ, icapRespModReadReply,
+ icap, 0); */
+ /* } */
         return;
     } else
 #endif
@@ -394,11 +396,16 @@
         if (status == 100) {
             debug(81, 5) ("icapRespModReadReply: 100 Continue received\n");
             icap->flags.wait_for_preview_reply = 0;
- /*
+ /* if http_server_eof
              * call again icapSendRespMod to handle data that
- * was received while waiting fot this ICAP response
+ * was received while waiting for this ICAP response
+ * else let http to call icapSendRespMod when new data arrived
              */
+ if(icap->flags.http_server_eof)
             icapSendRespMod(icap, NULL, 0, 0);
+ /* The following maybe needed ....(Tsantilas)*/
+ /*commSetSelect(fd, COMM_SELECT_READ, icapRespModReadReply, icap, 0);*/
+
             /*
              * reset the header to send the rest of the preview
              */
@@ -409,18 +416,21 @@
         } else if (status == 204) {
             debug(81, 5) ("icapRespModReadReply: 204 No modification received\n");
             icap->flags.wait_for_preview_reply = 0;
- if (icap->flags.http_server_eof) {
+
+ /* if (icap->flags.http_server_eof) {*/ /*Comment out by Tsantilas*/
                 /* Reset is required to avoid duplicate stmemFreeDataUpto ,
                  * but will I loose all info now ? */
                 /* storeEntryReset(icap->respmod.entry); */
                 /* stmemFreeDataUpto(&(entry->mem_obj->data_hdr), -icap->sc); */
- fwdComplete(icap->httpState->fwd);
- } else {
- commSetSelect(fd, COMM_SELECT_READ, icapRespModReadReply,
- icap, 0);
- }
- comm_close(fd);
- return;
+ /* fwdComplete(icap->httpState->fwd);*/ /*Comment out by Tsantilas*/
+ /* } else {*/ /*Comment out by Tsantilas*/
+ /* commSetSelect(fd, COMM_SELECT_READ, icapRespModReadReply,*/ /*Comment out by Tsantilas*/
+ /* icap, 0); */ /*Comment out by Tsantilas*/
+ /* }*/ /*Comment out by Tsantilas*/
+ /* comm_close(fd);*/ /*Comment out by Tsantilas*/
+ /* return; */ /*Comment out by Tsantilas*/
+ /*Previous region comment out because not needed!!!
+ The following code handles better returned 204 status. (Tsantilas) */
 #endif
         }
     }
Received on Thu Aug 19 2004 - 16:00:41 MDT