From: Christos Tsantilas Date: Mon, 10 Nov 2014 11:45:36 +0000 (+0200) Subject: Adapting 100-continue X-Git-Tag: merge-candidate-3-v1~497 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ec69bdb2463e4ddbb49432d6154069369da71a3f;p=thirdparty%2Fsquid.git Adapting 100-continue Currently squid fails to handle correctly "100 Continue" requests/responses when ICAP is used. The problems discussed in squid bugzilla: http://bugs.squid-cache.org/show_bug.cgi?id=4067 A short discussion of the problem: When a data upload request enters squid (eg a PUT request with "Expect: 100-continue" header), squid sends ICAP headers and HTTP headers to ICAP server and stucks waiting for ever the request body data. This patch implements the "force_request_body_continuation" access list directive which controls how Squid handles data upload requests from HTTP and send the request body to Squid. An allow match tells Squid to respond with the HTTP 100 or FTP 150 (Please Continue) control message on its own, before forwarding the request to an adaptation service or peer. Such a response usually forces the request sender to proceed with sending the body. A deny match tells Squid to delay that control response until the origin server confirms that the request body is needed. Delaying is the default behavior. This is a Measurement Factory project --- diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index 6932bb1a6b..df92ff28de 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -112,6 +112,7 @@ HttpRequest::init() icapHistory_ = NULL; #endif rangeOffsetLimit = -2; //a value of -2 means not checked yet + forcedBodyContinuation = false; } void @@ -252,6 +253,8 @@ HttpRequest::inheritProperties(const HttpMsg *aMsg) myportname = aReq->myportname; + forcedBodyContinuation = aReq->forcedBodyContinuation; + // main property is which connection the request was received on (if any) clientConnectionManager = aReq->clientConnectionManager; diff --git a/src/HttpRequest.h b/src/HttpRequest.h index abd37013b9..0df54cf1cc 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -202,6 +202,9 @@ public: /// A strong etag of the cached entry. Used for refreshing that entry. String etag; + /// whether we have responded with HTTP 100 or FTP 150 already + bool forcedBodyContinuation; + public: bool multipartRangeRequest() const; diff --git a/src/SquidConfig.h b/src/SquidConfig.h index 47dda24563..26d83a9171 100644 --- a/src/SquidConfig.h +++ b/src/SquidConfig.h @@ -377,6 +377,8 @@ public: acl_access* spoof_client_ip; acl_access *ftp_epsv; + + acl_access *forceRequestBodyContinuation; } accessList; AclDenyInfoList *denyInfoList; diff --git a/src/cf.data.pre b/src/cf.data.pre index 09c037d3bb..a524ddbc62 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -9339,4 +9339,33 @@ DOC_START See also: workers DOC_END +NAME: force_request_body_continuation +TYPE: acl_access +LOC: Config.accessList.forceRequestBodyContinuation +DEFAULT: none +DEFAULT_DOC: Deny, unless rules exist in squid.conf. +DOC_START + This option controls how Squid handles data upload requests from HTTP + and FTP agents that require a "Please Continue" control message response + to actually send the request body to Squid. It is mostly useful in + adaptation environments. + + When Squid receives an HTTP request with an "Expect: 100-continue" + header or an FTP upload command (e.g., STOR), Squid normally sends the + request headers or FTP command information to an adaptation service (or + peer) and waits for a response. Most adaptation services (and some + broken peers) may not respond to Squid at that stage because they may + decide to wait for the HTTP request body or FTP data transfer. However, + that request body or data transfer may never come because Squid has not + responded with the HTTP 100 or FTP 150 (Please Continue) control message + to the request sender yet! + + An allow match tells Squid to respond with the HTTP 100 or FTP 150 + (Please Continue) control message on its own, before forwarding the + request to an adaptation service or peer. Such a response usually forces + the request sender to proceed with sending the body. A deny match tells + Squid to delay that control response until the origin server confirms + that the request body is needed. Delaying is the default behavior. +DOC_END + EOF diff --git a/src/client_side.cc b/src/client_side.cc index 34a77c01ec..f885b018fc 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2448,7 +2448,7 @@ bool ConnStateData::serveDelayedError(ClientSocketContext *context) } #endif // USE_OPENSSL -static void +void clientProcessRequestFinished(ConnStateData *conn, const HttpRequest::Pointer &request) { /* @@ -2468,106 +2468,18 @@ void clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp, ClientSocketContext *context) { ClientHttpRequest *http = context->http; - HttpRequest::Pointer request; bool chunked = false; bool mustReplyToOptions = false; bool unsupportedTe = false; bool expectBody = false; + // We already have the request parsed and checked, so we + // only need to go through the final body/conn setup to doCallouts(). + assert(http->request); + HttpRequest::Pointer request = http->request; + // temporary hack to avoid splitting this huge function with sensitive code const bool isFtp = !hp; - if (isFtp) { - // In FTP, case, we already have the request parsed and checked, so we - // only need to go through the final body/conn setup to doCallouts(). - assert(http->request); - request = http->request; - } else { - - if (context->flags.parsed_ok == 0) { - clientStreamNode *node = context->getClientReplyContext(); - debugs(33, 2, "Invalid Request"); - conn->quitAfterError(NULL); - // setLogUri should called before repContext->setReplyToError - setLogUri(http, http->uri, true); - clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); - assert(repContext); - - // determine which error page templates to use for specific parsing errors - err_type errPage = ERR_INVALID_REQ; - switch (hp->request_parse_status) { - case Http::scRequestHeaderFieldsTooLarge: - // fall through to next case - case Http::scUriTooLong: - errPage = ERR_TOO_BIG; - break; - case Http::scMethodNotAllowed: - errPage = ERR_UNSUP_REQ; - break; - case Http::scHttpVersionNotSupported: - errPage = ERR_UNSUP_HTTPVERSION; - break; - default: - // use default ERR_INVALID_REQ set above. - break; - } - repContext->setReplyToError(errPage, hp->request_parse_status, hp->method(), http->uri, - conn->clientConnection->remote, NULL, conn->in.buf.c_str(), NULL); - assert(context->http->out.offset == 0); - context->pullData(); - return; - } - - if ((request = HttpRequest::CreateFromUrlAndMethod(http->uri, hp->method())) == NULL) { - clientStreamNode *node = context->getClientReplyContext(); - debugs(33, 5, "Invalid URL: " << http->uri); - conn->quitAfterError(request.getRaw()); - // setLogUri should called before repContext->setReplyToError - setLogUri(http, http->uri, true); - clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); - assert(repContext); - repContext->setReplyToError(ERR_INVALID_URL, Http::scBadRequest, hp->method(), http->uri, conn->clientConnection->remote, NULL, NULL, NULL); - assert(context->http->out.offset == 0); - context->pullData(); - return; - } - - /* RFC 2616 section 10.5.6 : handle unsupported HTTP major versions cleanly. */ - /* We currently only support 0.9, 1.0, 1.1 properly */ - /* TODO: move HTTP-specific processing into servers/HttpServer and such */ - if ( (hp->messageProtocol().major == 0 && hp->messageProtocol().minor != 9) || - (hp->messageProtocol().major > 1) ) { - - clientStreamNode *node = context->getClientReplyContext(); - debugs(33, 5, "Unsupported HTTP version discovered. :\n" << hp->messageProtocol()); - conn->quitAfterError(request.getRaw()); - // setLogUri should called before repContext->setReplyToError - setLogUri(http, http->uri, true); - clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); - assert (repContext); - repContext->setReplyToError(ERR_UNSUP_HTTPVERSION, Http::scHttpVersionNotSupported, hp->method(), http->uri, - conn->clientConnection->remote, NULL, NULL, NULL); - assert(context->http->out.offset == 0); - context->pullData(); - clientProcessRequestFinished(conn, request); - return; - } - - /* compile headers */ - if (hp->messageProtocol().major >= 1 && !request->parseHeader(*hp)) { - clientStreamNode *node = context->getClientReplyContext(); - debugs(33, 5, "Failed to parse request headers:\n" << hp->mimeHeader()); - conn->quitAfterError(request.getRaw()); - // setLogUri should called before repContext->setReplyToError - setLogUri(http, http->uri, true); - clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); - assert(repContext); - repContext->setReplyToError(ERR_INVALID_REQ, Http::scBadRequest, hp->method(), http->uri, conn->clientConnection->remote, NULL, NULL, NULL); - assert(context->http->out.offset == 0); - context->pullData(); - clientProcessRequestFinished(conn, request); - return; - } - } // Some blobs below are still HTTP-specific, but we would have to rewrite // this entire function to remove them from the FTP code path. Connection @@ -2701,11 +2613,6 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp, } } - if (!isFtp) { - http->request = request.getRaw(); - HTTPMSGLOCK(http->request); - } - clientSetKeepaliveFlag(http); // Let tunneling code be fully responsible for CONNECT requests if (http->request->method == Http::METHOD_CONNECT) { diff --git a/src/clients/FtpRelay.cc b/src/clients/FtpRelay.cc index 1b62d35ed3..852fe35ac1 100644 --- a/src/clients/FtpRelay.cc +++ b/src/clients/FtpRelay.cc @@ -577,7 +577,9 @@ Ftp::Relay::readDataReply() if (ctrl.replycode == 125 || ctrl.replycode == 150) { if (serverState() == fssHandleDataRequest) forwardPreliminaryReply(&Ftp::Relay::startDataDownload); - else // serverState() == fssHandleUploadRequest + else if (fwd->request->forcedBodyContinuation /*&& serverState() == fssHandleUploadRequest*/) + startDataUpload(); + else // serverState() == fssHandleUploadRequest forwardPreliminaryReply(&Ftp::Relay::startDataUpload); } else forwardReply(); diff --git a/src/http.cc b/src/http.cc index 3426a15f3c..48adde746f 100644 --- a/src/http.cc +++ b/src/http.cc @@ -782,8 +782,8 @@ HttpStateData::handle1xx(HttpReply *reply) Must(!flags.handling1xx); flags.handling1xx = true; - if (!request->canHandle1xx()) { - debugs(11, 2, HERE << "ignoring client-unsupported 1xx"); + if (!request->canHandle1xx() || request->forcedBodyContinuation) { + debugs(11, 2, "ignoring 1xx because it is " << (request->forcedBodyContinuation ? "already sent" : "not supported by client")); proceedAfter1xx(); return; } diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc index b8429ac57a..87f4e75d2b 100644 --- a/src/servers/FtpServer.cc +++ b/src/servers/FtpServer.cc @@ -9,6 +9,7 @@ /* DEBUG: section 33 Transfer protocol servers */ #include "squid.h" +#include "acl/FilledChecklist.h" #include "base/CharacterSet.h" #include "base/RefCount.h" #include "base/Subscription.h" @@ -1490,6 +1491,26 @@ Ftp::Server::handleUploadRequest(String &cmd, String ¶ms) if (!checkDataConnPre()) return false; + if (Config.accessList.forceRequestBodyContinuation) { + ClientHttpRequest *http = getCurrentContext()->http; + HttpRequest *request = http->request; + ACLFilledChecklist bodyContinuationCheck(Config.accessList.forceRequestBodyContinuation, request, NULL); + if (bodyContinuationCheck.fastCheck() == ACCESS_ALLOWED) { + request->forcedBodyContinuation = true; + if (checkDataConnPost()) { + // Write control Msg + writeEarlyReply(150, "Data connection opened"); + maybeReadUploadData(); + } else { + // wait for acceptDataConnection but tell it to call wroteEarlyReply + // after writing "150 Data connection opened" + typedef CommCbMemFunT Dialer; + AsyncCall::Pointer call = JobCallback(33, 5, Dialer, this, Ftp::Server::wroteEarlyReply); + onDataAcceptCall = call; + } + } + } + changeState(fssHandleUploadRequest, "handleDataRequest"); return true; diff --git a/src/servers/HttpServer.cc b/src/servers/HttpServer.cc index 37e3c62aa8..044b77d82f 100644 --- a/src/servers/HttpServer.cc +++ b/src/servers/HttpServer.cc @@ -9,8 +9,10 @@ /* DEBUG: section 33 Client-side Routines */ #include "squid.h" +#include "acl/FilledChecklist.h" #include "client_side.h" #include "client_side_request.h" +#include "client_side_reply.h" #include "comm/Write.h" #include "http/one/RequestParser.h" #include "HttpHeaderTools.h" @@ -47,10 +49,18 @@ protected: /* AsyncJob API */ virtual void start(); + void proceedAfterBodyContinuation(ClientSocketContext::Pointer context); + private: void processHttpRequest(ClientSocketContext *const context); void handleHttpRequestData(); + /// Handles parsing results. May generate and deliver an error reply + /// to the client if parsing is failed, or parses the url and build the + /// HttpRequest object using parsing results. + /// Return false if parsing is failed, true otherwise. + bool buildHttpRequest(ClientSocketContext *context); + Http1::RequestParserPointer parser_; HttpRequestMethod method_; ///< parsed HTTP method @@ -127,9 +137,134 @@ Http::Server::parseOneRequest() return context; } +void clientProcessRequestFinished(ConnStateData *conn, const HttpRequest::Pointer &request); + +bool +Http::Server::buildHttpRequest(ClientSocketContext *context) +{ + HttpRequest::Pointer request; + ClientHttpRequest *http = context->http; + if (context->flags.parsed_ok == 0) { + clientStreamNode *node = context->getClientReplyContext(); + debugs(33, 2, "Invalid Request"); + quitAfterError(NULL); + // setLogUri should called before repContext->setReplyToError + setLogUri(http, http->uri, true); + clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); + assert(repContext); + + // determine which error page templates to use for specific parsing errors + err_type errPage = ERR_INVALID_REQ; + switch (parser_->request_parse_status) { + case Http::scRequestHeaderFieldsTooLarge: + // fall through to next case + case Http::scUriTooLong: + errPage = ERR_TOO_BIG; + break; + case Http::scMethodNotAllowed: + errPage = ERR_UNSUP_REQ; + break; + case Http::scHttpVersionNotSupported: + errPage = ERR_UNSUP_HTTPVERSION; + break; + default: + // use default ERR_INVALID_REQ set above. + break; + } + repContext->setReplyToError(errPage, parser_->request_parse_status, parser_->method(), http->uri, + clientConnection->remote, NULL, in.buf.c_str(), NULL); + assert(context->http->out.offset == 0); + context->pullData(); + return false; + } + + if ((request = HttpRequest::CreateFromUrlAndMethod(http->uri, parser_->method())) == NULL) { + clientStreamNode *node = context->getClientReplyContext(); + debugs(33, 5, "Invalid URL: " << http->uri); + quitAfterError(request.getRaw()); + // setLogUri should called before repContext->setReplyToError + setLogUri(http, http->uri, true); + clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); + assert(repContext); + repContext->setReplyToError(ERR_INVALID_URL, Http::scBadRequest, parser_->method(), http->uri, clientConnection->remote, NULL, NULL, NULL); + assert(context->http->out.offset == 0); + context->pullData(); + return false; + } + + /* RFC 2616 section 10.5.6 : handle unsupported HTTP major versions cleanly. */ + /* We currently only support 0.9, 1.0, 1.1 properly */ + /* TODO: move HTTP-specific processing into servers/HttpServer and such */ + if ( (parser_->messageProtocol().major == 0 && parser_->messageProtocol().minor != 9) || + (parser_->messageProtocol().major > 1) ) { + + clientStreamNode *node = context->getClientReplyContext(); + debugs(33, 5, "Unsupported HTTP version discovered. :\n" << parser_->messageProtocol()); + quitAfterError(request.getRaw()); + // setLogUri should called before repContext->setReplyToError + setLogUri(http, http->uri, true); + clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); + assert (repContext); + repContext->setReplyToError(ERR_UNSUP_HTTPVERSION, Http::scHttpVersionNotSupported, parser_->method(), http->uri, + clientConnection->remote, NULL, NULL, NULL); + assert(context->http->out.offset == 0); + context->pullData(); + clientProcessRequestFinished(this, request); + return false; + } + + /* compile headers */ + if (parser_->messageProtocol().major >= 1 && !request->parseHeader(*parser_.getRaw())) { + clientStreamNode *node = context->getClientReplyContext(); + debugs(33, 5, "Failed to parse request headers:\n" << parser_->mimeHeader()); + quitAfterError(request.getRaw()); + // setLogUri should called before repContext->setReplyToError + setLogUri(http, http->uri, true); + clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); + assert(repContext); + repContext->setReplyToError(ERR_INVALID_REQ, Http::scBadRequest, parser_->method(), http->uri, clientConnection->remote, NULL, NULL, NULL); + assert(context->http->out.offset == 0); + context->pullData(); + clientProcessRequestFinished(this, request); + return false; + } + + http->request = request.getRaw(); + HTTPMSGLOCK(http->request); + + return true; +} + +void +Http::Server::proceedAfterBodyContinuation(ClientSocketContext::Pointer context) +{ + debugs(33, 5, "Body Continuation written"); + clientProcessRequest(this, parser_, context.getRaw()); +} + void Http::Server::processParsedRequest(ClientSocketContext *context) { + if (!buildHttpRequest(context)) + return; + + if (Config.accessList.forceRequestBodyContinuation) { + ClientHttpRequest *http = context->http; + HttpRequest *request = http->request; + ACLFilledChecklist bodyContinuationCheck(Config.accessList.forceRequestBodyContinuation, request, NULL); + if (bodyContinuationCheck.fastCheck() == ACCESS_ALLOWED) { + debugs(33, 5, "Body Continuation forced"); + request->forcedBodyContinuation = true; + //sendControlMsg + HttpReply::Pointer rep = new HttpReply; + rep->sline.set(Http::ProtocolVersion(1,1), Http::scContinue); + + typedef UnaryMemFunT CbDialer; + const AsyncCall::Pointer cb = asyncCall(11, 3, "Http::Server::proceedAfterBodyContinuation", CbDialer(this, &Http::Server::proceedAfterBodyContinuation, ClientSocketContext::Pointer(context))); + sendControlMsg(HttpControlMsg(rep, cb)); + return; + } + } clientProcessRequest(this, parser_, context); }