Re: Patch for implementing ap_document_root as a hook
On Apr 23, 2007, at 10:46 AM, Brian J. France wrote: On Apr 23, 2007, at 10:32 AM, Jakob Goldbach wrote: -1 on the face of things. The map_to_storage hook was added to accomplish what you desire. I thought map_to_storage was made to do per-dir configuration. Not path-translation. The problem is not really doing the translation. I can always provide my own translate handler in my module. But in the current API I cannot to set my env. variables at will. They will be overwritten by ap_add_common_vars which returns carved-in-stone docroot from ap_document_root. We need this same functionality (would like to back port to 2.2 if possible). I'm +1 on the concept but -0 on the provided method of doing it... We currently hack the doc root in the post read hook in 1.3, would like to be able to do it with out hacking the core and screwing around with internal structs at runtime. VERY doubtful that 1.3 will be updated to do this.
Re: Patch for implementing ap_document_root as a hook
On Apr 26, 2007, at 8:35 AM, Jim Jagielski wrote: We currently hack the doc root in the post read hook in 1.3, would like to be able to do it with out hacking the core and screwing around with internal structs at runtime. VERY doubtful that 1.3 will be updated to do this. I don't need it in 1.3, but I would like to have a clean way to do it in 2.x that doesn't include replacing data in the internal structs at runtime and putting it back at the end of the request. Brian
Re: RFC: replace r-subprocess_env was Re: Patch for implementing ap_document_root as a hook
Thoughts?
I like it.
I prefer this generel env. API rather than making
ap_add_{common,cgi}_vars hook'able.
/Jakob
Patch for implementing ap_document_root as a hook
Hi,
Attached is a patch which implements ap_document_root(r) as a hook.
This way modules can set document_root on the fly. (think vhost_alias)
AND get the right DOCUMENT_ROOT env. variable (as set by
ap_add_common_vars(r)).
The patch also changes ap_core_translate to use ap_document_root(r)
instead of conf-ap_document_root. This way, modules that just need
to point to a different docroot won't have to implement a translate
hook by appending r-uri til r-filename, but just rely on
ap_document_root.
Comments ?
/Jakob
Index: server/core.c
===
--- server/core.c (revision 530676)
+++ server/core.c (working copy)
@@ -69,12 +69,16 @@
APR_HOOK_STRUCT(
APR_HOOK_LINK(get_mgmt_items)
+APR_HOOK_LINK(document_root)
)
AP_IMPLEMENT_HOOK_RUN_ALL(int, get_mgmt_items,
(apr_pool_t *p, const char *val, apr_hash_t *ht),
(p, val, ht), OK, DECLINED)
+AP_IMPLEMENT_HOOK_RUN_FIRST(const char *,document_root,
+ (request_rec *r), (r), NULL)
+
/* Server core module... This module provides support for really basic
* server operations, including options and commands which control the
* operation of other modules. Consider this the bureaucracy module.
@@ -682,7 +686,7 @@
: DEFAULT_CONTENT_TYPE;
}
-AP_DECLARE(const char *) ap_document_root(request_rec *r) /* Don't use this! */
+AP_DECLARE(const char *) ap_core_document_root(request_rec *r)
{
core_server_config *conf;
@@ -3390,7 +3394,7 @@
while (*path == '/') {
++path;
}
-if ((rv = apr_filepath_merge(r-filename, conf-ap_document_root, path,
+if ((rv = apr_filepath_merge(r-filename, ap_document_root(r), path,
APR_FILEPATH_TRUENAME
| APR_FILEPATH_SECUREROOT, r-pool))
!= APR_SUCCESS) {
@@ -3413,7 +3417,7 @@
while (*path == '/') {
++path;
}
-if ((rv = apr_filepath_merge(r-filename, conf-ap_document_root, path,
+if ((rv = apr_filepath_merge(r-filename, ap_document_root(r), path,
APR_FILEPATH_TRUENAME
| APR_FILEPATH_SECUREROOT, r-pool))
!= APR_SUCCESS) {
@@ -3863,6 +3867,7 @@
APR_OPTIONAL_HOOK(proxy, create_req, core_create_proxy_req, NULL, NULL,
APR_HOOK_MIDDLE);
ap_hook_pre_mpm(ap_create_scoreboard, NULL, NULL, APR_HOOK_MIDDLE);
+ap_hook_document_root(ap_core_document_root, NULL, NULL, APR_HOOK_REALLY_LAST);
/* register the core's insert_filter hook and register core-provided
* filters
Index: include/http_core.h
===
--- include/http_core.h (revision 530676)
+++ include/http_core.h (working copy)
@@ -148,11 +148,10 @@
/**
* Retrieve the document root for this server
* @param r The current request
- * @warning Don't use this! If your request went through a Userdir, or
- * something like that, it'll screw you. But it's back-compatible...
* @return The document root
*/
-AP_DECLARE(const char *) ap_document_root(request_rec *r);
+AP_DECLARE(const char *) ap_core_document_root(request_rec *r);
+#define ap_document_root(r) ap_run_document_root(r)
/**
* Lookup the remote client's DNS name or IP address
@@ -629,6 +628,14 @@
AP_DECLARE_HOOK(int, get_mgmt_items,
(apr_pool_t *p, const char * val, apr_hash_t *ht))
+/**
+ * This hook allows modules to return customized documentroot
+ * @param r the current request
+ * @ingroup hooks
+ */
+AP_DECLARE_HOOK(const char*,document_root,(request_rec *r))
+
+
/* -- */
/* --
Re: Patch for implementing ap_document_root as a hook
It is now in bugzilla as #42192 /Jakob
Re: Patch for implementing ap_document_root as a hook
Jakob Goldbach wrote: Hi, Attached is a patch which implements ap_document_root(r) as a hook. This way modules can set document_root on the fly. (think vhost_alias) AND get the right DOCUMENT_ROOT env. variable (as set by ap_add_common_vars(r)). The patch also changes ap_core_translate to use ap_document_root(r) instead of conf-ap_document_root. This way, modules that just need to point to a different docroot won't have to implement a translate hook by appending r-uri til r-filename, but just rely on ap_document_root. -1 on the face of things. The map_to_storage hook was added to accomplish what you desire. Unfortunately it is not coupled to the DOCUMENT_ROOT variable, but I'd look at remedying this over building on ap_document_root, which should simply go away, IMHO.
Re: Patch for implementing ap_document_root as a hook
-1 on the face of things. The map_to_storage hook was added to accomplish what you desire. I thought map_to_storage was made to do per-dir configuration. Not path-translation. The problem is not really doing the translation. I can always provide my own translate handler in my module. But in the current API I cannot to set my env. variables at will. They will be overwritten by ap_add_common_vars which returns carved-in-stone docroot from ap_document_root. My only other option is to patch every single module which calls add_common_vars, that is, cgi,cgid, fastcgi, includes,... I thought a document_root hook was more elegant. Or maybe a add_common_vars hook? [I would be happy to supply it] Unfortunately it is not coupled to the DOCUMENT_ROOT variable, but I'd look at remedying this over building on ap_document_root, which should simply go away, IMHO. What's so bad about ap_document_root? I know the source says 'dont use' because it won't be right with mod_userdir etc. But with a hook it would be possible to get right. /Jakob
Re: Patch for implementing ap_document_root as a hook
On Apr 23, 2007, at 10:32 AM, Jakob Goldbach wrote: -1 on the face of things. The map_to_storage hook was added to accomplish what you desire. I thought map_to_storage was made to do per-dir configuration. Not path-translation. The problem is not really doing the translation. I can always provide my own translate handler in my module. But in the current API I cannot to set my env. variables at will. They will be overwritten by ap_add_common_vars which returns carved-in-stone docroot from ap_document_root. We need this same functionality (would like to back port to 2.2 if possible). We currently hack the doc root in the post read hook in 1.3, would like to be able to do it with out hacking the core and screwing around with internal structs at runtime. Brian My only other option is to patch every single module which calls add_common_vars, that is, cgi,cgid, fastcgi, includes,... I thought a document_root hook was more elegant. Or maybe a add_common_vars hook? [I would be happy to supply it] Unfortunately it is not coupled to the DOCUMENT_ROOT variable, but I'd look at remedying this over building on ap_document_root, which should simply go away, IMHO. What's so bad about ap_document_root? I know the source says 'dont use' because it won't be right with mod_userdir etc. But with a hook it would be possible to get right. /Jakob
Re: Patch for implementing ap_document_root as a hook
Brian J. France wrote: On Apr 23, 2007, at 10:32 AM, Jakob Goldbach wrote: -1 on the face of things. The map_to_storage hook was added to accomplish what you desire. I thought map_to_storage was made to do per-dir configuration. Not path-translation. The problem is not really doing the translation. I can always provide my own translate handler in my module. But in the current API I cannot to set my env. variables at will. They will be overwritten by ap_add_common_vars which returns carved-in-stone docroot from ap_document_root. We need this same functionality (would like to back port to 2.2 if possible). We currently hack the doc root in the post read hook in 1.3, would like to be able to do it with out hacking the core and screwing around with internal structs at runtime. +1, I've been down this road before too. The problem is that many applications use DOCUMENT_ROOT. I kinda think that they shouldn't, because the entire concept of an document root isn't very valid with Alias / Rewrite rules changing the path, but it doesn't change that people are using it. Either we need to change ap_add_common_vars to allow overrides easier (and hopefully in a more generic way, for all environment variables), or we need to truly make it pluggable, like the path that this patch is going down. I do have issues with how this patch does it, but its general direction isn't wrong. -Paul
Re: Patch for implementing ap_document_root as a hook
On 4/23/07 11:33 AM, Paul Querna [EMAIL PROTECTED] wrote: +1, I've been down this road before too. +1 on the concept. Still looking at patch. -- Brian Akins Chief Operations Engineer Turner Digital Media Technologies
Re: Patch for implementing ap_document_root as a hook
Brian J. France wrote: On Apr 23, 2007, at 10:32 AM, Jakob Goldbach wrote: -1 on the face of things. The map_to_storage hook was added to accomplish what you desire. I thought map_to_storage was made to do per-dir configuration. Not path-translation. Actually it does both. An amusing use case is The problem is not really doing the translation. I can always provide my own translate handler in my module. But in the current API I cannot to set my env. variables at will. They will be overwritten by ap_add_common_vars which returns carved-in-stone docroot from ap_document_root. We need this same functionality (would like to back port to 2.2 if possible). We currently hack the doc root in the post read hook in 1.3, would like to be able to do it with out hacking the core and screwing around with internal structs at runtime. I don't quite see this happening, but maybe. If the docroot could be cached from the translate_name/map_to_storage phase, you could obviously abuse it as necessary. I thought a document_root hook was more elegant. Or maybe a add_common_vars hook? [I would be happy to supply it] ***ding ding ding*** - hooking add_common_vars is something I could get behind. What's so bad about ap_document_root? I know the source says 'dont use' because it won't be right with mod_userdir etc. But with a hook it would be possible to get right. What is the docroot of http://example.com/user/webapp/login - is it the path to /user/webapp/ or somewhere in the admin's htdoc? I find the concept fundamentally flawed :)
RFC: replace r-subprocess_env was Re: Patch for implementing ap_document_root as a hook
This idea has been rattling around in my head off and on for a while. What is we replaced all the r-subprocess_env with something a little more interesting... General environment API: /* directly set an env variable. Will always show up in env list */ apr_status_t ap_set_env(request_rec *r, const char *key, const char *val) /* Get the value of an env var. */ const char *ap_get_env(request_rec *r, const char *key And the interesting ones: /* Set a handler for a given key for env variables. Can choose whether or not the key shows up in the list. */ apr_status_t ap_set_env_handler(constchar *key, ap_env_func *func, int show_in_list) /* Return a list of available (exposed) env variables suitable for iteration */ apr_array_header_t *ap_env_list( request_rec *r, const char *key) ap_env_func would be : const char* my_env_handler(request_rec*r, const char* key) This would allow most env variables to be overridden easily. Also, many env variables could be set lazily, ie, only calculate it when someone actually needs it. A good example of this is when you occasionally use UNIQUE_ID and only want the calculation to be done when you actually need it, not on every single request. The handler could cache it's results if it wanted. We may want a flag to say cache is okay or not. Or do caching in env itself... The actually handler, could actually be a hook. For example, the handler for DOCUMENT_ROOT could actually be a wrapper around a hook. Thoughts? -- Brian Akins Chief Operations Engineer Turner Digital Media Technologies
Re: Patch for implementing ap_document_root as a hook
William A. Rowe, Jr. wrote: Brian J. France wrote: On Apr 23, 2007, at 10:32 AM, Jakob Goldbach wrote: -1 on the face of things. The map_to_storage hook was added to accomplish what you desire. I thought map_to_storage was made to do per-dir configuration. Not path-translation. Actually it does both. An amusing use case is ...not the one I was thinking of, this one is more fun... jump to the ftp_cmd_rnto(...) implementation in http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c?revision=522706view=markup
