From: Henrik Nordstrom Date: Sat, 15 Mar 2008 23:43:18 +0000 (+0100) Subject: Properly build a cloned reply, and skip reading of large headers X-Git-Tag: BASIC_TPROXY4~3^2^2~13 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b297bcd06f78f38f83aca7053f93b53d40e7ff4f;p=thirdparty%2Fsquid.git Properly build a cloned reply, and skip reading of large headers more work remains to fix up the other paths (cache hits, IMS, Vary?) --- diff --git a/squid3.dox b/squid3.dox index cf16c43a6d..67fdc248dd 100644 --- a/squid3.dox +++ b/squid3.dox @@ -634,7 +634,7 @@ HTML_OUTPUT = tmp # each generated HTML page (for example: .htm,.php,.asp). If it is left blank # doxygen will generate files with .html extension. -HTML_FILE_EXTENSION = .dyn +#HTML_FILE_EXTENSION = .dyn # The HTML_HEADER tag can be used to specify a personal HTML header for # each generated HTML page. If it is left blank doxygen will generate a diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc index 7acb589c11..939f4ba413 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -350,68 +350,47 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result) sendClientOldEntry(); } - // we have a partial reply from the origin - else if (STORE_PENDING == http->storeEntry()->store_status && 0 == status) { - // header is too large, send old entry - - if (reqsize >= HTTP_REQBUF_SZ) { - debugs(88, 3, "handleIMSReply: response from origin is too large '" << http->storeEntry()->url() << "', sending old entry to client" ); - http->logType = LOG_TCP_REFRESH_FAIL; - sendClientOldEntry(); - } + HttpReply *old_rep = (HttpReply *) old_entry->getReply(); - // everything looks fine, we're just waiting for more data - else { - debugs(88, 3, "handleIMSReply: incomplete headers for '" << http->storeEntry()->url() << "', waiting for more data" ); - reqofs = reqsize; - waitForMoreData(); - } - } + // origin replied 304 - // we have a reply from the origin - else { - HttpReply *old_rep = (HttpReply *) old_entry->getReply(); + if (status == HTTP_NOT_MODIFIED) { + http->logType = LOG_TCP_REFRESH_UNMODIFIED; - // origin replied 304 + // update headers on existing entry + HttpReply *old_rep = (HttpReply *) old_entry->getReply(); + old_rep->updateOnNotModified(http->storeEntry()->getReply()); + old_entry->timestampsSet(); - if (status == HTTP_NOT_MODIFIED) { - http->logType = LOG_TCP_REFRESH_UNMODIFIED; + // if client sent IMS - // update headers on existing entry - HttpReply *old_rep = (HttpReply *) old_entry->getReply(); - old_rep->updateOnNotModified(http->storeEntry()->getReply()); - old_entry->timestampsSet(); - - // if client sent IMS - - if (http->request->flags.ims) { - // forward the 304 from origin - debugs(88, 3, "handleIMSReply: origin replied 304, revalidating existing entry and forwarding 304 to client"); - sendClientUpstreamResponse(); - } else { - // send existing entry, it's still valid - debugs(88, 3, "handleIMSReply: origin replied 304, revalidating existing entry and sending " << - old_rep->sline.status << " to client"); - sendClientOldEntry(); - } - } + if (http->request->flags.ims) { + // forward the 304 from origin + debugs(88, 3, "handleIMSReply: origin replied 304, revalidating existing entry and forwarding 304 to client"); + sendClientUpstreamResponse(); + } else { + // send existing entry, it's still valid + debugs(88, 3, "handleIMSReply: origin replied 304, revalidating existing entry and sending " << + old_rep->sline.status << " to client"); + sendClientOldEntry(); + } + } - // origin replied with a non-error code - else if (status > HTTP_STATUS_NONE && status < HTTP_INTERNAL_SERVER_ERROR) { - // forward response from origin - http->logType = LOG_TCP_REFRESH_MODIFIED; - debugs(88, 3, "handleIMSReply: origin replied " << status << ", replacing existing entry and forwarding to client"); - sendClientUpstreamResponse(); - } + // origin replied with a non-error code + else if (status > HTTP_STATUS_NONE && status < HTTP_INTERNAL_SERVER_ERROR) { + // forward response from origin + http->logType = LOG_TCP_REFRESH_MODIFIED; + debugs(88, 3, "handleIMSReply: origin replied " << status << ", replacing existing entry and forwarding to client"); + sendClientUpstreamResponse(); + } - // origin replied with an error - else { - // ignore and let client have old entry - http->logType = LOG_TCP_REFRESH_FAIL; - debugs(88, 3, "handleIMSReply: origin replied with error " << - status << ", sending old entry (" << old_rep->sline.status << ") to client"); - sendClientOldEntry(); - } + // origin replied with an error + else { + // ignore and let client have old entry + http->logType = LOG_TCP_REFRESH_FAIL; + debugs(88, 3, "handleIMSReply: origin replied with error " << + status << ", sending old entry (" << old_rep->sline.status << ") to client"); + sendClientOldEntry(); } } @@ -470,33 +449,6 @@ clientReplyContext::cacheHit(StoreIOBuffer result) /* update size of the request */ reqsize = result.length + reqofs; - if (e->getReply()->sline.status == 0) { - /* - * we don't have full reply headers yet; either wait for more or - * punt to clientProcessMiss. - */ - - if (e->mem_status == IN_MEMORY || e->store_status == STORE_OK) { - processMiss(); - } else if (result.length + reqofs >= HTTP_REQBUF_SZ - && http->out.offset == 0) { - processMiss(); - } else { - debugs(88, 3, "clientCacheHit: waiting for HTTP reply headers"); - reqofs += result.length; - assert(reqofs <= HTTP_REQBUF_SZ); - /* get the next users' buffer */ - StoreIOBuffer tempBuffer; - tempBuffer.offset = http->out.offset + reqofs; - tempBuffer.length = next()->readBuffer.length - reqofs; - tempBuffer.data = next()->readBuffer.data + reqofs; - storeClientCopy(sc, e, - tempBuffer, CacheHit, this); - } - - return; - } - /* * Got the headers, now grok them */ @@ -1390,32 +1342,14 @@ clientReplyContext::buildReplyHeader() void -clientReplyContext::buildReply(const char *buf, size_t size) +clientReplyContext::cloneReply() { - size_t k = headersEnd(buf, size); - - if (!k) - return; - assert(reply == NULL); - HttpReply *rep = new HttpReply; + HttpReply *rep = http->storeEntry()->getReply()->clone(); reply = HTTPMSGLOCK(rep); - if (!reply->parseCharBuf(buf, k)) { - /* parsing failure, get rid of the invalid reply */ - HTTPMSGUNLOCK(reply); - - if (http->request->range) { - debugs(0,0,HERE << "look for bug here"); - /* this will fail and destroy request->range */ - // clientBuildRangeHeader(http, reply); - } - - return; - } - /* enforce 1.0 reply version */ reply->sline.version = HttpVersion(1,0); @@ -1738,12 +1672,11 @@ void clientReplyContext::startSendProcess() { debugs(88, 5, "clientReplyContext::startSendProcess: triggering store read to SendMoreData"); - assert(reqofs <= HTTP_REQBUF_SZ); /* TODO: copy into the supplied buffer */ StoreIOBuffer tempBuffer; tempBuffer.offset = reqofs; - tempBuffer.length = next()->readBuffer.length - reqofs; - tempBuffer.data = next()->readBuffer.data + reqofs; + tempBuffer.length = next()->readBuffer.length; + tempBuffer.data = next()->readBuffer.data; storeClientCopy(sc, http->storeEntry(), tempBuffer, SendMoreData, this); } @@ -1837,8 +1770,10 @@ clientReplyContext::processReplyAccessResult(bool accessAllowed) http->loggingEntry(http->storeEntry()); ssize_t body_size = reqofs - reply->hdr_sz; - - assert(body_size >= 0); + if (body_size < 0) { + reqofs = reply->hdr_sz; + body_size = 0; + } debugs(88, 3, "clientReplyContext::sendMoreData: Appending " << (int) body_size << " bytes after " << reply->hdr_sz << @@ -1868,7 +1803,7 @@ clientReplyContext::processReplyAccessResult(bool accessAllowed) StoreIOBuffer tempBuffer; char *buf = next()->readBuffer.data; - char *body_buf = buf + reply->hdr_sz; + char *body_buf = buf + reply->hdr_sz - next()->readBuffer.offset; //Server side may disable ranges under some circumstances. @@ -1952,40 +1887,23 @@ clientReplyContext::sendMoreData (StoreIOBuffer result) return; } - if (!reply) { - reply = HTTPMSGLOCK(entry->mem_obj->getReply()->clone()); - } - if ((long)reqofs < reply->hdr_sz) { - waitForMoreData(); - return; - } - if (reply) { + cloneReply(); - /* handle headers */ + /* handle headers */ - if (Config.onoff.log_mime_hdrs) { - size_t k; - - if ((k = headersEnd(buf, reqofs))) { - safe_free(http->al.headers.reply); - http->al.headers.reply = (char *)xcalloc(k + 1, 1); - xstrncpy(http->al.headers.reply, buf, k); - } - } - - holdingBuffer = result; - processReplyAccess(); - return; + if (Config.onoff.log_mime_hdrs) { + size_t k; - } else { - debugs(88, 0, "clientReplyContext::sendMoreData: Unable to parse reply headers within a single HTTP_REQBUF_SZ length buffer"); - StoreIOBuffer tempBuffer; - tempBuffer.flags.error = 1; - /* XXX FIXME: make an html error page here */ - sendStreamError(tempBuffer); - return; + if ((k = headersEnd(buf, reqofs))) { + safe_free(http->al.headers.reply); + http->al.headers.reply = (char *)xcalloc(k + 1, 1); + xstrncpy(http->al.headers.reply, buf, k); + } } - fatal ("clientReplyContext::sendMoreData: Unreachable code reached \n"); + + holdingBuffer = result; + processReplyAccess(); + return; } diff --git a/src/client_side_reply.h b/src/client_side_reply.h index a69d011106..be2d932903 100644 --- a/src/client_side_reply.h +++ b/src/client_side_reply.h @@ -132,7 +132,7 @@ private: void processReplyAccess(); static PF ProcessReplyAccessResult; void processReplyAccessResult(bool accessAllowed); - void buildReply(const char *buf, size_t size); + void cloneReply(); void buildReplyHeader (); bool alwaysAllowResponse(http_status sline) const; int checkTransferDone();