From: Amos Jeffries Date: Sat, 11 Oct 2014 19:44:29 +0000 (-0700) Subject: Update RFC 7230 compliance X-Git-Tag: merge-candidate-3-v1~506^2~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a4c74dd8333b5e89065c3dcf807eca4ed4b88941;p=thirdparty%2Fsquid.git Update RFC 7230 compliance * Use status 414 on too-long URLs * Use 431 on too-bit mime headers * Enable the RFC 7230 tolerant parser CRLF / LF waiting for requests. * Documentation updates. --- diff --git a/src/client_side.cc b/src/client_side.cc index 69b915c887..1f0625d7bc 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2166,7 +2166,7 @@ parseHttpRequest(ConnStateData *csd, const Http1::RequestParserPointer &hp) } if (!parsedOk) { - if (hp->request_parse_status == Http::scHeaderTooLarge) + if (hp->request_parse_status == Http::scRequestHeaderFieldsTooLarge || hp->request_parse_status == Http::scUriTooLong) return csd->abortRequestParsing("error:request-too-large"); return csd->abortRequestParsing("error:invalid-request"); @@ -2494,22 +2494,27 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp, 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::scHeaderTooLarge: - repContext->setReplyToError(ERR_TOO_BIG, Http::scBadRequest, method, http->uri, conn->clientConnection->remote, NULL, conn->in.buf.c_str(), NULL); + case Http::scRequestHeaderFieldsTooLarge: + // fall through to next case + case Http::scUriTooLong: + errPage = ERR_TOO_BIG; break; case Http::scMethodNotAllowed: - repContext->setReplyToError(ERR_UNSUP_REQ, Http::scMethodNotAllowed, method, http->uri, - conn->clientConnection->remote, NULL, conn->in.buf.c_str(), NULL); + errPage = ERR_UNSUP_REQ; break; case Http::scHttpVersionNotSupported: - repContext->setReplyToError(ERR_UNSUP_HTTPVERSION, Http::scHttpVersionNotSupported, hp->method(), http->uri, - conn->clientConnection->remote, NULL, conn->in.buf.c_str(), NULL); + errPage = ERR_UNSUP_HTTPVERSION; break; default: - repContext->setReplyToError(ERR_INVALID_REQ, hp->request_parse_status, method, http->uri, - conn->clientConnection->remote, NULL, conn->in.buf.c_str(), NULL); + // 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; diff --git a/src/http/one/RequestParser.cc b/src/http/one/RequestParser.cc index 87bbfdcc81..3226be5c75 100644 --- a/src/http/one/RequestParser.cc +++ b/src/http/one/RequestParser.cc @@ -22,16 +22,11 @@ Http::One::RequestParser::clear() /** * Attempt to parse the first line of a new request message. * - * Governed by RFC 2616 section 4.1 + * Governed by RFC 7230 section 3.5 * " - * In the interest of robustness, servers SHOULD ignore any empty - * line(s) received where a Request-Line is expected. In other words, if - * the server is reading the protocol stream at the beginning of a - * message and receives a CRLF first, it should ignore the CRLF. - * - * ... To restate what is explicitly forbidden by the - * BNF, an HTTP/1.1 client MUST NOT preface or follow a request with an - * extra CRLF. + * In the interest of robustness, a server that is expecting to receive + * and parse a request-line SHOULD ignore at least one empty line (CRLF) + * received prior to the request-line. * " * * Parsing state is stored between calls to avoid repeating buffer scans. @@ -40,7 +35,6 @@ Http::One::RequestParser::clear() void Http::One::RequestParser::skipGarbageLines() { -#if WHEN_RFC_COMPLIANT // CRLF or bare-LF is what RFC 2616 tolerant parsers do ... if (Config.onoff.relaxed_header_parser) { if (Config.onoff.relaxed_header_parser < 0 && (buf_[0] == '\r' || buf_[0] == '\n')) debugs(74, DBG_IMPORTANT, "WARNING: Invalid HTTP Request: " << @@ -52,7 +46,6 @@ Http::One::RequestParser::skipGarbageLines() buf_.consume(1); } } -#endif /* XXX: this is a Squid-specific tolerance * it appears never to have been relevant outside out unit-tests @@ -79,8 +72,7 @@ Http::One::RequestParser::skipGarbageLines() * * Governed by: * RFC 1945 section 5.1 - * RFC 2616 section 5.1 - * RFC 7230 + * RFC 7230 section 3.1 and 3.5 * * Parsing state is stored between calls. However the current implementation * begins parsing from scratch on every call. @@ -148,8 +140,10 @@ Http::One::RequestParser::parseRequestFirstLine() } } - // RFC 2616 section 5.1 - // "No CR or LF is allowed except in the final CRLF sequence" + // RFC 7230 section 3.1.1 does not prohibit embeded CR like RFC 2616 used to. + // However it does explicitly state an exact syntax which omits un-encoded CR + // and defines 400 (Bad Request) as the required action when + // handed an invalid request-line. request_parse_status = Http::scBadRequest; return -1; } @@ -159,8 +153,8 @@ Http::One::RequestParser::parseRequestFirstLine() // DoS protection against long first-line if ((size_t)buf_.length() >= Config.maxRequestHeaderSize) { debugs(33, 5, "Too large request-line"); - // XXX: return URL-too-log status code if second_whitespace is not yet found. - request_parse_status = Http::scHeaderTooLarge; + // RFC 7230 section 3.1.1 mandatory 414 response if URL longer than acceptible. + request_parse_status = Http::scUriTooLong; return -1; } @@ -177,7 +171,7 @@ Http::One::RequestParser::parseRequestFirstLine() // DoS protection against long first-line if ((size_t)(req.end-req.start) >= Config.maxRequestHeaderSize) { debugs(33, 5, "Too large request-line"); - request_parse_status = Http::scHeaderTooLarge; + request_parse_status = Http::scUriTooLong; return -1; } @@ -241,7 +235,7 @@ Http::One::RequestParser::parseRequestFirstLine() req.v_start = last_whitespace + 1; req.v_end = line_end; - /* RFC 2616 section 10.5.6 : handle unsupported HTTP major versions cleanly. */ + /* RFC 7230 section 2.6 : handle unsupported HTTP major versions cleanly. */ if ((req.v_end - req.v_start +1) < (int)Http1magic.length() || !buf_.substr(req.v_start, SBuf::npos).startsWith(Http1magic)) { // non-HTTP/1 protocols not supported / implemented. request_parse_status = Http::scHttpVersionNotSupported; @@ -338,7 +332,7 @@ Http::One::RequestParser::parse(const SBuf &aBuf) if ((mimeHeaderBytes = headersEnd(buf_.c_str(), buf_.length())) == 0) { if (buf_.length()+firstLineSize() >= Config.maxRequestHeaderSize) { debugs(33, 5, "Too large request"); - request_parse_status = Http::scHeaderTooLarge; + request_parse_status = Http::scRequestHeaderFieldsTooLarge; parsingStage_ = HTTP_PARSE_DONE; } else debugs(33, 5, "Incomplete request, waiting for end of headers"); @@ -356,7 +350,7 @@ Http::One::RequestParser::parse(const SBuf &aBuf) // Squid could handle these headers, but admin does not want to if (messageHeaderSize() >= Config.maxRequestHeaderSize) { debugs(33, 5, "Too large request"); - request_parse_status = Http::scHeaderTooLarge; + request_parse_status = Http::scRequestHeaderFieldsTooLarge; return false; } } diff --git a/src/http/one/RequestParser.h b/src/http/one/RequestParser.h index 4481f68210..d4693b391e 100644 --- a/src/http/one/RequestParser.h +++ b/src/http/one/RequestParser.h @@ -41,7 +41,7 @@ private: void skipGarbageLines(); int parseRequestFirstLine(); - /// Offsets for pieces of the (HTTP request) Request-Line as per RFC 2616 + /// Offsets for pieces of the (HTTP request) Request-Line as per RFC 7230 section 3.1.1. /// only valid before and during parse stage HTTP_PARSE_FIRST struct request_offsets { int start, end;