Bug 28903

Summary: Hooks to add environment variables to CGI and other scripted content handlers
Product: Apache httpd-2 Reporter: Jim Schneider <jschneid>
Component: CoreAssignee: Apache HTTPD Bugs Mailing List <bugs>
Severity: enhancement Keywords: MassUpdate
Priority: P3    
Version: 2.0-HEAD   
Target Milestone: ---   
Hardware: All   
OS: All   

Description Jim Schneider 2004-05-11 16:51:39 UTC
There is currently no way to override the environment variables set by
ap_set_common_vars or ap_set_cgi_vars.  The following patch introduces a pair of
hook functions (add_vars and add_cgi_vars) that can be used to add new
enviroment variables, or override or remove existing environment variables.  The
hook functions take an apr_table_t * as their only parameter, which is the
pointer to the subprocess_env member of the request record.

--<cut here>--
Index: include/http_request.h
diff -U3 -r1.1.1.2 http_request.h
--- include/http_request.h      2003/04/10 19:09:56
+++ include/http_request.h      2004/05/11 16:28:04
@@ -404,6 +404,18 @@
 AP_DECLARE_HOOK(void,insert_filter,(request_rec *r))

+ * This hook allows modules to add environment variables during
+ * ap_add_common_vars()
+ */
+AP_DECLARE_HOOK(void,add_vars,(apr_table_t *t))
+ * This hook allows modules to add environment variables during
+ * ap_add_cgi_vars()
+ */
+AP_DECLARE_HOOK(void,add_cgi_vars,(apr_table_t *t))
 AP_DECLARE(int) ap_location_walk(request_rec *r);
 AP_DECLARE(int) ap_directory_walk(request_rec *r);
 AP_DECLARE(int) ap_file_walk(request_rec *r);
Index: server/util_script.c
diff -U3 -r1.1.1.2 util_script.c
--- server/util_script.c        2003/04/10 19:09:56
+++ server/util_script.c        2004/05/11 16:28:04
@@ -85,6 +85,16 @@
 #include <os2.h>

+/* Hook structure for add_vars and add_cgi_vars */
+               APR_HOOK_LINK(add_vars)
+               APR_HOOK_LINK(add_cgi_vars)
+/* implement ap_run_add_vars and ap_run_add_cgi_vars */
+AP_IMPLEMENT_HOOK_VOID(add_vars, (apr_table_t *t), (t))
+AP_IMPLEMENT_HOOK_VOID(add_cgi_vars, (apr_table_t *t), (t))
  * Various utility functions which are common to a whole lot of
  * script-type extensions mechanisms, and might as well be gathered
@@ -305,6 +315,7 @@

+    ap_run_add_vars(e);
     if (e != r->subprocess_env) {
       apr_table_overlap(r->subprocess_env, e, APR_OVERLAP_TABLES_SET);
@@ -416,6 +427,7 @@
+    ap_run_add_cgi_vars(e);
Comment 1 André Malo 2004-05-11 17:01:25 UTC
Hmm. What is the advantage compared to fixup, which is, what mod_env uses (where
you can take all these actions, except for HTTP_ stuff, which should not be
modified anyway [CGI/1.1 spec])?
Comment 2 Jim Schneider 2004-05-11 17:31:14 UTC
Actually, ap_set_common_vars happens after ap_run_fixups, so anything done in
the fixups phase can be undone in ap_set_common_vars (or ap_set_cgi_vars).  If
you need to modify the variables set in either of these places, you're out of luck.

As for not modifying the HTTP_.* and other CGI variables, there are cases where
it makes sense to modify them (for example, proxied Apache instances, or cases
where network transport is handled by another entity).  If I'm able to change
what's in the SERVER_ADDR and SERVER_PORT fields (for example), I can use a
particular badly-written CGI program that creates its own self-referential URLs
when I move to a proxied scheme (no, fixing the CGI program isn't really an
option - it's a compiled C++ program provided by a third party).

Don't get me wrong, I'm not saying we should abandon any specs here, but I don't
think the mere existance of a specification should prohibit adding tools that
_may_ be used to violate it.
Comment 3 André Malo 2004-05-11 19:10:51 UTC

I still see a couple of issues:

- why do we need *two* hooks? as far as I can see, one would be enough (after
all variables are set)
- in such a general function that is called from arbitrary pieces of code, the
hook would be misplaced.

If you want to modify variables that are inherited to the CGI, I think a better
place for that hook would be in mod_cgi itself (And with r as parameter, since
you need to get the information from somewhere). Such as "ap_run_fixup_cgi_env".

How does this sound?
Comment 4 Jim Schneider 2004-05-11 19:47:19 UTC
I provided two hooks because sometimes both ap_add_common_vars and
ap_add_cgi_vars are called, and sometimes only ap_add_common_vars is called, and
I figured it would be more general to have both hooks available.

I'm calling the hooks in ap_add_common_vars and ap_add_cgi_vars because that's
where the variables are set.  Calling them elsewhere seems more error-prone -
you need to find all occurences of ap_add_common_vars or ap_add_cgi_vars, trace
the execution path, and install the hooks right before the variables get used.

Also, I'm passing a pointer to a table because ap_add_common_vars overwrites
whatever is in the subprocess_env table.  I'm not averse to passing the
request_rec as an additional parameter, but the apr_table_t needs to be passed,
too, or it won't work in ap_add_common_vars.
Comment 5 André Malo 2004-05-11 20:37:51 UTC
I disagree.

The problems needs to be nailed down where the hole is, not where the hammer is.
So what do you ant to achieve? A modification of the environment passed to a
CGI. mod_cgi should be changed only then.

Adding hooks to a basic *utility* function is not an option. That is really
error prone, because you're starting to influence code that you cannot control.

Finally, one hook with (r) is enough. You want to modify r->subprocess_env after
it was filled in, nothing else. How it was built before is totally the same to
the hook function.

Other opinions would be appreciated here.
Comment 6 Geoffrey Young 2004-05-12 00:00:59 UTC
the simple way would be that if a flag is set (configuration directive or
whatnot) and if the variable is already defined in subprocess_env, use the
existing value instead.  the directive could look like

  AllowEnvironmentOverride On

you could also do 
  AllowEnvironmentOverride REMOTE_USER

or whatnot, but the ability to use mod_env or other means to set subprocess_env
usually comes with the ability to edit httpd.conf as well.
Comment 7 Jim Schneider 2004-05-12 12:56:01 UTC
That's a great idea.  I'll work on it this morning.  Any preference as to doing
this as a standalone module, or tying it into a standard module?
Comment 8 William A. Rowe Jr. 2018-11-07 21:09:27 UTC
Please help us to refine our list of open and current defects; this is a mass update of old and inactive Bugzilla reports which reflect user error, already resolved defects, and still-existing defects in httpd.

As repeatedly announced, the Apache HTTP Server Project has discontinued all development and patch review of the 2.2.x series of releases. The final release 2.2.34 was published in July 2017, and no further evaluation of bug reports or security risks will be considered or published for 2.2.x releases. All reports older than 2.4.x have been updated to status RESOLVED/LATER; no further action is expected unless the report still applies to a current version of httpd.

If your report represented a question or confusion about how to use an httpd feature, an unexpected server behavior, problems building or installing httpd, or working with an external component (a third party module, browser etc.) we ask you to start by bringing your question to the User Support and Discussion mailing list, see [https://httpd.apache.org/lists.html#http-users] for details. Include a link to this Bugzilla report for completeness with your question.

If your report was clearly a defect in httpd or a feature request, we ask that you retest using a modern httpd release (2.4.33 or later) released in the past year. If it can be reproduced, please reopen this bug and change the Version field above to the httpd version you have reconfirmed with.

Your help in identifying defects or enhancements still applicable to the current httpd server software release is greatly appreciated.