Hi,
Some questions about bug 3113 and squid-3.0.STABLE26
http://bugs.squid-cache.org/show_bug.cgi?id=3113
"Squid can eat far too much memory when uploading files."
a) Does anyone have a backport for this bug to squid-3.0?
The fix is commited on squid-3.1 and squid-3.2 but a patch does not
appear to be avaiable
for squid-3.0.
b) Assuming the answer to question 'a)' is no:
Is anyone able/willing to review the attached patch?
This is a backport (or at least an attmept) to squid-3.0.
The 'patch' is based on:
* http://bugs.squid-cache.org/attachment.cgi?id=2327 - "Possible fix,
fourth iteration"
* http://bazaar.launchpad.net/~squid/squid/3.1/revision/10171 - "Bug
3113: Squid can eat far too much memory when uploading files"
[I obviously tested this and everything appears to be working but a
review would be appreciated]
Best regards,
Bram
diff -Naur squid-3.0.STABLE26.orig/src/cache_cf.cc squid-3.0.STABLE26/src/cache_cf.cc
--- squid-3.0.STABLE26.orig/src/cache_cf.cc 2011-08-27 17:09:25.000000000 +0200
+++ squid-3.0.STABLE26/src/cache_cf.cc 2011-12-21 23:25:56.000000000 +0100
@@ -700,6 +700,14 @@
}
#endif
+
+ // prevent infinite fetch loops in the request parser
+ // due to buffer full but not enough data recived to finish parse
+ if (Config.maxRequestBufferSize <= Config.maxRequestHeaderSize) {
+ fatalf("Client request buffer of %u bytes cannot hold a request with %u bytes of headers." \
+ " Change client_request_buffer_max or request_header_max_size limits.",
+ (uint32_t)Config.maxRequestBufferSize, (uint32_t)Config.maxRequestHeaderSize);
+ }
}
/* Parse a time specification from the config file. Store the
diff -Naur squid-3.0.STABLE26.orig/src/cf.data.pre squid-3.0.STABLE26/src/cf.data.pre
--- squid-3.0.STABLE26.orig/src/cf.data.pre 2011-08-27 17:09:25.000000000 +0200
+++ squid-3.0.STABLE26/src/cf.data.pre 2011-12-21 23:12:37.000000000 +0100
@@ -2831,6 +2831,17 @@
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 -Naur squid-3.0.STABLE26.orig/src/client_side.cc squid-3.0.STABLE26/src/client_side.cc
--- squid-3.0.STABLE26.orig/src/client_side.cc 2011-08-27 17:09:25.000000000 +0200
+++ squid-3.0.STABLE26/src/client_side.cc 2011-12-22 00:00:47.000000000 +0100
@@ -198,12 +198,13 @@
if (reading())
return;
+ if (!maybeMakeSpaceAvailable())
+ return;
+
reading(true);
debugs(33, 4, "clientReadSomeData: FD " << fd << ": reading request...");
- makeSpaceAvailable();
-
/* Make sure we are not still reading from the client side! */
/* XXX this could take a bit of CPU time! aiee! -- adrian */
assert(!comm_has_pending_read(fd));
@@ -2065,13 +2066,22 @@
return result;
}
-void
-ConnStateData::makeSpaceAvailable()
+bool
+ConnStateData::maybeMakeSpaceAvailable()
{
if (getAvailableBufferLength() < 2) {
- in.buf = (char *)memReallocBuf(in.buf, in.allocatedSize * 2, &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
@@ -2362,7 +2372,8 @@
connNoteUseOfBuffer(conn.getRaw(), http->req_sz);
notedUseOfBuffer = true;
- conn->handleRequestBodyData();
+ if (!conn->handleRequestBodyData()) // may comm_close and stop producing
+ goto finish;
if (!request->body_pipe->exhausted())
conn->readSomeData();
@@ -2567,7 +2578,8 @@
if (size > 0) {
kb_incr(&statCounter.client_http.kbytes_in, size);
- conn->handleReadData(buf, size);
+ if (!conn->handleReadData(buf, size))
+ return;
/* The above may close the connection under our feets */
if (!conn->isOpen())
@@ -2626,7 +2638,7 @@
}
// called when new request data has been read from the socket
-void
+bool
ConnStateData::handleReadData(char *buf, size_t size)
{
char *current_buf = in.addressToReadInto();
@@ -2640,12 +2652,13 @@
// if we are reading a body, stuff data into the body pipe
if (bodyPipe != NULL)
- handleRequestBodyData();
+ return handleRequestBodyData();
+ return true;
}
// called when new request body data has been buffered in in.buf
// may close the connection if we were closing and piped everything out
-void
+bool
ConnStateData::handleRequestBodyData()
{
assert(bodyPipe != NULL);
@@ -2730,14 +2743,19 @@
* because mayNeedMoreData is true if request size is not known.
*/
comm_close(fd);
+ return false;
}
}
+ return true;
}
void
ConnStateData::noteMoreBodySpaceAvailable(BodyPipe &)
{
- handleRequestBodyData();
+ if (!handleRequestBodyData())
+ return;
+
+ readSomeData();
}
void
diff -Naur squid-3.0.STABLE26.orig/src/client_side.h squid-3.0.STABLE26/src/client_side.h
--- squid-3.0.STABLE26.orig/src/client_side.h 2011-08-27 17:09:25.000000000 +0200
+++ squid-3.0.STABLE26/src/client_side.h 2011-12-21 23:23:54.000000000 +0100
@@ -143,7 +143,7 @@
bool areAllContextsForThisConnection() const;
void freeAllContexts();
void readNextRequest();
- void makeSpaceAvailable();
+ bool maybeMakeSpaceAvailable();
ClientSocketContext::Pointer getCurrentContext() const;
void addContextToQueue(ClientSocketContext * context);
int getConcurrentRequestCount() const;
@@ -217,8 +217,8 @@
virtual void noteMoreBodySpaceAvailable(BodyPipe &);
virtual void noteBodyConsumerAborted(BodyPipe &);
- void handleReadData(char *buf, size_t size);
- void handleRequestBodyData();
+ bool handleReadData(char *buf, size_t size);
+ bool handleRequestBodyData();
void startDechunkingRequest(HttpParser *hp);
bool parseRequestChunks(HttpParser *hp);
diff -Naur squid-3.0.STABLE26.orig/src/structs.h squid-3.0.STABLE26/src/structs.h
--- squid-3.0.STABLE26.orig/src/structs.h 2011-08-27 17:09:25.000000000 +0200
+++ squid-3.0.STABLE26/src/structs.h 2011-12-21 23:24:49.000000000 +0100
@@ -256,6 +256,7 @@
size_t maxRequestHeaderSize;
int64_t maxRequestBodySize;
int64_t maxChunkedRequestBodySize;
+ size_t maxRequestBufferSize;
size_t maxReplyHeaderSize;
acl_size_t *ReplyBodySize;
