On Aug 16, 2016 4:39 PM, "Yann Ylavic" <ylavic....@gmail.com> wrote: > > On Tue, Aug 16, 2016 at 8:11 PM, <wr...@apache.org> wrote: > > Author: wrowe > > Date: Tue Aug 16 18:11:14 2016 > > New Revision: 1756540 > > > > URL: http://svn.apache.org/viewvc?rev=1756540&view=rev > > Log: > > Rename the previously undocumented HTTPProtocol directive > > to EnforceHTTPProtocol, and invert the default behavior > > to strictly observe RFC 7230 unless otherwise configured. > > And Document This. > > > > The relaxation option is renamed 'Unsafe'. 'Strict' is no > > longer case sensitive. 'min=0.9|1.0' is now the verbose > > 'Allow0.9' or 'Require1.0' case-insenstive grammer. The > > exclusivity tests have been modified to detect conflicts. > > > > The 'strict,log' option failed to enforce strict conformance, > > and has been removed. Unsafe, informational logging is possible > > in any loadable module, after the request data is unsafely > > accepted. > > This probably requires a MMN bump (possibly major w.r.t trunk).
Agreed, I'll note it. Was thinking this was an internal config but it is in a published header, as you note. > > Modified: > > httpd/httpd/trunk/docs/manual/mod/core.xml > > httpd/httpd/trunk/modules/http/http_filters.c > > httpd/httpd/trunk/server/core.c > > httpd/httpd/trunk/server/protocol.c > > httpd/httpd/trunk/server/vhost.c > > Looks like changes to include/http_core.h are missing, at least > AP_HTTP_CONFORMANCE_UNSAFE is undefined for now. Will fix that along with MMN. > > Modified: httpd/httpd/trunk/server/core.c > > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1756540&r1=1756539&r2=1756540&view=diff > > ============================================================================== > > --- httpd/httpd/trunk/server/core.c (original) > > +++ httpd/httpd/trunk/server/core.c Tue Aug 16 18:11:14 2016 > > @@ -4011,37 +4011,39 @@ static const char *set_protocols_honor_o > [] > > +static const char *set_enforce_http_protocol(cmd_parms *cmd, void *dummy, > > + const char *arg) > > { > [] > > + if ((conf->http09_enable & AP_HTTP09_ENABLE) && > > + (conf->http09_enable & AP_HTTP09_DISABLE)) { > > + return "EnforceHttpProtocol 'Allow0.9' and 'Require1.0'" > > + " are mutually exclusive"; > > + } > > So why define two separate keywords for those? > Wouldn't "Forbid0.9" be simpler? IMO, no. I anticipated we might someday add Require1.1 as a third alternative, as you alluded to below. > > -AP_INIT_ITERATE("HttpProtocol", set_http_protocol, NULL, RSRC_CONF, > > - "'min=0.9' (default) or 'min=1.0' to allow/deny HTTP/0.9; " > > - "'liberal', 'strict', 'strict,log-only'"), > > +AP_INIT_ITERATE("EnforceHttpProtocol", set_enforce_http_protocol, NULL, RSRC_CONF, > > + "'Allow0.9' or 'Require1.0' (default) to allow or deny HTTP/0.9; " > > + "'Unsafe' or 'Strict' (default) to process incorrect requests"), > > The original min= was naturally mutually exclusive, and the original > <option>= looks more extensible (to me)... It appeared that both exclusivity tests were borked because the value was first set, ergo the old bit was unset, and then a meaningless test occurred (doh). Allow0.9 or Require1.0 is more readily apparent in perusing someone else's config files. We have too many uses of generic (and case sensitive? Ick) keywords that makes it harder to spot misconfiguration. Do you really want to grep for the token and the directive name, or an arbitrary search for min= which has a number of contextual meanings? > Also, since this directive will mainly be used to relax HTTP protocol > (wrt RFC 7230) for compatibility reasons, EnforceHttpProtocol may not > be appropriate. > > How about HttpProtocolPolicy [Strict|Unsafe] min=[0.9|1.0|1.1] ? I'm open to suggestions, what about HTTPProtocolOptions (option list)? > > Modified: httpd/httpd/trunk/server/protocol.c > > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/protocol.c?rev=1756540&r1=1756539&r2=1756540&view=diff > > ============================================================================== > > --- httpd/httpd/trunk/server/protocol.c (original) > > +++ httpd/httpd/trunk/server/protocol.c Tue Aug 16 18:11:14 2016 > > @@ -687,13 +686,11 @@ static int read_request_line(request_rec > > if (strict) { > > ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02418) > > "Invalid protocol '%s'", r->protocol); > > - if (enforce_strict) { > > - r->proto_num = HTTP_VERSION(1,0); > > - r->protocol = "HTTP/1.0"; > > - r->connection->keepalive = AP_CONN_CLOSE; > > - r->status = HTTP_BAD_REQUEST; > > - return 0; > > - } > > + r->proto_num = HTTP_VERSION(1,0); > > + r->protocol = "HTTP/1.0"; > > + r->connection->keepalive = AP_CONN_CLOSE; > > + r->status = HTTP_BAD_REQUEST; > > + return 0; > > } > > if (3 == sscanf(r->protocol, "%4s/%u.%u", http, &major, &minor) > > && (ap_cstr_casecmp("http", http) == 0) > > While at it, "HTTP" (caps) looks a good strict candidate here :) Yup, agreed. The request line logic is what I've been working on today. Also to be added today or Wednesday, a distinct HTTPProtocolOption to enforce SP/HTAB, or relax for other whitespace as noted in 7230 s3.5, but substituting SP for added safety.
