--------------------- PatchSet 8545 Date: 2006/08/24 13:30:30 Author: adri Branch: parserwork Tag: (none) Log: * Shift ClientCheckHeaderStateData into the global structure/typedef declarations * Fix a bug with my introduction of a test request parser, oops! * Refactor out the code which handles a request out of clientReadRequest() in preparation for some major surgery Members: src/HttpMsg.c:1.8.8.2->1.8.8.3 src/client_side.c:1.143.2.3->1.143.2.4 src/structs.h:1.128.2.3->1.128.2.4 src/typedefs.h:1.40.2.1->1.40.2.2 Index: squid/src/HttpMsg.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/HttpMsg.c,v retrieving revision 1.8.8.2 retrieving revision 1.8.8.3 diff -u -r1.8.8.2 -r1.8.8.3 --- squid/src/HttpMsg.c 23 Aug 2006 14:50:52 -0000 1.8.8.2 +++ squid/src/HttpMsg.c 24 Aug 2006 13:30:30 -0000 1.8.8.3 @@ -1,6 +1,6 @@ /* - * $Id: HttpMsg.c,v 1.8.8.2 2006/08/23 14:50:52 adri Exp $ + * $Id: HttpMsg.c,v 1.8.8.3 2006/08/24 13:30:30 adri Exp $ * * DEBUG: section 74 HTTP Message * AUTHOR: Alex Rousskov @@ -135,7 +135,7 @@ retcode = 1; finish: - debug(1, 1) ("Parser: retval %d: from %d->%d: method %d->%d; url %d->%d; version %d->%d\n", + debug(1, 2) ("Parser: retval %d: from %d->%d: method %d->%d; url %d->%d; version %d->%d\n", retcode, hmsg->req_start, hmsg->req_end, hmsg->m_start, hmsg->m_end, hmsg->u_start, hmsg->u_end, Index: squid/src/client_side.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/client_side.c,v retrieving revision 1.143.2.3 retrieving revision 1.143.2.4 diff -u -r1.143.2.3 -r1.143.2.4 --- squid/src/client_side.c 23 Aug 2006 14:50:53 -0000 1.143.2.3 +++ squid/src/client_side.c 24 Aug 2006 13:30:30 -0000 1.143.2.4 @@ -1,6 +1,6 @@ /* - * $Id: client_side.c,v 1.143.2.3 2006/08/23 14:50:53 adri Exp $ + * $Id: client_side.c,v 1.143.2.4 2006/08/24 13:30:30 adri Exp $ * * DEBUG: section 33 Client-side Routines * AUTHOR: Duane Wessels @@ -2572,14 +2572,6 @@ } } -typedef struct { - clientHttpRequest *http; - char *buf; - ssize_t size; - const char *body_buf; - ssize_t body_size; -} clientCheckHeaderStateData; - CBDATA_TYPE(clientCheckHeaderStateData); static void clientHttpLocationRewriteCheck(clientCheckHeaderStateData * state); @@ -3517,7 +3509,7 @@ *method_p = METHOD_NONE; *status = -1; - if (httpMsgParseRequestLine(hmsg)) { + if (! httpMsgParseRequestLine(hmsg)) { return NULL; }; @@ -3778,92 +3770,27 @@ } } -static void -clientReadRequest(int fd, void *data) + +/* + * Attempt to parse a request. + * + * @return 1 if parsing should continue; 0 if parsing should immediately stop + */ +static int +clientParseRequest(int fd, ConnStateData *conn) { - ConnStateData *conn = data; int parser_return_code = 0; request_t *request = NULL; - int size; + int nrequests; + size_t req_line_sz = 0; + HttpMsgBuf hmsg; method_t method; clientHttpRequest *http = NULL; clientHttpRequest **H = NULL; char *prefix = NULL; ErrorState *err = NULL; fde *F = &fd_table[fd]; - int len = conn->in.size - conn->in.offset - 1; - HttpMsgBuf hmsg; - debug(33, 4) ("clientReadRequest: FD %d: reading request...\n", fd); - if (len == 0) { - /* Grow the request memory area to accomodate for a large request */ - conn->in.buf = memReallocBuf(conn->in.buf, conn->in.size * 2, &conn->in.size); - debug(33, 2) ("growing request buffer: offset=%ld size=%ld\n", - (long) conn->in.offset, (long) conn->in.size); - len = conn->in.size - conn->in.offset - 1; - } - statCounter.syscalls.sock.reads++; - size = FD_READ_METHOD(fd, conn->in.buf + conn->in.offset, len); - if (size > 0) { - fd_bytes(fd, size, FD_READ); - kb_incr(&statCounter.client_http.kbytes_in, size); - } - /* - * Don't reset the timeout value here. The timeout value will be - * set to Config.Timeout.request by httpAccept() and - * clientWriteComplete(), and should apply to the request as a - * whole, not individual read() calls. Plus, it breaks our - * lame half-close detection - */ - if (size > 0) { - conn->in.offset += size; - conn->in.buf[conn->in.offset] = '\0'; /* Terminate the string */ - } else if (size == 0) { - if (conn->chr == NULL && conn->in.offset == 0) { - /* no current or pending requests */ - debug(33, 4) ("clientReadRequest: FD %d closed\n", fd); - comm_close(fd); - return; - } else if (!Config.onoff.half_closed_clients) { - /* admin doesn't want to support half-closed client sockets */ - debug(33, 3) ("clientReadRequest: FD %d aborted (half_closed_clients disabled)\n", fd); - comm_close(fd); - return; - } - /* It might be half-closed, we can't tell */ - debug(33, 5) ("clientReadRequest: FD %d closed?\n", fd); - F->flags.socket_eof = 1; - conn->defer.until = squid_curtime + 1; - conn->defer.n++; - fd_note(fd, "half-closed"); - /* There is one more close check at the end, to detect aborted - * (partial) requests. At this point we can't tell if the request - * is partial. - */ - /* Continue to process previously read data */ - } else if (size < 0) { - if (!ignoreErrno(errno)) { - debug(50, 2) ("clientReadRequest: FD %d: %s\n", fd, xstrerror()); - comm_close(fd); - return; - } else if (conn->in.offset == 0) { - debug(50, 2) ("clientReadRequest: FD %d: no data to process (%s)\n", fd, xstrerror()); - } - /* Continue to process previously read data */ - } - cbdataLock(conn); /* clientProcessBody might pull the connection under our feets */ - /* Process request body if any */ - if (conn->in.offset > 0 && conn->body.callback != NULL) { - clientProcessBody(conn); - if (!cbdataValid(conn)) { - cbdataUnlock(conn); - return; - } - } - /* Process next request */ - while (conn->in.offset > 0 && conn->body.size_left == 0) { - int nrequests; - size_t req_line_sz = 0; /* Skip leading (and trailing) whitespace */ while (conn->in.offset > 0 && xisspace(conn->in.buf[0])) { xmemmove(conn->in.buf, conn->in.buf + 1, conn->in.offset - 1); @@ -3871,14 +3798,14 @@ } conn->in.buf[conn->in.offset] = '\0'; /* Terminate the string */ if (conn->in.offset == 0) - break; + return 0; /* Limit the number of concurrent requests to 2 */ for (H = &conn->chr, nrequests = 0; *H; H = &(*H)->next, nrequests++); if (nrequests >= (Config.onoff.pipeline_prefetch ? 2 : 1)) { debug(33, 3) ("clientReadRequest: FD %d max concurrent requests reached\n", fd); debug(33, 5) ("clientReadRequest: FD %d defering new request until one is done\n", fd); conn->defer.until = squid_curtime + 100; /* Reset when a request is complete */ - break; + return 0; } conn->in.buf[conn->in.offset] = '\0'; /* Terminate the string */ if (nrequests == 0) @@ -3925,7 +3852,7 @@ http->entry = clientCreateStoreEntry(http, method, null_request_flags); errorAppendEntry(http->entry, err); safe_free(prefix); - break; + return 0; } if ((request = urlParse(method, http->uri)) == NULL) { debug(33, 5) ("Invalid URL: %s\n", http->uri); @@ -3937,7 +3864,7 @@ http->entry = clientCreateStoreEntry(http, method, null_request_flags); errorAppendEntry(http->entry, err); safe_free(prefix); - break; + return 0; } /* compile headers */ /* we should skip request line! */ @@ -3953,7 +3880,7 @@ http->entry = clientCreateStoreEntry(http, method, null_request_flags); errorAppendEntry(http->entry, err); safe_free(prefix); - break; + return 0; } safe_free(prefix); safe_free(http->log_uri); @@ -4002,7 +3929,7 @@ http->log_type = LOG_TCP_DENIED; http->entry = clientCreateStoreEntry(http, request->method, null_request_flags); errorAppendEntry(http->entry, err); - break; + return 0; } if (!clientCheckContentLength(request)) { err = errorCon(ERR_INVALID_REQ, HTTP_LENGTH_REQUIRED); @@ -4012,7 +3939,7 @@ http->log_type = LOG_TCP_DENIED; http->entry = clientCreateStoreEntry(http, request->method, null_request_flags); errorAppendEntry(http->entry, err); - break; + return 0; } http->request = requestLink(request); http->orig_request = requestLink(request); @@ -4031,7 +3958,7 @@ http->entry = clientCreateStoreEntry(http, METHOD_NONE, null_request_flags); errorAppendEntry(http->entry, err); - break; + return 0; } } if (request->method == METHOD_CONNECT) { @@ -4044,7 +3971,7 @@ debugObj(33, 1, "Previous request:\n", conn->chr->request, (ObjPackMethod) & httpRequestPackDebug); debugObj(33, 1, "This request:\n", request, (ObjPackMethod) & httpRequestPackDebug); } - break; + return 0; } else { clientCheckFollowXForwardedFor(http); } @@ -4068,10 +3995,93 @@ http->entry = clientCreateStoreEntry(http, METHOD_NONE, null_request_flags); errorAppendEntry(http->entry, err); } - break; + return 0; } if (!cbdataValid(conn)) - break; + return 0; + + /* Success .. of sorts. Ie, keep trying to parse */ + return 1; +} + +static void +clientReadRequest(int fd, void *data) +{ + ConnStateData *conn = data; + int size; + fde *F = &fd_table[fd]; + int len = conn->in.size - conn->in.offset - 1; + + debug(33, 4) ("clientReadRequest: FD %d: reading request...\n", fd); + if (len == 0) { + /* Grow the request memory area to accomodate for a large request */ + conn->in.buf = memReallocBuf(conn->in.buf, conn->in.size * 2, &conn->in.size); + debug(33, 2) ("growing request buffer: offset=%ld size=%ld\n", + (long) conn->in.offset, (long) conn->in.size); + len = conn->in.size - conn->in.offset - 1; + } + statCounter.syscalls.sock.reads++; + size = FD_READ_METHOD(fd, conn->in.buf + conn->in.offset, len); + if (size > 0) { + fd_bytes(fd, size, FD_READ); + kb_incr(&statCounter.client_http.kbytes_in, size); + } + /* + * Don't reset the timeout value here. The timeout value will be + * set to Config.Timeout.request by httpAccept() and + * clientWriteComplete(), and should apply to the request as a + * whole, not individual read() calls. Plus, it breaks our + * lame half-close detection + */ + if (size > 0) { + conn->in.offset += size; + conn->in.buf[conn->in.offset] = '\0'; /* Terminate the string */ + } else if (size == 0) { + if (conn->chr == NULL && conn->in.offset == 0) { + /* no current or pending requests */ + debug(33, 4) ("clientReadRequest: FD %d closed\n", fd); + comm_close(fd); + return; + } else if (!Config.onoff.half_closed_clients) { + /* admin doesn't want to support half-closed client sockets */ + debug(33, 3) ("clientReadRequest: FD %d aborted (half_closed_clients disabled)\n", fd); + comm_close(fd); + return; + } + /* It might be half-closed, we can't tell */ + debug(33, 5) ("clientReadRequest: FD %d closed?\n", fd); + F->flags.socket_eof = 1; + conn->defer.until = squid_curtime + 1; + conn->defer.n++; + fd_note(fd, "half-closed"); + /* There is one more close check at the end, to detect aborted + * (partial) requests. At this point we can't tell if the request + * is partial. + */ + /* Continue to process previously read data */ + } else if (size < 0) { + if (!ignoreErrno(errno)) { + debug(50, 2) ("clientReadRequest: FD %d: %s\n", fd, xstrerror()); + comm_close(fd); + return; + } else if (conn->in.offset == 0) { + debug(50, 2) ("clientReadRequest: FD %d: no data to process (%s)\n", fd, xstrerror()); + } + /* Continue to process previously read data */ + } + cbdataLock(conn); /* clientProcessBody might pull the connection under our feets */ + /* Process request body if any */ + if (conn->in.offset > 0 && conn->body.callback != NULL) { + clientProcessBody(conn); + if (!cbdataValid(conn)) { + cbdataUnlock(conn); + return; + } + } + /* Process next request */ + while (conn->in.offset > 0 && conn->body.size_left == 0) { + if (! clientParseRequest(fd, conn)) + break; } /* while offset > 0 && conn->body.size_left == 0 */ if (!cbdataValid(conn)) { cbdataUnlock(conn); Index: squid/src/structs.h =================================================================== RCS file: /cvsroot/squid-sf//squid/src/structs.h,v retrieving revision 1.128.2.3 retrieving revision 1.128.2.4 diff -u -r1.128.2.3 -r1.128.2.4 --- squid/src/structs.h 23 Aug 2006 14:51:00 -0000 1.128.2.3 +++ squid/src/structs.h 24 Aug 2006 13:30:32 -0000 1.128.2.4 @@ -1,6 +1,6 @@ /* - * $Id: structs.h,v 1.128.2.3 2006/08/23 14:51:00 adri Exp $ + * $Id: structs.h,v 1.128.2.4 2006/08/24 13:30:32 adri Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -2498,4 +2498,13 @@ int v_start, v_end; }; +struct _clientCheckHeaderStateData { + clientHttpRequest *http; + char *buf; + ssize_t size; + const char *body_buf; + ssize_t body_size; +}; + + #endif /* SQUID_STRUCTS_H */ Index: squid/src/typedefs.h =================================================================== RCS file: /cvsroot/squid-sf//squid/src/typedefs.h,v retrieving revision 1.40.2.1 retrieving revision 1.40.2.2 diff -u -r1.40.2.1 -r1.40.2.2 --- squid/src/typedefs.h 19 Aug 2006 16:52:47 -0000 1.40.2.1 +++ squid/src/typedefs.h 24 Aug 2006 13:30:32 -0000 1.40.2.2 @@ -1,6 +1,6 @@ /* - * $Id: typedefs.h,v 1.40.2.1 2006/08/19 16:52:47 adri Exp $ + * $Id: typedefs.h,v 1.40.2.2 2006/08/24 13:30:32 adri Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -407,5 +407,6 @@ typedef void STLVCB(VaryData * vary, void *cbdata); typedef struct _HttpMsgBuf HttpMsgBuf; +typedef struct _clientCheckHeaderStateData clientCheckHeaderStateData; #endif /* SQUID_TYPEDEFS_H */