[squid-users] Re: [PATCH] Revised fix for download corruption\

Not true. half_closed_clients has not been changed on all my servers (and therefore defaults to on).

The corruption is trivially reproducible with the following patch (made my testing much easier). The patch reproduces what occurs when someone sends a TCP reset in the middle of downloading a page. Think about how a page is downloaded with keepalives - one image at a time. The user pushes 'stop' in their browser before finishing the page, and the behavior found in this patch happens.

The below patch, when combined with the following perl script should help anyone out who wants to dig deeper into this - though I'm happy with the bandaid for now.

-Phil

#!/usr/bin/perl

use IO::Socket::INET;
use Net::RawIP;

my $daddr="1.2.3.4";
my $dport=3128;
my $request="GET /schedule.gif HTTP/1.1\nUser-Agent: CORRUPTSQUID\nHost: www.foo.com\nConnection: keep-alive\n\n";

my $sock = new IO::Socket::INET (
        PeerAddr => $daddr,
        PeerPort => $dport,
        Proto => 'tcp',
);

print $sock "$request";
sleep (1);
print $sock "$request";
sleep (1);
shutdown($sock,2);

diff -ru original/src/client_side.c squid-2.5.STABLE1/src/client_side.c
--- original/src/client_side.c Sun Sep 22 21:04:03 2002
+++ squid-2.5.STABLE1/src/client_side.c Mon Nov 4 16:31:35 2002
@@ -2086,6 +2086,7 @@
 static void
 clientWriteComplete(int fd, char *bufnotused, size_t size, int errflag, void *data)
 {
+ fde *F = &fd_table[fd];
     clientHttpRequest *http = data;
     StoreEntry *entry = http->entry;
     int done;
@@ -2140,6 +2141,10 @@
        } else if (http->request->flags.proxy_keepalive) {
            debug(33, 5) ("clientWriteComplete: FD %d Keeping Alive\n", fd);
            clientKeepaliveNextRequest(http);
+ if (F->flags.corrupt) {
+ debug(33, 5) ("clientWriteComplete: FD %d flagging for corruption next time\n", fd);
+ F->flags.secondtime = 1;
+ }
        } else {
            comm_close(fd);
        }
@@ -2797,6 +2802,7 @@
     request_t *request = NULL;
     int size;
     void *p;
+ const char *str;
     method_t method;
     clientHttpRequest *http = NULL;
     clientHttpRequest **H = NULL;
@@ -2954,6 +2960,12 @@
                        http->uri, prefix);
                /* continue anyway? */
            }
+ if ((str = httpHeaderGetStr(&request->header, HDR_USER_AGENT))) {
+ if (strstr(str, "CORRUPTSQUID") != NULL) {
+ debug(33, 5) ("clientReadRequest: FD %d marked for corruption\n", fd);
+ F->flags.corrupt = 1;
+ }
+ }
            request->flags.accelerated = http->flags.accel;
            if (!http->flags.internal) {
                if (internalCheck(strBuf(request->urlpath))) {
diff -ru original/src/comm_select.c squid-2.5.STABLE1/src/comm_select.c
--- original/src/comm_select.c Sat Apr 27 01:48:42 2002
+++ squid-2.5.STABLE1/src/comm_select.c Mon Nov 4 16:30:58 2002
@@ -452,6 +452,14 @@
                        comm_poll_http_incoming();
                }
            }
+
+ /* If we flagged this FD for corruption, close it down if this is second time around */
+ debug(5, 6) ("comm_poll: FD %d flags.corrupt=%d flags.secondtime=%d\n", fd, F->flags.corrupt, F->flags.secondtime);
+ if (F->flags.corrupt && F->flags.secondtime) {
+ debug(5, 6) ("comm_poll: FD %d marked for corruption - closing\n", fd);
+ comm_close(fd);
+ }
+
            if (revents & (POLLWRNORM | POLLOUT | POLLHUP | POLLERR)) {
                debug(5, 5) ("comm_poll: FD %d ready for writing\n", fd);
                if ((hdl = F->write_handler)) {
diff -ru original/src/structs.h squid-2.5.STABLE1/src/structs.h
--- original/src/structs.h Sat Sep 7 16:11:23 2002
+++ squid-2.5.STABLE1/src/structs.h Mon Nov 4 16:30:58 2002
@@ -767,6 +767,8 @@
        unsigned int nodelay:1;
        unsigned int close_on_exec:1;
        unsigned int read_pending:1;
+ unsigned int corrupt:1;
+ unsigned int secondtime:1;
     } flags;
     int bytes_read;
     int bytes_written;

On Tue, Nov 05, 2002 at 12:50:08AM +0100, Henrik Nordstrom wrote:
> Also this only occurs on "half_closed_clients off" and the client
> aborts the request on a cache hit while data is currently beeing
> swapped in or in certain conditions on pipelined requests where the
> next reply is a cache hit.
Received on Mon Nov 04 2002 - 17:38:15 MST