Compliance: Forward 1xx control messages to clients that support them. Forward 1xx control messages to all HTTP/1.1 clients and to HTTP/1.0 clients that sent an Expect: 100-continue header unless the 1xx message fails the optional http_reply_access check described below. RFC 2616 requires clients to accept 1xx control messages, even if they did not send Expect headers. However, 1xx control messages prohibited by http_reply_access are ignored and not forwarded. This can be used to protect broken HTTP/1.1 clients or naive HTTP/1.0 clients that unknowingly forward 100-continue headers, for example. Only fast checks are supported at this time. The patch removes ignore_expect_100 squid.conf option and the corresponding code because - the reasons to treat 100-continue specially have changed since we can now forward 1xx responses; - rejection of 100-continue request can still be done using a combination of the existing http_access and deny_info options; - hiding of 100-continue header from next hops can still be done using an existing request_header_access option; - 100 (Continue) responses can be hidden from clients using http_reply_access check described above. We still respond with 417 Expectation Failed to requests with expectations other than 100-continue. Implementation notes: We forward control messages one-at-a-time and stop processing the server response while the 1xx message is being written to client, to avoid server-driven DoS attacks with large number of 1xx messages. 1xx forwarding is done via async calls from HttpStateData to ConnStateData/ClientSocketContext. The latter then calls back to notify HttpStateData that the message was written out to client. If any one of the two async messages is not fired, HttpStateData will get stuck unless it is destroyed due to an external event/error. The code assumes such event/error will always happen because when ConnStateData/ClientSocketContext is gone, HttpStateData job should be terminated. This requires more testing/thought, but should still be better than not forwarding 1xx messages at all. === modified file 'src/HttpRequest.cc' --- src/HttpRequest.cc 2010-08-24 04:18:51 +0000 +++ src/HttpRequest.cc 2010-08-31 04:36:54 +0000 @@ -644,3 +644,15 @@ return rangeOffsetLimit; } + +bool +HttpRequest::canHandle1xx() const +{ + // old clients do not support 1xx unless they sent Expect: 100-continue + // (we reject all other HDR_EXPECT values so just check for HDR_EXPECT) + if (http_ver <= HttpVersion(1,0) && !header.has(HDR_EXPECT)) + return false; + + // others must support 1xx control messages + return true; +} === modified file 'src/HttpRequest.h' --- src/HttpRequest.h 2010-04-17 10:38:50 +0000 +++ src/HttpRequest.h 2010-08-30 23:57:32 +0000 @@ -41,6 +41,7 @@ #if ICAP_CLIENT #include "adaptation/icap/History.h" #endif +#include "base/CbcPointer.h" #include "client_side.h" #if USE_SQUID_EUI #include "eui/Eui48.h" @@ -88,6 +89,9 @@ /* are responses to this request potentially cachable */ bool cacheable() const; + /// whether the client is likely to be able to handle a 1xx reply + bool canHandle1xx() const; + /* Now that we care what host contains it is better off being protected. */ /* HACK: These two methods are only inline to get around Makefile dependancies */ /* caused by HttpRequest being used in places it really shouldn't. */ @@ -248,6 +252,9 @@ cbdataReferenceDone(pinned_connection); } + /// client-side conn manager, if known; used for 1xx response forwarding + CbcPointer clientConnection; + int64_t getRangeOffsetLimit(); /* the result of this function gets cached in rangeOffsetLimit */ private: === modified file 'src/Makefile.am' --- src/Makefile.am 2010-08-20 16:15:46 +0000 +++ src/Makefile.am 2010-08-30 23:12:32 +0000 @@ -354,6 +354,7 @@ HttpBody.cc \ HttpMsg.cc \ HttpMsg.h \ + HttpControlMsg.h \ HttpReply.cc \ HttpReply.h \ HttpRequest.cc \ === modified file 'src/cf.data.pre' --- src/cf.data.pre 2010-08-24 11:31:44 +0000 +++ src/cf.data.pre 2010-08-31 04:37:27 +0000 @@ -4165,21 +4165,6 @@ or response to be rejected. DOC_END -NAME: ignore_expect_100 -COMMENT: on|off -IFDEF: USE_HTTP_VIOLATIONS -TYPE: onoff -LOC: Config.onoff.ignore_expect_100 -DEFAULT: off -DOC_START - This option makes Squid ignore any Expect: 100-continue header present - in the request. RFC 2616 requires that Squid being unable to satisfy - the response expectation MUST return a 417 error. - - Note: Enabling this is a HTTP protocol violation, but some clients may - not handle it well.. -DOC_END - COMMENT_START TIMEOUTS ----------------------------------------------------------------------------- === modified file 'src/client_side.cc' --- src/client_side.cc 2010-08-24 04:18:51 +0000 +++ src/client_side.cc 2010-08-31 04:40:22 +0000 @@ -342,6 +342,59 @@ return newContext; } +void +ClientSocketContext::writeControlMsg(HttpControlMsg &msg) +{ + HttpReply *rep = msg.reply; + Must(rep); + + // apply selected clientReplyContext::buildReplyHeader() mods + // it is not clear what headers are required for control messages + rep->header.removeHopByHopEntries(); + rep->header.putStr(HDR_CONNECTION, "keep-alive"); + httpHdrMangleList(&rep->header, http->request, ROR_REPLY); + + // remember the callback + cbControlMsgSent = msg.cbSuccess; + + MemBuf *mb = rep->pack(); + + AsyncCall::Pointer call = commCbCall(33, 5, "ClientSocketContext::wroteControlMsg", + CommIoCbPtrFun(&WroteControlMsg, this)); + comm_write_mbuf(fd(), mb, call); + + delete mb; +} + +/// called when we wrote the 1xx response +void +ClientSocketContext::wroteControlMsg(int fd, char *, size_t, comm_err_t errflag, int xerrno) +{ + if (errflag == COMM_ERR_CLOSING) + return; + + if (errflag == COMM_OK) { + ScheduleCallHere(cbControlMsgSent); + return; + } + + debugs(33, 3, HERE << "1xx writing failed: " << xstrerr(xerrno)); + // no error notification: see HttpControlMsg.h for rationale and + // note that some errors are detected elsewhere (e.g., close handler) + + // close on 1xx errors to be conservative and to simplify the code + // (if we do not close, we must notify the source of a failure!) + comm_close(fd); +} + +/// wroteControlMsg() wrapper: ClientSocketContext is not an AsyncJob +void +ClientSocketContext::WroteControlMsg(int fd, char *bufnotused, size_t size, comm_err_t errflag, int xerrno, void *data) +{ + ClientSocketContext *context = static_cast(data); + context->wroteControlMsg(fd, bufnotused, size, errflag, xerrno); +} + #if USE_IDENT static void clientIdentDone(const char *ident, void *data) @@ -2552,16 +2605,9 @@ } if (request->header.has(HDR_EXPECT)) { - int ignore = 0; -#if USE_HTTP_VIOLATIONS - if (Config.onoff.ignore_expect_100) { - String expect = request->header.getList(HDR_EXPECT); - if (expect.caseCmp("100-continue") == 0) - ignore = 1; - expect.clean(); - } -#endif - if (!ignore) { + const String expect = request->header.getList(HDR_EXPECT); + const bool supportedExpect = (expect.caseCmp("100-continue") == 0); + if (!supportedExpect) { clientStreamNode *node = context->getClientReplyContext(); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); @@ -3884,6 +3930,24 @@ delete bodyParser; // TODO: pool } +void +ConnStateData::sendControlMsg(HttpControlMsg msg) +{ + ClientSocketContext::Pointer context = getCurrentContext(); + if (context != NULL) { + context->writeControlMsg(msg); // will call msg.cbSuccess + return; + } + + if (!isOpen()) { + debugs(33, 3, HERE << "ignoring 1xx due to earlier closure"); + return; + } + + debugs(33, 3, HERE << " closing due to missing context for 1xx"); + comm_close(fd); +} + /* This is a comm call normally scheduled by comm_close() */ void ConnStateData::clientPinnedConnectionClosed(const CommCloseCbParams &io) === modified file 'src/client_side.h' --- src/client_side.h 2010-08-24 00:02:15 +0000 +++ src/client_side.h 2010-08-30 23:58:08 +0000 @@ -40,6 +40,7 @@ #include "CommCalls.h" #include "eui/Eui48.h" #include "eui/Eui64.h" +#include "HttpControlMsg.h" #include "RefCount.h" #include "StoreIOBuffer.h" @@ -112,6 +113,13 @@ void registerWithConn(); void noteIoError(const int xerrno); ///< update state to reflect I/O error + // starts writing 1xx control message to the client + void writeControlMsg(HttpControlMsg &msg); + +protected: + static void WroteControlMsg(int fd, char *bufnotused, size_t size, comm_err_t errflag, int xerrno, void *data); + void wroteControlMsg(int fd, char *bufnotused, size_t size, comm_err_t errflag, int xerrno); + private: CBDATA_CLASS(ClientSocketContext); void prepareReply(HttpReply * rep); @@ -120,6 +128,9 @@ void deRegisterWithConn(); void doClose(); void initiateClose(const char *reason); + + AsyncCall::Pointer cbControlMsgSent; ///< notifies HttpControlMsg Source + bool mayUseConnection_; /* This request may use the connection. Don't read anymore requests for now */ bool connRegistered_; }; @@ -128,7 +139,7 @@ class ConnectionDetail; /** A connection to a socket */ -class ConnStateData : public BodyProducer/*, public RefCountable*/ +class ConnStateData : public BodyProducer, public HttpControlMsgSink { public: @@ -148,6 +159,9 @@ int getConcurrentRequestCount() const; bool isOpen() const; + // HttpControlMsgSink API + virtual void sendControlMsg(HttpControlMsg msg); + int fd; /// chunk buffering and parsing algorithm state === modified file 'src/client_side_reply.cc' --- src/client_side_reply.cc 2010-08-24 04:07:00 +0000 +++ src/client_side_reply.cc 2010-08-30 23:12:32 +0000 @@ -261,6 +261,8 @@ http->storeEntry(entry); assert(http->out.offset == 0); + http->request->clientConnection = http->getConn(); + /* * A refcounted pointer so that FwdState stays around as long as * this clientReplyContext does @@ -680,6 +682,8 @@ if (http->flags.internal) r->protocol = PROTO_INTERNAL; + r->clientConnection = http->getConn(); + /** Start forwarding to get the new object from network */ FwdState::fwdStart(http->getConn() != NULL ? http->getConn()->fd : -1, http->storeEntry(), === modified file 'src/http.cc' --- src/http.cc 2010-08-24 04:18:51 +0000 +++ src/http.cc 2010-08-31 04:16:14 +0000 @@ -42,6 +42,7 @@ #include "acl/FilledChecklist.h" #include "auth/UserRequest.h" +#include "base/AsyncJobCalls.h" #include "base/TextException.h" #if DELAY_POOLS #include "DelayPools.h" @@ -49,6 +50,7 @@ #include "errorpage.h" #include "fde.h" #include "http.h" +#include "HttpControlMsg.h" #include "HttpHdrContRange.h" #include "HttpHdrSc.h" #include "HttpHdrScTarget.h" @@ -705,23 +707,9 @@ readBuf->consume(header_bytes_read); } - /* Skip 1xx messages for now. Advertised in Via as an internal 1.0 hop */ if (newrep->sline.protocol == PROTO_HTTP && newrep->sline.status >= 100 && newrep->sline.status < 200) { - -#if WHEN_HTTP11_EXPECT_HANDLED - /* When HTTP/1.1 check if the client is expecting a 1xx reply and maybe pass it on */ - if (orig_request->header.has(HDR_EXPECT)) { - // TODO: pass to the client anyway? - } -#endif - delete newrep; - debugs(11, 2, HERE << "1xx headers consume " << header_bytes_read << " bytes header."); - header_bytes_read = 0; - if (reply_bytes_read > 0) - debugs(11, 2, HERE << "1xx headers consume " << reply_bytes_read << " bytes reply."); - reply_bytes_read = 0; + handle1xx(newrep); ctx_exit(ctx); - processReplyHeader(); return; } @@ -752,6 +740,64 @@ ctx_exit(ctx); } +/// ignore or start forwarding the 1xx response (a.k.a., control message) +void +HttpStateData::handle1xx(HttpReply *reply) +{ + HttpMsgPointerT msg(reply); // will destroy reply if unused + + // one 1xx at a time: we must not be called while waiting for previous 1xx + Must(!flags.handling1xx); + flags.handling1xx = true; + + if (!orig_request->canHandle1xx()) { + debugs(11, 2, HERE << "ignoring client-unsupported 1xx"); + proceedAfter1xx(); + return; + } + +#if USE_HTTP_VIOLATIONS + // check whether the 1xx response forwarding is allowed by squid.conf + if (Config.accessList.reply) { + ACLFilledChecklist ch(Config.accessList.reply, request, NULL); + ch.reply = HTTPMSGLOCK(reply); + if (!ch.fastCheck()) { // TODO: support slow lookups? + debugs(11, 3, HERE << "ignoring denied 1xx"); + proceedAfter1xx(); + return; + } + } +#endif // USE_HTTP_VIOLATIONS + + debugs(11, 2, HERE << "forwarding 1xx to client"); + + // the Sink will use this to call us back after writing 1xx to the client + typedef NullaryMemFunT CbDialer; + const AsyncCall::Pointer cb = JobCallback(11, 3, CbDialer, this, + HttpStateData::proceedAfter1xx); + CallJobHere1(11, 4, orig_request->clientConnection, ConnStateData, + ConnStateData::sendControlMsg, HttpControlMsg(msg, cb)); + // If the call is not fired, then the Sink is gone, and HttpStateData + // will terminate due to an aborted store entry or another similar error. + // If we get stuck, it is not handle1xx fault if we could get stuck + // for similar reasons without a 1xx response. +} + +/// restores state and resumes processing after 1xx is ignored or forwarded +void +HttpStateData::proceedAfter1xx() +{ + Must(flags.handling1xx); + + debugs(11, 2, HERE << "consuming " << header_bytes_read << + " header and " << reply_bytes_read << " body bytes read after 1xx"); + header_bytes_read = 0; + reply_bytes_read = 0; + + CallJobHere(11, 3, this, HttpStateData, HttpStateData::processReply); +} + + /** * returns true if the peer can support connection pinning */ @@ -1132,6 +1178,20 @@ } } + processReply(); +} + +/// processes the already read and buffered response data, possibly after +/// waiting for asynchronous 1xx control message processing +void +HttpStateData::processReply() { + + if (flags.handling1xx) { // we came back after handling a 1xx response + debugs(11, 5, HERE << "done with 1xx handling"); + flags.handling1xx = false; + Must(!flags.headers_parsed); + } + if (!flags.headers_parsed) { // have not parsed headers yet? PROF_start(HttpStateData_processReplyHeader); processReplyHeader(); @@ -1155,6 +1215,12 @@ bool HttpStateData::continueAfterParsingHeader() { + if (flags.handling1xx) { + debugs(11, 5, HERE << "wait for 1xx handling"); + Must(!flags.headers_parsed); + return false; + } + if (!flags.headers_parsed && !eof) { debugs(11, 9, HERE << "needs more at " << readBuf->contentSize()); flags.do_next_read = 1; === modified file 'src/http.h' --- src/http.h 2010-07-28 18:04:45 +0000 +++ src/http.h 2010-08-30 23:12:32 +0000 @@ -81,6 +81,10 @@ protected: virtual HttpRequest *originalRequest(); + void processReply(); + void proceedAfter1xx(); + void handle1xx(HttpReply *msg); + private: AsyncCall::Pointer closeHandler; enum ConnectionStatus { === modified file 'src/structs.h' --- src/structs.h 2010-08-24 00:02:15 +0000 +++ src/structs.h 2010-08-31 04:38:19 +0000 @@ -393,7 +393,6 @@ #if USE_HTTP_VIOLATIONS int reload_into_ims; - int ignore_expect_100; #endif int offline; @@ -760,6 +759,7 @@ unsigned int proxying:1; unsigned int keepalive:1; unsigned int only_if_cached:1; + unsigned int handling1xx:1; ///< we are ignoring or forwarding 1xx response unsigned int headers_parsed:1; unsigned int front_end_https:2; unsigned int originpeer:1;