--------------------- PatchSet 10410 Date: 2008/01/15 13:04:32 Author: adri Branch: s27_adri Tag: (none) Log: Prepare clientTryParseRequest() to not touch the conn->in buffer directly. * Pass in a buf/len instead of just the conn being touched; * move the whitespace skipping back into clientTryParseRequest() and don't "trim" the buffer. This change does modify the code semantics a bit. Now the buffer can fill with whitespace/newlines and thus a broken UA could hit the request buffer max limit by simply dumping lots of whitespace. The aim here however is to minimise as much direct buffer modification as possible so eventually we can use read only buffer references everywhere except the "fill" operator. There's now only three places where the buffer is being consumed (two in client_side.c and one in the SSL code.) Members: src/client_side.c:1.202.2.9.4.37->1.202.2.9.4.38 Index: squid/src/client_side.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/client_side.c,v retrieving revision 1.202.2.9.4.37 retrieving revision 1.202.2.9.4.38 diff -u -r1.202.2.9.4.37 -r1.202.2.9.4.38 --- squid/src/client_side.c 14 Jan 2008 08:38:09 -0000 1.202.2.9.4.37 +++ squid/src/client_side.c 15 Jan 2008 13:04:32 -0000 1.202.2.9.4.38 @@ -1,6 +1,6 @@ /* - * $Id: client_side.c,v 1.202.2.9.4.37 2008/01/14 08:38:09 adri Exp $ + * $Id: client_side.c,v 1.202.2.9.4.38 2008/01/15 13:04:32 adri Exp $ * * DEBUG: section 33 Client-side Routines * AUTHOR: Duane Wessels @@ -3996,7 +3996,7 @@ * <0 : error; stop parsing */ static int -clientTryParseRequest(ConnStateData * conn, int *cbytes, clientHttpRequest **chttp) +clientTryParseRequest(ConnStateData * conn, int *cbytes, clientHttpRequest **chttp, const char *sbuf, int slen) { int fd = conn->fd; int nrequests; @@ -4008,10 +4008,19 @@ request_t *request = NULL; HttpMsgBuf msg; + const char *buf = sbuf; + int len = slen; + *chttp = NULL; *cbytes = 0; - HttpMsgBufInit(&msg, clientConnBuf(&conn->in), clientConnBufLen(&conn->in)); /* XXX for now there's no deallocation function needed but this may change */ + /* Skip whitespace */ + for (; len > 0 && xisspace(*buf); buf++, len--) + ; + if (len <= 0) + return 0; + + HttpMsgBufInit(&msg, buf, len); /* XXX for now there's no deallocation function needed but this may change */ /* Limit the number of concurrent requests to 2 */ for (n = conn->reqs.head, nrequests = 0; n; n = n->next, nrequests++); if (nrequests >= (Config.onoff.pipeline_prefetch ? 2 : 1)) { @@ -4032,10 +4041,10 @@ * Partial request received; reschedule until parseHttpRequest() * is happy with the input */ - if (clientConnBufLen(&conn->in) >= Config.maxRequestHeaderSize) { + if (len >= Config.maxRequestHeaderSize) { /* The request is too large to handle */ debug(33, 1) ("Request header is too large (%d bytes)\n", - (int) clientConnBufLen(&conn->in)); + (int) len); debug(33, 1) ("Config 'request_header_max_size'= %ld bytes.\n", (long int) Config.maxRequestHeaderSize); err = errorCon(ERR_TOO_BIG, HTTP_REQUEST_URI_TOO_LONG, NULL); @@ -4061,7 +4070,7 @@ debug(33, 1) ("clientReadRequest: FD %d (%s:%d) Invalid Request\n", fd, fd_table[fd].ipaddr, fd_table[fd].remote_port); err = errorCon(ERR_INVALID_REQ, HTTP_BAD_REQUEST, NULL); err->src_addr = conn->peer.sin_addr; - err->request_hdrs = xstrndup(clientConnBuf(&conn->in), clientConnBufLen(&conn->in)); + err->request_hdrs = xstrndup(buf, len); http->log_type = LOG_TCP_DENIED; http->entry = clientCreateStoreEntry(http, method, null_request_flags); errorAppendEntry(http->entry, err); @@ -4268,16 +4277,9 @@ while (cbdataValid(conn) && clientConnBufLen(&conn->in) > 0 && conn->body.size_left == 0) { /* Ret tells us how many bytes was consumed - 0 == didn't consume request, > 0 == consumed, -1 == error, -2 == CONNECT request stole the connection */ - /* Skip leading (and trailing) whitespace */ - clientConnBufTrimWhitespace(&conn->in); - if (clientConnBufLen(&conn->in) == 0) { - ret = 0; - break; - } - chttp = NULL; cbytes = 0; - ret = clientTryParseRequest(conn, &cbytes, &chttp); + ret = clientTryParseRequest(conn, &cbytes, &chttp, clientConnBuf(&conn->in), clientConnBufLen(&conn->in)); if (cbytes > 0) { /* If we read past the end of this request, move the remaining data to the beginning */