You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ondřej Surý <on...@sury.org> on 2010/07/06 10:50:16 UTC
Per request DocumentRoot
Hi,
I am a author of mod-vhost-ldap module and I have received complaints
about DocumentRoot being unstable. After a fixing of dozen bugs and
further debugging I have found out, that DocumentRoot is shared per
process and not per thread and thus I am unable to fix that as a
self-contained fix, since ap_document_root gets overwritten under a
heavy load.
I have found some suggestions to replace ap_document_root(request_rec
r) with a hook, but I think that much simpler and elegant solution is
to implement DocumentRoot as per request and initialize it every time
new request is created. This solution also means that apache2 stays
ABI compatible.
What is the general feeling of this list? Would such patch be
accepted? (Ie. should I bother with creating bug report with patch)
Ondrej
P.S.: I know that ap_document_root(r) should not be used, but the
reality disagrees. Also ap_document_root(request rec r) is used by
some modules maintained within tree (most notable mod_rewrite).
--
Ondřej Surý <on...@sury.org>
http://blog.rfc1925.org/
Re: Per request DocumentRoot
Posted by Graham Leggett <mi...@sharp.fm>.
On 06 Jun 2011, at 10:59 PM, Stefan Fritsch wrote:
> These should not be changed for now. The core only translates relative
> to the "real" document root. Everything else is done by modules.
>
> If we ever allow to set the DocumentRoot inside <Location> blocks,
> this may need to be re-evaluated.
That might be nice to have, you could avoid having to Alias everything.
Regards,
Graham
--
Re: Per request DocumentRoot
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Monday 06 June 2011, Ondřej Surý wrote:
> Looks good to me, although I think you might have missed few lines
> where conf->ap_document_root is referenced directly in
> core.c:ap_core_translate(request_rec *r):
> But I don't understand the apache internals so well to tell whether
> it's not a false positive.
These should not be changed for now. The core only translates relative
to the "real" document root. Everything else is done by modules.
If we ever allow to set the DocumentRoot inside <Location> blocks,
this may need to be re-evaluated.
Re: Per request DocumentRoot
Posted by Ondřej Surý <on...@sury.org>.
On Sun, Jun 5, 2011 at 23:44, Stefan Fritsch <sf...@sfritsch.de> wrote:
> On Tuesday 17 May 2011, Ondřej Surý wrote:
>> this is the semi-annual ping to check whether I can find enough
>> support to get per request r->document_root implemented in 2.3.x.
>
> Something like that is now implemented in trunk in
>
> http://svn.apache.org/viewvc?view=revision&revision=1132494
>
> Comments welcome.
Looks good to me, although I think you might have missed few lines
where conf->ap_document_root is referenced directly in
core.c:ap_core_translate(request_rec *r):
https://issues.apache.org/bugzilla/attachment.cgi?id=25843&action=diff#server/core.c_sec2
https://issues.apache.org/bugzilla/attachment.cgi?id=25843&action=diff#server/core.c_sec3
But I don't understand the apache internals so well to tell whether
it's not a false positive.
O.
--
Ondřej Surý <on...@sury.org>
Re: Per request DocumentRoot
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tuesday 17 May 2011, Ondřej Surý wrote:
> this is the semi-annual ping to check whether I can find enough
> support to get per request r->document_root implemented in 2.3.x.
Something like that is now implemented in trunk in
http://svn.apache.org/viewvc?view=revision&revision=1132494
Comments welcome.
Cheers,
Stefan
Re: Per request DocumentRoot
Posted by Ondřej Surý <on...@sury.org>.
Hi,
this is the semi-annual ping to check whether I can find enough
support to get per request r->document_root implemented in 2.3.x.
The patch is attached at
https://issues.apache.org/bugzilla/show_bug.cgi?id=49705
And I am willing to update it to current 2.3.x if there is enough
interest. Or implement it in some another way. Or <something>. It is
really a blocker if users of my mod-vhost-ldap (and any other vhost
modules) want to rewrite document root (for PHP f.e.).
O.
On Wed, Oct 27, 2010 at 09:57, Ondřej Surý <on...@sury.org> wrote:
> Hi,
>
> I would like to raise this issue again. The proposed solution by
> Anders Kaseorg:
>
>> FWIW, when I ran into this problem in mod-vhost-ldap, I fixed it by
>> making a copy of the entire server_rec structure in the request pool.
>
> doesn't really work with mod_rewrite, because mod_rewrite contains this code:
>
> if (conf->server != r->server) {
> return DECLINED;
> }
>
> in the hook_uri2file (translate_name hook).
>
> which prevents mod_rewrite to run if server_rec structure is modified
> before mod_rewrite. However all virtual document root modules need to
> be run before mod_rewrite for -f and -d to work.
>
> As far as I can see, the moving the document_root to request_rec is
> only viable solution in the long run for virtualization modules.
>
> Ondrej
>
> On Thu, Aug 5, 2010 at 11:53, Ondřej Surý <on...@sury.org> wrote:
>> On Thu, Aug 5, 2010 at 10:25, William A. Rowe Jr. <wr...@rowe-clan.net> wrote:
>>> On 8/5/2010 3:22 AM, Ondřej Surý wrote:
>>>> William,
>>>>
>>>> On Thu, Aug 5, 2010 at 10:18, William A. Rowe Jr. <wr...@rowe-clan.net> wrote:
>>>>> On 8/5/2010 3:12 AM, Ondřej Surý wrote:
>>>>>> I have a question how to proceed. Is there really no interest in doing
>>>>>> this? (And fixing mod-vhost-alias as well.)
>>>>>>
>>>>>> Or should I fill the bug and attach the patch? Or ...?
>>>>>
>>>>> Please don't confuse ignoring your patch with lack of interest. Everyone
>>>>> here is a volunteer and dedicates the cycles they have free to attacking
>>>>> the most pressing issue they see at the time.
>>>>
>>>> Sorry if that offended you or anybody else. I didn't ment to sound
>>>> harsh as I was just merely asking what to do now.
>>>
>>> You didn't and I didn't mean to sound irritated, either, just trying to frame
>>> what happens on this particular list for your info.
>>
>> Ok :).
>>
>>> Many 'nice to have' things for Apache 2.4 are ignored as more critical issues
>>> in 2.2 are addressed, but with rising interest in frequent alpha/betas, we can
>>> all hope that improves :)
>>
>> I've filled this under #49705
>>
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=49705
>>
>> Ondrej
>> --
>> Ondřej Surý <on...@sury.org>
>>
>
>
>
> --
> Ondřej Surý <on...@sury.org>
>
--
Ondřej Surý <on...@sury.org>
Re: Per request DocumentRoot
Posted by Ondřej Surý <on...@sury.org>.
Hi,
I would like to raise this issue again. The proposed solution by
Anders Kaseorg:
> FWIW, when I ran into this problem in mod-vhost-ldap, I fixed it by
> making a copy of the entire server_rec structure in the request pool.
doesn't really work with mod_rewrite, because mod_rewrite contains this code:
if (conf->server != r->server) {
return DECLINED;
}
in the hook_uri2file (translate_name hook).
which prevents mod_rewrite to run if server_rec structure is modified
before mod_rewrite. However all virtual document root modules need to
be run before mod_rewrite for -f and -d to work.
As far as I can see, the moving the document_root to request_rec is
only viable solution in the long run for virtualization modules.
Ondrej
On Thu, Aug 5, 2010 at 11:53, Ondřej Surý <on...@sury.org> wrote:
> On Thu, Aug 5, 2010 at 10:25, William A. Rowe Jr. <wr...@rowe-clan.net> wrote:
>> On 8/5/2010 3:22 AM, Ondřej Surý wrote:
>>> William,
>>>
>>> On Thu, Aug 5, 2010 at 10:18, William A. Rowe Jr. <wr...@rowe-clan.net> wrote:
>>>> On 8/5/2010 3:12 AM, Ondřej Surý wrote:
>>>>> I have a question how to proceed. Is there really no interest in doing
>>>>> this? (And fixing mod-vhost-alias as well.)
>>>>>
>>>>> Or should I fill the bug and attach the patch? Or ...?
>>>>
>>>> Please don't confuse ignoring your patch with lack of interest. Everyone
>>>> here is a volunteer and dedicates the cycles they have free to attacking
>>>> the most pressing issue they see at the time.
>>>
>>> Sorry if that offended you or anybody else. I didn't ment to sound
>>> harsh as I was just merely asking what to do now.
>>
>> You didn't and I didn't mean to sound irritated, either, just trying to frame
>> what happens on this particular list for your info.
>
> Ok :).
>
>> Many 'nice to have' things for Apache 2.4 are ignored as more critical issues
>> in 2.2 are addressed, but with rising interest in frequent alpha/betas, we can
>> all hope that improves :)
>
> I've filled this under #49705
>
> https://issues.apache.org/bugzilla/show_bug.cgi?id=49705
>
> Ondrej
> --
> Ondřej Surý <on...@sury.org>
>
--
Ondřej Surý <on...@sury.org>
Re: Per request DocumentRoot
Posted by Ondřej Surý <on...@sury.org>.
On Thu, Aug 5, 2010 at 10:25, William A. Rowe Jr. <wr...@rowe-clan.net> wrote:
> On 8/5/2010 3:22 AM, Ondřej Surý wrote:
>> William,
>>
>> On Thu, Aug 5, 2010 at 10:18, William A. Rowe Jr. <wr...@rowe-clan.net> wrote:
>>> On 8/5/2010 3:12 AM, Ondřej Surý wrote:
>>>> I have a question how to proceed. Is there really no interest in doing
>>>> this? (And fixing mod-vhost-alias as well.)
>>>>
>>>> Or should I fill the bug and attach the patch? Or ...?
>>>
>>> Please don't confuse ignoring your patch with lack of interest. Everyone
>>> here is a volunteer and dedicates the cycles they have free to attacking
>>> the most pressing issue they see at the time.
>>
>> Sorry if that offended you or anybody else. I didn't ment to sound
>> harsh as I was just merely asking what to do now.
>
> You didn't and I didn't mean to sound irritated, either, just trying to frame
> what happens on this particular list for your info.
Ok :).
> Many 'nice to have' things for Apache 2.4 are ignored as more critical issues
> in 2.2 are addressed, but with rising interest in frequent alpha/betas, we can
> all hope that improves :)
I've filled this under #49705
https://issues.apache.org/bugzilla/show_bug.cgi?id=49705
Ondrej
--
Ondřej Surý <on...@sury.org>
Re: Per request DocumentRoot
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 8/5/2010 3:22 AM, Ondřej Surý wrote:
> William,
>
> On Thu, Aug 5, 2010 at 10:18, William A. Rowe Jr. <wr...@rowe-clan.net> wrote:
>> On 8/5/2010 3:12 AM, Ondřej Surý wrote:
>>> I have a question how to proceed. Is there really no interest in doing
>>> this? (And fixing mod-vhost-alias as well.)
>>>
>>> Or should I fill the bug and attach the patch? Or ...?
>>
>> Please don't confuse ignoring your patch with lack of interest. Everyone
>> here is a volunteer and dedicates the cycles they have free to attacking
>> the most pressing issue they see at the time.
>
> Sorry if that offended you or anybody else. I didn't ment to sound
> harsh as I was just merely asking what to do now.
You didn't and I didn't mean to sound irritated, either, just trying to frame
what happens on this particular list for your info.
Many 'nice to have' things for Apache 2.4 are ignored as more critical issues
in 2.2 are addressed, but with rising interest in frequent alpha/betas, we can
all hope that improves :)
Re: Per request DocumentRoot
Posted by Ondřej Surý <on...@sury.org>.
William,
On Thu, Aug 5, 2010 at 10:18, William A. Rowe Jr. <wr...@rowe-clan.net> wrote:
> On 8/5/2010 3:12 AM, Ondřej Surý wrote:
>> I have a question how to proceed. Is there really no interest in doing
>> this? (And fixing mod-vhost-alias as well.)
>>
>> Or should I fill the bug and attach the patch? Or ...?
>
> Please don't confuse ignoring your patch with lack of interest. Everyone
> here is a volunteer and dedicates the cycles they have free to attacking
> the most pressing issue they see at the time.
Sorry if that offended you or anybody else. I didn't ment to sound
harsh as I was just merely asking what to do now.
> You certainly can, some people track bugzilla better than the dev list.
> Others don't follow bugzilla closely, so both a bug and occassional dev@
> reminder are both good.
Thanks, I'll do that.
Ondrej
--
Ondřej Surý <on...@sury.org>
Re: Per request DocumentRoot
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 8/5/2010 3:12 AM, Ondřej Surý wrote:
> I have a question how to proceed. Is there really no interest in doing
> this? (And fixing mod-vhost-alias as well.)
>
> Or should I fill the bug and attach the patch? Or ...?
Please don't confuse ignoring your patch with lack of interest. Everyone
here is a volunteer and dedicates the cycles they have free to attacking
the most pressing issue they see at the time.
You certainly can, some people track bugzilla better than the dev list.
Others don't follow bugzilla closely, so both a bug and occassional dev@
reminder are both good.
Re: Per request DocumentRoot
Posted by Ondřej Surý <on...@sury.org>.
I have a question how to proceed. Is there really no interest in doing
this? (And fixing mod-vhost-alias as well.)
Or should I fill the bug and attach the patch? Or ...?
Thanks,
Ondrej
On Mon, Jul 19, 2010 at 09:30, Ondřej Surý <on...@sury.org> wrote:
>>> We could do something interesting with moving this to the req rec in 2.4
>>> (on 2.3 trunk) but it wouldn't solve your issue today with 2.2 or 2.0,
>>> which will not change.
>>
>> I am fine with that it doesn't get into 2.2 or 2.0. But I think that
>> fixing this in the right place would allow all other vhost modules to
>> benefit from that. mod-vhost-alias could be fixed as well.
>
> Here's the first version of patch for 2.3-trunk which implements
> per-request document_root.
>
> I have also updated m-v-a to use this new functionality.
>
> It compiles cleanly, however I'm not sure it it's complete. I hope I
> found all places where new request_rec is initialized, but I may have
> missed something.
>
> Ondrej
> --
> Ondřej Surý <on...@sury.org>
> http://blog.rfc1925.org/
>
--
Ondřej Surý <on...@sury.org>
Re: Per request DocumentRoot
Posted by Ondřej Surý <on...@sury.org>.
>> We could do something interesting with moving this to the req rec in 2.4
>> (on 2.3 trunk) but it wouldn't solve your issue today with 2.2 or 2.0,
>> which will not change.
>
> I am fine with that it doesn't get into 2.2 or 2.0. But I think that
> fixing this in the right place would allow all other vhost modules to
> benefit from that. mod-vhost-alias could be fixed as well.
Here's the first version of patch for 2.3-trunk which implements
per-request document_root.
I have also updated m-v-a to use this new functionality.
It compiles cleanly, however I'm not sure it it's complete. I hope I
found all places where new request_rec is initialized, but I may have
missed something.
Ondrej
--
Ondřej Surý <on...@sury.org>
http://blog.rfc1925.org/
Re: Per request DocumentRoot
Posted by Ondřej Surý <on...@sury.org>.
On Wed, Jul 7, 2010 at 13:32, William A. Rowe Jr. <wr...@rowe-clan.net> wrote:
> On 7/6/2010 12:24 PM, Anders Kaseorg wrote:
>> On Tue, 2010-07-06 at 10:50 +0200, Ondřej Surý wrote:
>>> I am a author of mod-vhost-ldap module and I have received complaints
>>> about DocumentRoot being unstable. After a fixing of dozen bugs and
>>> further debugging I have found out, that DocumentRoot is shared per
>>> process and not per thread and thus I am unable to fix that as a
>>> self-contained fix, since ap_document_root gets overwritten under a
>>> heavy load.
>>
>> FWIW, when I ran into this problem in mod-vhost-ldap, I fixed it by
>> making a copy of the entire server_rec structure in the request pool.
>> My mod-vhost-ldap patch is below.
Thanks for the patch, I'll merge it into mod-vhost-ldap. I was blind
:), thanks for kicking me into right direction.
> That's the way to approach it, mod_ftp does the same.
Anyway it seems to be a little overkill to copy whole server structure
for each request. So I would like to "fix" that properly.
> We could do something interesting with moving this to the req rec in 2.4
> (on 2.3 trunk) but it wouldn't solve your issue today with 2.2 or 2.0,
> which will not change.
I am fine with that it doesn't get into 2.2 or 2.0. But I think that
fixing this in the right place would allow all other vhost modules to
benefit from that. mod-vhost-alias could be fixed as well.
Just a side note: I would be willing to donate whole mod-vhost-ldap
code to ASF. Is there an interested to merge external modules like
this?
Ondrej
--
Ondřej Surý <on...@sury.org>
http://blog.rfc1925.org/
Re: Per request DocumentRoot
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 7/6/2010 12:24 PM, Anders Kaseorg wrote:
> On Tue, 2010-07-06 at 10:50 +0200, Ondřej Surý wrote:
>> I am a author of mod-vhost-ldap module and I have received complaints
>> about DocumentRoot being unstable. After a fixing of dozen bugs and
>> further debugging I have found out, that DocumentRoot is shared per
>> process and not per thread and thus I am unable to fix that as a
>> self-contained fix, since ap_document_root gets overwritten under a
>> heavy load.
>
> FWIW, when I ran into this problem in mod-vhost-ldap, I fixed it by
> making a copy of the entire server_rec structure in the request pool.
> My mod-vhost-ldap patch is below.
That's the way to approach it, mod_ftp does the same.
We could do something interesting with moving this to the req rec in 2.4
(on 2.3 trunk) but it wouldn't solve your issue today with 2.2 or 2.0,
which will not change.
Re: Per request DocumentRoot
Posted by Anders Kaseorg <an...@MIT.EDU>.
On Tue, 2010-07-06 at 10:50 +0200, Ondřej Surý wrote:
> I am a author of mod-vhost-ldap module and I have received complaints
> about DocumentRoot being unstable. After a fixing of dozen bugs and
> further debugging I have found out, that DocumentRoot is shared per
> process and not per thread and thus I am unable to fix that as a
> self-contained fix, since ap_document_root gets overwritten under a
> heavy load.
FWIW, when I ran into this problem in mod-vhost-ldap, I fixed it by
making a copy of the entire server_rec structure in the request pool.
My mod-vhost-ldap patch is below.
Anders
-- 8< --
>From ac2bea6ec55c4991667420d5bf503e82abd16e80 Mon Sep 17 00:00:00 2001
From: Anders Kaseorg <an...@mit.edu>
Date: Fri, 12 Feb 2010 07:47:59 +0000
Subject: [PATCH] Copy the server_rec instead of corrupting it in place.
Signed-off-by: Anders Kaseorg <an...@mit.edu>
---
mod_vhost_ldap.c | 32 +++++++++++++++++++++++++++-----
1 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/mod_vhost_ldap.c b/mod_vhost_ldap.c
index f3b729a..ff53d11 100644
--- a/mod_vhost_ldap.c
+++ b/mod_vhost_ldap.c
@@ -94,6 +94,8 @@ typedef struct mod_vhost_ldap_request_t {
char *attributes[] =
{ "apacheServerName", "apacheDocumentRoot", "apacheScriptAlias", "apacheSuexecUid", "apacheSuexecGid", "apacheServerAdmin", 0 };
+static int total_modules;
+
#if (APR_MAJOR_VERSION >= 1)
static APR_OPTIONAL_FN_TYPE(uldap_connection_close) *util_ldap_connection_close;
static APR_OPTIONAL_FN_TYPE(uldap_connection_find) *util_ldap_connection_find;
@@ -117,6 +119,13 @@ static void ImportULDAPOptFn(void)
static int mod_vhost_ldap_post_config(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *ptemp, server_rec *s)
{
+ module **m;
+
+ /* Stolen from modules/generators/mod_cgid.c */
+ total_modules = 0;
+ for (m = ap_preloaded_modules; *m != NULL; m++)
+ total_modules++;
+
/* make sure that mod_ldap (util_ldap) is loaded */
if (ap_find_linked_module("util_ldap.c") == NULL) {
ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, s,
@@ -421,7 +430,6 @@ command_rec mod_vhost_ldap_cmds[] = {
#define FILTER_LENGTH MAX_STRING_LEN
static int mod_vhost_ldap_translate_name(request_rec *r)
{
- request_rec *top = (r->main)?r->main:r;
mod_vhost_ldap_request_t *reqc;
apr_table_t *e;
int failures = 0;
@@ -575,17 +583,31 @@ fallback:
return DECLINED;
}
- top->server->server_hostname = apr_pstrdup (top->pool, reqc->name);
+ if ((r->server = apr_pmemdup(r->pool, r->server,
+ sizeof(*r->server))) == NULL)
+ return HTTP_INTERNAL_SERVER_ERROR;
+
+ r->server->server_hostname = reqc->name;
if (reqc->admin) {
- top->server->server_admin = apr_pstrdup (top->pool, reqc->admin);
+ r->server->server_admin = reqc->admin;
}
// set environment variables
- e = top->subprocess_env;
+ e = r->subprocess_env;
apr_table_addn (e, "SERVER_ROOT", reqc->docroot);
- core->ap_document_root = apr_pstrdup(top->pool, reqc->docroot);
+ if ((r->server->module_config =
+ apr_pmemdup(r->pool, r->server->module_config,
+ sizeof(void *) *
+ (total_modules + DYNAMIC_MODULE_LIMIT))) == NULL)
+ return HTTP_INTERNAL_SERVER_ERROR;
+
+ if ((core = apr_pmemdup(r->pool, core, sizeof(*core))) == NULL)
+ return HTTP_INTERNAL_SERVER_ERROR;
+ ap_set_module_config(r->server->module_config, &core_module, core);
+
+ core->ap_document_root = reqc->docroot;
ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r,
"[mod_vhost_ldap.c]: translated to %s", r->filename);
--
1.7.1.1