From: adrian <> Date: Tue, 26 Sep 2006 21:19:22 +0000 (+0000) Subject: Various changes to break out the request parsing state and remove extra malloc/copies X-Git-Tag: SQUID_3_0_PRE5~48 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a5baffbadcda654e9cef5781aab97b829d3dcab6;p=thirdparty%2Fsquid.git Various changes to break out the request parsing state and remove extra malloc/copies * Create a HttpParser structure to encapsulate the current parsing state * Add some functions to return the current request/header length with some checks (which are expensive and should be inlined later once the code is proven to work.) * Use the connection buffer rather than a temporary buffer to parse the request headers, saving a malloc and copy (the 'prefix' buffer) --- diff --git a/src/HttpMsg.cc b/src/HttpMsg.cc index 20e720ab32..68fc1a7910 100644 --- a/src/HttpMsg.cc +++ b/src/HttpMsg.cc @@ -1,6 +1,6 @@ /* - * $Id: HttpMsg.cc,v 1.32 2006/09/26 13:30:09 adrian Exp $ + * $Id: HttpMsg.cc,v 1.33 2006/09/26 15:19:22 adrian Exp $ * * DEBUG: section 74 HTTP Message * AUTHOR: Alex Rousskov @@ -389,3 +389,55 @@ HttpMsg::_unlock() if (0 == lock_count) delete this; } + + +void +HttpParserInit(HttpParser *hdr, const char *buf, int bufsiz) +{ + hdr->state = 1; + hdr->buf = buf; + hdr->bufsiz = bufsiz; + hdr->req_start = hdr->req_end = -1; + hdr->hdr_start = hdr->hdr_end = -1; +} + +/* XXX This should eventually turn into something inlined or #define'd */ +int +HttpParserReqSz(HttpParser *hp) +{ + assert(hp->state == 1); + assert(hp->req_start != -1); + assert(hp->req_end != -1); + return hp->req_end - hp->req_start + 1; +} + + +/* + * This +1 makes it 'right' but won't make any sense if + * there's a 0 byte header? This won't happen normally - a valid header + * is at -least- a blank line (\n, or \r\n.) + */ +int +HttpParserHdrSz(HttpParser *hp) +{ + assert(hp->state == 1); + assert(hp->hdr_start != -1); + assert(hp->hdr_end != -1); + return hp->hdr_end - hp->hdr_start + 1; +} + +const char * +HttpParserHdrBuf(HttpParser *hp) +{ + assert(hp->state == 1); + assert(hp->hdr_start != -1); + assert(hp->hdr_end != -1); + return hp->buf + hp->hdr_start; +} + +int +HttpParserRequestLen(HttpParser *hp) +{ + return hp->hdr_end - hp->req_start + 1; +} + diff --git a/src/HttpMsg.h b/src/HttpMsg.h index 1ddf444d1b..2b81133102 100644 --- a/src/HttpMsg.h +++ b/src/HttpMsg.h @@ -1,6 +1,6 @@ /* - * $Id: HttpMsg.h,v 1.10 2006/09/26 13:30:09 adrian Exp $ + * $Id: HttpMsg.h,v 1.11 2006/09/26 15:19:22 adrian Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -100,6 +100,22 @@ protected: }; +/* Temporary parsing state; might turn into the replacement parser later on */ +struct _HttpParser { + char state; + const char *buf; + int bufsiz; + + int req_start, req_end; + int hdr_start, hdr_end; +}; +typedef struct _HttpParser HttpParser; + +extern void HttpParserInit(HttpParser *, const char *buf, int len); +extern int HttpParserReqSz(HttpParser *); +extern int HttpParserHdrSz(HttpParser *); +extern const char * HttpParserHdrBuf(HttpParser *); +extern int HttpParserRequestLen(HttpParser *hp); SQUIDCEXTERN int httpMsgIsolateHeaders(const char **parse_start, int len, const char **blk_start, const char **blk_end); diff --git a/src/client_side.cc b/src/client_side.cc index 033ca00705..870ba5ffc3 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -1,6 +1,6 @@ /* - * $Id: client_side.cc,v 1.737 2006/09/26 13:30:09 adrian Exp $ + * $Id: client_side.cc,v 1.738 2006/09/26 15:19:22 adrian Exp $ * * DEBUG: section 33 Client-side Routines * AUTHOR: Duane Wessels @@ -130,8 +130,7 @@ static PF requestTimeout; static PF clientLifetimeTimeout; static ClientSocketContext *parseHttpRequestAbort(ConnStateData::Pointer & conn, const char *uri); -static ClientSocketContext *parseHttpRequest(ConnStateData::Pointer &, method_t *, - char **, size_t *, HttpVersion *); +static ClientSocketContext *parseHttpRequest(ConnStateData::Pointer &, HttpParser *, method_t *, HttpVersion *); #if USE_IDENT static IDCB clientIdentDone; #endif @@ -1926,16 +1925,13 @@ prepareTransparentURL(ConnStateData::Pointer & conn, ClientHttpRequest *http, ch * Sets result->flags.parsed_ok to 1 if we have a good request. */ static ClientSocketContext * -parseHttpRequest(ConnStateData::Pointer & conn, method_t * method_p, - char **prefix_p, size_t * req_line_sz_p, HttpVersion *http_ver) +parseHttpRequest(ConnStateData::Pointer & conn, HttpParser *hp, method_t * method_p, HttpVersion *http_ver) { char *inbuf = NULL; char *url = NULL; char *req_hdr = NULL; char *t; char *end; - size_t header_sz; /* size of headers, not including first line */ - size_t prefix_sz; /* size of whole request (req-line + headers) */ size_t req_sz; ClientHttpRequest *http; ClientSocketContext *result; @@ -1943,38 +1939,43 @@ parseHttpRequest(ConnStateData::Pointer & conn, method_t * method_p, const char *http_version; /* pre-set these values to make aborting simpler */ - *prefix_p = NULL; *method_p = METHOD_NONE; /* Read the HTTP message. HTTP/0.9 is detected by the absence of a HTTP signature */ - if ((t = (char *)memchr(conn->in.buf, '\n', conn->in.notYetUsed)) == NULL) { + if ((t = (char *)memchr(hp->buf, '\n', hp->bufsiz)) == NULL) { debug(33, 5) ("Incomplete request, waiting for end of request line\n"); return NULL; } - *req_line_sz_p = t - conn->in.buf + 1; - http_version = findTrailingHTTPVersion(conn->in.buf); + hp->req_start = 0; + hp->req_end = t - hp->buf; + http_version = findTrailingHTTPVersion(hp->buf); if (http_version) { - if ((req_sz = headersEnd(conn->in.buf, conn->in.notYetUsed)) == 0) { + if ((req_sz = headersEnd(hp->buf, hp->bufsiz)) == 0) { debug(33, 5) ("Incomplete request, waiting for end of headers\n"); return NULL; } } else { debug(33, 3) ("parseHttpRequest: Missing HTTP identifier\n"); - req_sz = t - conn->in.buf + 1; /* HTTP/0.9 requests */ + req_sz = t - hp->buf + 1; /* HTTP/0.9 requests */ } - assert(req_sz <= conn->in.notYetUsed); + assert(req_sz <= (size_t) hp->bufsiz); + /* Will the following be true with HTTP/0.9 requests? probably not .. */ + /* So the rest of the code will need to deal with '0'-byte headers (ie, none, so don't try parsing em) */ + assert(req_sz > 0); + hp->hdr_end = req_sz - 1; + hp->hdr_start = hp->req_end + 1; /* Use memcpy, not strdup! */ inbuf = (char *)xmalloc(req_sz + 1); - xmemcpy(inbuf, conn->in.buf, req_sz); + xmemcpy(inbuf, hp->buf, req_sz); *(inbuf + req_sz) = '\0'; /* and adjust http_version to point into the new copy */ if (http_version) - http_version = inbuf + (http_version - conn->in.buf); + http_version = inbuf + (http_version - hp->buf); /* Enforce max_request_size */ @@ -2006,13 +2007,11 @@ parseHttpRequest(ConnStateData::Pointer & conn, method_t * method_p, * Process headers after request line * TODO: Use httpRequestParse here. */ - req_hdr = inbuf + *req_line_sz_p; - - header_sz = req_sz - *req_line_sz_p; + req_hdr = inbuf + hp->req_end; debug(33, 3) ("parseHttpRequest: req_hdr = {%s}\n", req_hdr); - end = req_hdr + header_sz; + end = req_hdr + HttpParserHdrSz(hp); debug(33, 3) ("parseHttpRequest: end = {%s}\n", end); @@ -2022,17 +2021,15 @@ parseHttpRequest(ConnStateData::Pointer & conn, method_t * method_p, return parseHttpRequestAbort(conn, "error:double-CR"); } - prefix_sz = end - inbuf; - debug(33, 3) ("parseHttpRequest: prefix_sz = %d, req_line_sz = %d\n", - (int) prefix_sz, (int) *req_line_sz_p); + (int) HttpParserRequestLen(hp), HttpParserReqSz(hp)); - assert(prefix_sz <= conn->in.notYetUsed); + assert((size_t) HttpParserRequestLen(hp) <= (size_t) hp->bufsiz); /* Ok, all headers are received */ http = new ClientHttpRequest(conn); - http->req_sz = prefix_sz; + http->req_sz = HttpParserRequestLen(hp); result = ClientSocketContextNew(http); @@ -2048,14 +2045,8 @@ parseHttpRequest(ConnStateData::Pointer & conn, method_t * method_p, clientReplyStatus, newServer, clientSocketRecipient, clientSocketDetach, newClient, tempBuffer); - *prefix_p = (char *)xmalloc(prefix_sz + 1); - - xmemcpy(*prefix_p, conn->in.buf, prefix_sz); - - *(*prefix_p + prefix_sz) = '\0'; - debug(33, 5) ("parseHttpRequest: Request Header is\n%s\n", - (*prefix_p) + *req_line_sz_p); + (hp->buf) + hp->hdr_start); #if THIS_VIOLATES_HTTP_SPECS_ON_URL_TRANSFORMATION @@ -2247,7 +2238,7 @@ clientAfterReadingRequests(int fd, ConnStateData::Pointer &conn, int do_next_rea } static void -clientProcessRequest(ConnStateData::Pointer &conn, ClientSocketContext *context, method_t method, char *prefix, size_t req_line_sz, HttpVersion http_ver) +clientProcessRequest(ConnStateData::Pointer &conn, HttpParser *hp, ClientSocketContext *context, method_t method, HttpVersion http_ver) { ClientHttpRequest *http = context->http; HttpRequest *request = NULL; @@ -2287,9 +2278,9 @@ clientProcessRequest(ConnStateData::Pointer &conn, ClientSocketContext *context, /* compile headers */ /* we should skip request line! */ /* XXX should actually know the damned buffer size here */ - if (!request->parseHeader(prefix + req_line_sz, strlen(prefix + req_line_sz))) { + if (!request->parseHeader(HttpParserHdrBuf(hp), HttpParserHdrSz(hp))) { clientStreamNode *node = context->getClientReplyContext(); - debug(33, 5) ("Failed to parse request headers:\n%s\n", prefix); + debug(33, 5) ("Failed to parse request headers:\n%s\n", HttpParserHdrBuf(hp)); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); repContext->setReplyToError( @@ -2452,15 +2443,14 @@ static bool clientParseRequest(ConnStateData::Pointer conn, bool &do_next_read) { method_t method; - char *prefix = NULL; ClientSocketContext *context; bool parsed_req = false; HttpVersion http_ver; + HttpParser hp; debug(33, 5) ("clientParseRequest: FD %d: attempting to parse\n", conn->fd); while (conn->in.notYetUsed > 0 && conn->bodySizeLeft() == 0) { - size_t req_line_sz; connStripBufferWhitespace (conn); /* Limit the number of concurrent requests to 2 */ @@ -2473,14 +2463,16 @@ clientParseRequest(ConnStateData::Pointer conn, bool &do_next_read) /* Terminate the string */ conn->in.buf[conn->in.notYetUsed] = '\0'; + /* Begin the parsing */ + HttpParserInit(&hp, conn->in.buf, conn->in.notYetUsed); + /* Process request */ PROF_start(parseHttpRequest); - context = parseHttpRequest(conn, &method, &prefix, &req_line_sz, &http_ver); + context = parseHttpRequest(conn, &hp, &method, &http_ver); PROF_stop(parseHttpRequest); /* partial or incomplete request */ if (!context) { - safe_free(prefix); if (!connKeepReadingIncompleteRequest(conn)) connCancelIncompleteRequests(conn); @@ -2494,9 +2486,8 @@ clientParseRequest(ConnStateData::Pointer conn, bool &do_next_read) commSetTimeout(conn->fd, Config.Timeout.lifetime, clientLifetimeTimeout, context->http); - clientProcessRequest(conn, context, method, prefix, req_line_sz, http_ver); + clientProcessRequest(conn, &hp, context, method, http_ver); - safe_free(prefix); parsed_req = true; if (context->mayUseConnection()) { @@ -2513,6 +2504,7 @@ clientParseRequest(ConnStateData::Pointer conn, bool &do_next_read) continue; /* while offset > 0 && conn->bodySizeLeft() == 0 */ } } /* while offset > 0 && conn->bodySizeLeft() == 0 */ + /* XXX where to 'finish' the parsing pass? */ return parsed_req; }