Author: Graham Keeling <graham@equiinet.com>

Bug 3113: Squid can eat far too much memory when uploading files

Problem description:
  Uploading a large file to a web site on the internet, squid's client
input buffer will increase far faster than it can be emptied to
the target website, and the machine will swiftly run out of memory.

This patch adds the client_request_buffer_max_size configuration
parameter  which specifies the maximum buffer size of a client request.

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 8f5e1f383f0f74566f9212a0fb5eb8139518f885..8bc463be23092e1af7a18dfb33e4df9db24437dd 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -4033,6 +4033,17 @@ DOC_START
        be no limit imposed.
 DOC_END
 
+NAME: client_request_buffer_max_size
+COMMENT: (bytes)
+TYPE: b_size_t
+DEFAULT: 512 KB
+LOC: Config.maxRequestBufferSize
+DOC_START
+       This specifies the maximum buffer size of a client request.
+       It prevents squid eating too much memory when somebody uploads
+       a large file.
+DOC_END
+
 NAME: chunked_request_body_max_size
 COMMENT: (bytes)
 TYPE: b_int64_t

diff --git a/src/client_side.cc b/src/client_side.cc
index 4d0d54530e51eb6343fea13771c1fab917b0e7f9..01efa3de13b53e7f0b7e9f9692d64f1c0e98e11c 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -256,7 +256,8 @@ ConnStateData::readSomeData()
 
     debugs(33, 4, "clientReadSomeData: FD " << fd << ": reading request...");
 
-    makeSpaceAvailable();
+    if(!maybeMakeSpaceAvailable())
+        return;
 
     typedef CommCbMemFunT<ConnStateData, CommIoCbParams> Dialer;
     reader = JobCallback(33, 5,
@@ -2249,13 +2250,22 @@ ConnStateData::getAvailableBufferLength() const
     return result;
 }
 
-void
-ConnStateData::makeSpaceAvailable()
+bool
+ConnStateData::maybeMakeSpaceAvailable()
 {
     if (getAvailableBufferLength() < 2) {
-        in.buf = (char *)memReallocBuf(in.buf, in.allocatedSize * 2, &in.allocatedSize);
-        debugs(33, 2, "growing request buffer: notYetUsed=" << in.notYetUsed << " size=" << in.allocatedSize);
+       size_t newSize;
+       if (in.allocatedSize >= Config.maxRequestBufferSize) {
+           debugs(33, 4, "request buffer full: client_request_buffer_max_size=" << Config.maxRequestBufferSize);
+           return false;
+       }
+       if ((newSize=in.allocatedSize * 2) > Config.maxRequestBufferSize) {
+           newSize=Config.maxRequestBufferSize;
+       }
+       in.buf = (char *)memReallocBuf(in.buf, newSize, &in.allocatedSize);
+       debugs(33, 2, "growing request buffer: notYetUsed=" << in.notYetUsed << " size=" << in.allocatedSize);
     }
+    return true;
 }
 
 void
@@ -2968,7 +2978,10 @@ ConnStateData::abortChunkedRequestBody(const err_type error)
 void
 ConnStateData::noteMoreBodySpaceAvailable(BodyPipe::Pointer )
 {
-    handleRequestBodyData();
+    if (!handleRequestBodyData()) 
+        return;
+
+    readSomeData();
 }
 
 void

diff --git a/src/client_side.h b/src/client_side.h
index 707b748eac0ecdafe5a0c0f9a953c62a338087ba..cbacbe26f7f470c229db79eb082a4c3968d00487 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -153,7 +153,7 @@ public:
     void freeAllContexts();
     void notifyAllContexts(const int xerrno); ///< tell everybody about the err
     void readNextRequest();
-    void makeSpaceAvailable();
+    bool maybeMakeSpaceAvailable();
     ClientSocketContext::Pointer getCurrentContext() const;
     void addContextToQueue(ClientSocketContext * context);
     int getConcurrentRequestCount() const;

diff --git a/src/structs.h b/src/structs.h
index 43490ed6d65f5b717dca2f3386f13278fab6acce..e898fe5d814e5ff223c8b9ddf3f442b26f509c33 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -199,6 +199,7 @@ struct SquidConfig {
     size_t maxRequestHeaderSize;
     int64_t maxRequestBodySize;
     int64_t maxChunkedRequestBodySize;
+    size_t maxRequestBufferSize;
     size_t maxReplyHeaderSize;
     acl_size_t *ReplyBodySize;