You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2016/08/18 14:27:35 UTC
Re: svn commit: r1756729 - in /httpd/httpd/trunk:
docs/manual/mod/core.xml include/ap_mmn.h include/http_core.h server/core.c
server/protocol.c
On 08/18/2016 09:15 AM, wrowe@apache.org wrote:
> Author: wrowe
> Date: Thu Aug 18 07:15:06 2016
> New Revision: 1756729
>
> URL: http://svn.apache.org/viewvc?rev=1756729&view=rev
> Log:
> Perform correct, strict parsing of the request line, handling the
> http protocol tag, url and method appropriately, and attempting
> to extract values even in the presence of unusual whitespace in
> keeping with section 3.5, prior to responding with whatever
> error reply is needed. Conforms to RFC7230 in all respects,
> the section 3.5 optional behavior can be disabled by the user
> with a new HttpProtocolOptions StrictWhitespace flag. In all
> cases, the_request is regenerated from the parsed components
> with exactly two space characters.
>
> Shift sf's 'strict' method check from the Strict behavior because
> it violates forward proxy logic, adding a new RegisteredMethods
> flag, as it will certainly be useful to some.
>
>
>
> Modified:
> httpd/httpd/trunk/docs/manual/mod/core.xml
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/http_core.h
> httpd/httpd/trunk/server/core.c
> httpd/httpd/trunk/server/protocol.c
>
> Modified: httpd/httpd/trunk/include/http_core.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=1756729&r1=1756728&r2=1756729&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/http_core.h (original)
> +++ httpd/httpd/trunk/include/http_core.h Thu Aug 18 07:15:06 2016
> @@ -739,6 +739,16 @@ typedef struct {
> #define AP_HTTP_CONFORMANCE_STRICT 2
> char http_conformance;
>
> +#define AP_HTTP_WHITESPACE_UNSET 0
> +#define AP_HTTP_WHITESPACE_LENIENT 1
> +#define AP_HTTP_WHITESPACE_STRICT 2
> + char http_whitespace;
> +
> +#define AP_HTTP_METHODS_UNSET 0
> +#define AP_HTTP_METHODS_LENIENT 1
> +#define AP_HTTP_METHODS_REGISTERED 2
> + char http_methods;
> +
> #define AP_HTTP_CL_HEAD_ZERO_UNSET 0
> #define AP_HTTP_CL_HEAD_ZERO_ENABLE 1
> #define AP_HTTP_CL_HEAD_ZERO_DISABLE 2
Any particular reason, why this was implemented in a way that requires a major bump and thus is not backportable?
>
> Modified: httpd/httpd/trunk/server/protocol.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/protocol.c?rev=1756729&r1=1756728&r2=1756729&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/protocol.c (original)
> +++ httpd/httpd/trunk/server/protocol.c Thu Aug 18 07:15:06 2016
> @@ -559,18 +559,32 @@ AP_CORE_DECLARE(void) ap_parse_uri(reque
> }
> }
>
> -static int read_request_line(request_rec *r, apr_bucket_brigade *bb)
> +/* get the length of the field name for logging, but no more than 80 bytes */
> +#define LOG_NAME_MAX_LEN 80
> +static int field_name_len(const char *field)
> {
> - const char *ll;
> - const char *uri;
> - const char *pro;
> + const char *end = ap_strchr_c(field, ':');
> + if (end == NULL || end - field > LOG_NAME_MAX_LEN)
> + return LOG_NAME_MAX_LEN;
> + return end - field;
> +}
>
> +static int read_request_line(request_rec *r, apr_bucket_brigade *bb)
> +{
> + enum {
> + rrl_none, rrl_badmethod, rrl_badwhitespace, rrl_excesswhitespace,
> + rrl_missinguri, rrl_badprotocol, rrl_trailingtext
> + } deferred_error = rrl_none;
> + char *ll;
> + char *uri;
> unsigned int major = 1, minor = 0; /* Assume HTTP/1.0 if non-"HTTP" protocol */
> char http[5];
> apr_size_t len;
> int num_blank_lines = DEFAULT_LIMIT_BLANK_LINES;
> core_server_config *conf = ap_get_core_module_config(r->server->module_config);
> int strict = (conf->http_conformance != AP_HTTP_CONFORMANCE_UNSAFE);
> + const char *badwhitespace = strict ? "\t\n\v\f\r" : "\n\v\f\r";
> + int strictspaces = (conf->http_whitespace == AP_HTTP_WHITESPACE_STRICT);
>
> /* Read past empty lines until we get a real request line,
> * a read error, the connection closes (EOF), or we timeout.
> @@ -626,111 +640,211 @@ static int read_request_line(request_rec
> }
>
> r->request_time = apr_time_now();
> - ll = r->the_request;
> - r->method = ap_getword_white(r->pool, &ll);
>
> - uri = ap_getword_white(r->pool, &ll);
> + r->method = r->the_request;
> + if (apr_isspace(*r->method))
> + deferred_error = rrl_excesswhitespace;
> + for ( ; apr_isspace(*r->method); ++r->method)
> + ;
>
> - if (!*r->method || !*uri) {
> - r->status = HTTP_BAD_REQUEST;
> - r->proto_num = HTTP_VERSION(1,0);
> - r->protocol = "HTTP/1.0";
> - return 0;
> + if (strict) {
> + ll = (char*) ap_scan_http_token(r->method);
> + if ((ll == r->method) || (*ll && !apr_isspace(*ll))) {
> + deferred_error = rrl_badmethod;
> + ll = strpbrk(ll, "\t\n\v\f\r ");
> + }
> }
> -
> - /* Provide quick information about the request method as soon as known */
> -
> - r->method_number = ap_method_number_of(r->method);
> - if (r->method_number == M_GET && r->method[0] == 'H') {
> - r->header_only = 1;
> + else {
> + ll = strpbrk(r->method, "\t\n\v\f\r ");
> + }
> + if (!ll) {
> + if (deferred_error == rrl_none)
> + deferred_error = rrl_missinguri;
> + r->protocol = uri = "";
> + len = 0;
> + goto rrl_done;
> }
>
> - ap_parse_uri(r, uri);
> - if (r->status != HTTP_OK) {
> - r->proto_num = HTTP_VERSION(1,0);
> - r->protocol = "HTTP/1.0";
> - return 0;
> + if (strictspaces && ll[0] && (ll[0] != ' ' || apr_isspace(ll[1]))
> + && deferred_error == rrl_none) {
> + deferred_error = rrl_excesswhitespace;
> + }
> + for (uri = ll; apr_isspace(*uri); ++uri)
> + if (strchr(badwhitespace, *uri) && deferred_error == rrl_none)
> + deferred_error = rrl_badwhitespace;
> + *ll = '\0';
> + if (!*uri && deferred_error == rrl_none)
> + deferred_error = rrl_missinguri;
> +
> + if (!(ll = strpbrk(uri, " \t\n\v\f\r"))) {
> + r->protocol = "";
> + len = 0;
> + goto rrl_done;
> + }
> + for (r->protocol = ll; apr_isspace(*r->protocol); ++r->protocol)
> + if (strchr(badwhitespace, *r->protocol) && deferred_error == rrl_none
> + && deferred_error == rrl_none)
Double deferred_error == rrl_none ?
Regards
R�diger