Feedback requested below on a couple of issues... On Mon, Mar 14, 2016 at 11:42 AM, <traw...@apache.org> wrote:
> Author: trawick > Date: Mon Mar 14 15:42:45 2016 > New Revision: 1734947 > > URL: http://svn.apache.org/viewvc?rev=1734947&view=rev > Log: > Add CGIVar directive for configuring REQUEST_URI behavior > > The goal is to use this one directive to handle any configurable > CGI variable behavior; only one CGI variable is supported initially. > > Modified: > httpd/httpd/trunk/CHANGES > httpd/httpd/trunk/docs/manual/mod/core.xml > httpd/httpd/trunk/include/http_core.h > httpd/httpd/trunk/server/core.c > httpd/httpd/trunk/server/util_script.c > > ... > > Modified: httpd/httpd/trunk/include/http_core.h > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=1734947&r1=1734946&r2=1734947&view=diff > > ============================================================================== > --- httpd/httpd/trunk/include/http_core.h (original) > +++ httpd/httpd/trunk/include/http_core.h Mon Mar 14 15:42:45 2016 > @@ -673,6 +673,9 @@ typedef struct { > > unsigned int qualify_redirect_url :2; > ap_expr_info_t *expr_handler; /* forced with SetHandler */ > + > + /** Table of rules for building CGI variables, NULL if none > configured */ > + apr_hash_t *cgi_var_rules; > Any concerns with forcing the module/core/whatever to have to check if the table has been created before seeing if it has a rule for a particular variable? > } core_dir_config; > > /* macro to implement off by default behaviour */ > > Modified: httpd/httpd/trunk/server/core.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1734947&r1=1734946&r2=1734947&view=diff > > ============================================================================== > --- httpd/httpd/trunk/server/core.c (original) > +++ httpd/httpd/trunk/server/core.c Mon Mar 14 15:42:45 2016 > @@ -420,6 +420,15 @@ static void *merge_core_dir_configs(apr_ > > conf->cgi_pass_auth = new->cgi_pass_auth != AP_CGI_PASS_AUTH_UNSET ? > new->cgi_pass_auth : base->cgi_pass_auth; > > + if (new->cgi_var_rules) { > + if (!conf->cgi_var_rules) { > + conf->cgi_var_rules = new->cgi_var_rules; > + } > + else { > + conf->cgi_var_rules = apr_hash_overlay(a, new->cgi_var_rules, > conf->cgi_var_rules); > + } > + } > + > AP_CORE_MERGE_FLAG(qualify_redirect_url, conf, base, new); > > return (void*)conf; > @@ -1872,6 +1881,31 @@ static const char *set_cgi_pass_auth(cmd > return NULL; > } > > +static const char *set_cgi_var(cmd_parms *cmd, void *d_, > + const char *var, const char *rule_) > +{ > + core_dir_config *d = d_; > + char *rule = apr_pstrdup(cmd->pool, rule_); > + > + ap_str_tolower(rule); > + > + if (!strcmp(var, "REQUEST_URI")) { > + if (strcmp(rule, "current-uri") && strcmp(rule, "original-uri")) { > + return "Valid rules for REQUEST_URI are 'current-uri' and > 'original-uri'"; > + } > + } > + else { > + return apr_pstrcat(cmd->pool, "Unrecognized CGI variable: \"", > + var, "\"", NULL); > I can see letting third-party modules use this directive for their own purposes, which requires ignoring variables/rules we don't know about and simply storing them in the table. For the time being this refuses any unexpected variables/rules; that could be loosened up later if desired. But the possibility has some effect on the choice of rule representation -- flexible character string (current) vs. some enumerated value (more efficient). (Aside from letting third-party modules play without custom code in httpd, it also simplifies the httpd implementation to pass through anything. That's what happens essentially with "SetEnvIf Request_URI . proxy-fcgi-pathinfo".) Any thoughts? > + } > + > + if (!d->cgi_var_rules) { > + d->cgi_var_rules = apr_hash_make(cmd->pool); > + } > + apr_hash_set(d->cgi_var_rules, var, APR_HASH_KEY_STRING, rule); > + return NULL; > +} > + > static const char *set_qualify_redirect_url(cmd_parms *cmd, void *d_, int > flag) > { > core_dir_config *d = d_; > @@ -4565,6 +4599,8 @@ AP_INIT_TAKE12("LimitInternalRecursion", > AP_INIT_FLAG("CGIPassAuth", set_cgi_pass_auth, NULL, OR_AUTHCFG, > "Controls whether HTTP authorization headers, normally > hidden, will " > "be passed to scripts"), > +AP_INIT_TAKE2("CGIVar", set_cgi_var, NULL, OR_FILEINFO, > + "Controls how some CGI variables are set"), > AP_INIT_FLAG("QualifyRedirectURL", set_qualify_redirect_url, NULL, > OR_FILEINFO, > "Controls whether the REDIRECT_URL environment variable is > fully " > "qualified"), > > Modified: httpd/httpd/trunk/server/util_script.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_script.c?rev=1734947&r1=1734946&r2=1734947&view=diff > > ============================================================================== > --- httpd/httpd/trunk/server/util_script.c (original) > +++ httpd/httpd/trunk/server/util_script.c Mon Mar 14 15:42:45 2016 > @@ -370,12 +370,25 @@ static char *original_uri(request_rec *r > AP_DECLARE(void) ap_add_cgi_vars(request_rec *r) > { > apr_table_t *e = r->subprocess_env; > + core_dir_config *conf = > + (core_dir_config *)ap_get_core_module_config(r->per_dir_config); > + int request_uri_from_original = 1; > + const char *request_uri_rule; > > apr_table_setn(e, "GATEWAY_INTERFACE", "CGI/1.1"); > apr_table_setn(e, "SERVER_PROTOCOL", r->protocol); > apr_table_setn(e, "REQUEST_METHOD", r->method); > apr_table_setn(e, "QUERY_STRING", r->args ? r->args : ""); > - apr_table_setn(e, "REQUEST_URI", original_uri(r)); > + > + if (conf->cgi_var_rules) { > + request_uri_rule = apr_hash_get(conf->cgi_var_rules, > "REQUEST_URI", > + APR_HASH_KEY_STRING); > + if (request_uri_rule && !strcmp(request_uri_rule, "current-uri")) > { > + request_uri_from_original = 0; > + } > + } > + apr_table_setn(e, "REQUEST_URI", > + request_uri_from_original ? original_uri(r) : r->uri); > ssl_var_lookup("REQUEST_URI") and the expression support for it always use r->uri, which isn't the default behavior for processing r->subprocess_env. (shrug) > > /* Note that the code below special-cases scripts run from includes, > * because it "knows" that the sub_request has been hacked to have the > > > -- Born in Roswell... married an alien... http://emptyhammock.com/
