Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround for mod_perl introduced a bug for mod_rewrite #254

Open
niner opened this issue Oct 24, 2011 · 10 comments
Open

Workaround for mod_perl introduced a bug for mod_rewrite #254

niner opened this issue Oct 24, 2011 · 10 comments

Comments

@niner
Copy link

niner commented Oct 24, 2011

Commit 5a22236 introduced a workaround for a mod_perl issue. Since mod_perl squeezes multiple slashes into one in PATH_INFO, REQUEST_URI is used instead to generate a new PATH_INFO.

We use a content management system based on Catalyst 5.9 which uses Plack underneath. The CMS is running as FastCGI process and accessed via mod_fastcgi from Apache. We use mod_rewrite to provide short URLs for certain sub pages. With Catalyst 5.8 this works while with 5.9 this is broken, resulting in 404 errors from the CMS. The problem is, that PATH_INFO now contains the short URL which the CMS knows nothing about. Without Plack, REQUEST_URI contains the short URL while PATH_INFO contains the destination path which is what the CMS needs to find the right page.

Removing the above mentioned workaround for mod_perl fixes the issue for us.

@miyagawa
Copy link
Member

Can you post an example combination of PATH_INFO and REQUEST_URI, as well as the (possibly simplified version of) mod_rewrite configurations?

@niner
Copy link
Author

niner commented Oct 25, 2011

These are the relevant configuration statements for this issue:
RewriteEngine On
RewriteRule ^/kaarst /content/steuerberater/stb/wir_ueber_uns/unsere_standorte/steuerberater_kaarst/unsere_philosophie/index_ger.html [PT]
AliasMatch ^/(?!cgi-bin|error|icons)(.*)$ /srv/www/tmp/cms.fcgi/$1

This would be the desired result:
SCRIPT_NAME: /
REQUEST_URI: /kaarst
PATH_INFO: /content/steuerberater/stb/wir_ueber_uns/unsere_standorte/steuerberater_kaarst/unsere_philosophie/index_ger.html

This is the actual result with the mod_perl workaround in place in Plack:
SCRIPT_NAME: /
REQUEST_URI: /kaarst
PATH_INFO: /kaarst

@miyagawa
Copy link
Member

Thanks.

There's a similar one on FCGI http://groups.google.com/group/psgi-plack/browse_thread/thread/d690e1b57e79b709

@cho45 @bobtfish I don't know, but maybe the double slash fix should be extracted as a piece of middleware and disabled by default or something? This is pain in the ass (mod_rewrite generally is, though).

@bobtfish
Copy link
Member

Yeah, stopping this being default (so Catalyst doesn't use it) is the correct fix for what we want to do...

It is both a pain in the ass, and a mess, and we have a setting for 'old style' (where just PATH_INFO) is used, and 'new style' (where REQUEST_URI is used).

http://search.cpan.org/~bobtfish/Catalyst-Runtime-5.90005/lib/Catalyst.pm#CONFIGURATION (use_request_uri_for_path at the bottom of the list here)

@cho45
Copy link
Contributor

cho45 commented Oct 25, 2011

Okay... I understood the problem finally....

Unfortunately, this is too difficult to resolve fundamentally. So I agree with @bobtfish about the setting and the (smaller complaint) default.

However, I don't know if a middleware does it... (or fix in embed in code and switch with environment variable).

@miyagawa
Copy link
Member

I think a piece of middleware can do for FastCGI.

It basically has to look at REQUEST_URI, s/^SCRIPT_NAME// and then URI-decode it and stash the result into PATH_INFO.

For Apache, it would be a bit more complicated because of its LocationMatch madness.

@cho45
Copy link
Contributor

cho45 commented Oct 25, 2011

For Apache, an easiest way is that a middleware just flags to $env and P::H::Apache2 changes behavior by the flag...

@miyagawa
Copy link
Member

Apache handler runs before any middleware runs, so it might not work :)

We can extract the location handling as middleware as well, but that sounds a bit too painful to configure.

@cho45
Copy link
Contributor

cho45 commented Oct 25, 2011

Oh... I have overlooked.

@niner
Copy link
Author

niner commented Jan 16, 2012

Is there any progress on this issue? Is there something I can do to help fix this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants