Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

interaction issue caused by ordering of passenger's mod_dir block #3

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

saurik commented Oct 19, 2010

(This is my first "pull request": I therefore simply copied my commit message, as that seemed sufficient.)

Fix "crude" hook calling order of end_blocking_mod_dir to have parity with Apache's mod_dir.

When Passenger's end_blocking_mod_dir hook is executed, all of its "pre" dependencies, in this case mod_dir, are executed immediately. Unfortunately, this means that mod_dir, which is normally a very low priority HOOK_LAST, gets bumped up to HOOK_MIDDLE when mod_passenger is installed (even if it is inactive, and even if the hooks are totally neutered).

This breaks any other existing web applications (such as a mod_python installation) that are relying on being able to execute before mod_dir by having registered as HOOK_MIDDLE. To fix this, the flags for hooks dependent on mod_dir have been updated to match the calling order of the underlying module from Apache.

(As I do not have a similar issue with mod_autoindex I have not adjusted end_blocking_mod_autoindex to HOOK_MIDDLE; however, it should probably be looked at again, keeping this issue in mind.)

@saurik saurik Fix "crude" hook calling order of end_blocking_mod_dir to have parity…
… with Apache's mod_dir.

When Passenger's end_blocking_mod_dir hook is executed, all of its "pre" dependencies, in this case mod_dir, are executed immediately. Unfortunately, this means that mod_dir, which is normally a very low priority HOOK_LAST, gets bumped up to HOOK_MIDDLE when mod_passenger is installed (even if it is inactive, and even if the hooks are totally neutered).

This breaks any other existing web applications (such as a mod_python installation) that are relying on being able to execute before mod_dir by having registered as HOOK_MIDDLE. To fix this, the flags for hooks dependent on mod_dir have been updated to match the calling order of the underlying module from Apache.

(As I do not have a similar issue with mod_autoindex I have not adjusted end_blocking_mod_autoindex to HOOK_MIDDLE; however, it should probably be looked at again, keeping this issue in mind.)
806f138
Contributor

saurik commented Oct 20, 2010

(To be clear, my usage of the word "crude" was because that is how Apache's documentation describes the usage of that parameter, not because I thought Passenger's usage was to be inferred negatively.)

Owner

FooBarWidget commented Oct 27, 2010

Interesting, I didn't know that Apache would "bump" hooks to the middle. In intention is clearly to install code to be executed after mod_dir, not to readjust mod_dir's priority.

I need to have a better look at the code to decide whether this change would break anything.

Contributor

saurik commented Oct 27, 2010

Neither did I (know that it did that), and I will note that I'm willing to believe that my analysis of the situation is wrong (that that is the specific underlying cause of the reordering), but I have definitely determined that the reordering of mod_dir (which is supposed to be LAST) to happen before mod_python (which is MIDDLE and should therefore come before mod_dir) occurs when mod_passenger registers its dependent hook with MIDDLE (and not when mod_passenger registers itself as LAST).

Contributor

saurik commented Feb 24, 2011

Is there anything I can do to help explain this? The change is a single argument to a single function that will only affect people who are attempting to do deployments of multiple large-scale mod_dir-intercepting Apache frameworks into the same process: changing this behavior should not affect anyone else but will make this corner case actually possible for those who are running into it, without having to keep maintaining forked copies of Passenger. It was really disappointing to read on Hacker News today that Passenger 3.0.3 was released without this patch included. :(

Owner

FooBarWidget commented Feb 24, 2011

Could you investigate whether bumping both start_blocking_mod_dir and stop_blocking_mod_dir to APR_HOOK_LAST works and whether you can find any new incompatibilities introduced by the change?

Contributor

saurik commented Feb 24, 2011

I have verified that moving both start_blocking_mod_dir and stop_blocking_mod_dir to APR_HOOK_LAST "works for me".

Owner

FooBarWidget commented Feb 24, 2011

I've merged in your patch. Thanks.

This was referenced May 29, 2014

@FooBarWidget FooBarWidget added a commit that referenced this pull request Mar 22, 2017

@FooBarWidget FooBarWidget # This is a combination of 5 commits.
# This is the 1st commit message:
SpawningKit: split spawning preparation code into its own file

# This is the commit message #2:

externspawner

# This is the commit message #3:

...

# This is the commit message #4:

Revert "..."

This reverts commit dd82bc346d791ff7df899148252241c59ad76cf8.

# This is the commit message #5:

Revert "externspawner"

This reverts commit 544a097fd5532a46874c9ebc32081059bea1685b.
2a45e33

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment