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

"Fix" for lighttpd in Handler/FCGI.pm breaks Apache+FastCGI #299

Open
ttulttul opened this issue Jun 27, 2012 · 13 comments
Open

"Fix" for lighttpd in Handler/FCGI.pm breaks Apache+FastCGI #299

ttulttul opened this issue Jun 27, 2012 · 13 comments

Comments

@ttulttul
Copy link

This code breaks path handling in Apache+FastCGI:

        # lighttpd munges multiple slashes in PATH_INFO into one. Try recovering it
        my $uri = URI->new("http://localhost" .  $env->{REQUEST_URI});
        $env->{PATH_INFO} = uri_unescape($uri->path);
        $env->{PATH_INFO} =~ s/^\Q$env->{SCRIPT_NAME}\E//;

Can someone please make this code execute only when the server really actually is lighttpd?

@miyagawa
Copy link
Member

Break how? Provide more details and/or unit test to reproduce the issue.

Sent from my iPhone

On Wednesday, June 27, 2012 at 2:18 PM, ttulttul wrote:

This code breaks path handling in Apache+FastCGI:

# lighttpd munges multiple slashes in PATH_INFO into one. Try recovering it
my $uri = URI->new("http://localhost" . $env->{REQUEST_URI});
$env->{PATH_INFO} = uri_unescape($uri->path);
$env->{PATH_INFO} =~ s/^\Q$env->{SCRIPT_NAME}\E//;

Can someone please make this code execute only when the server really actually is lighttpd?


Reply to this email directly or view it on GitHub:
https://github.com/miyagawa/Plack/issues/299

@ttulttul
Copy link
Author

I don't have time to put together a unit test - sorry about that - but here is an example of the breakage.

Before this code, PATH_INFO = "/localhost/login", SCRIPT_NAME = "", and REQUEST_URI = "/login".
After this code, PATH_INFO = "/login". The "/localhost" stuff is really important in our application and should not have been removed.

Why throw away the old contents of PATH_INFO and assume that REQUEST_URI will be an appropriate replacement?

@miyagawa
Copy link
Member

Do you have some own weird mod_rewrite setup? I can't see how you get /localhost passed to the PATH_INFO except it's added inside this code, which is just a fake marker to represent the hostname. If the SCRIPT_NAME doesn't contain /localhost in itself, the code should NOT remove it from PATH_INFO.

@miyagawa
Copy link
Member

The comment is meant to say that this was originally to fix the issue with lighttpd, but as the commit you pointed out suggests with the github issue, this fix is universal to work with any FastCGI setup.

@KnowZero
Copy link

That line also breaks IIS+FCGI as displayed in Issue #284 has a working test script of how it breaks IIS.

@miyagawa
Copy link
Member

Yes, repeating slashes inside its own SCRIPT_NAME might be an issue - but /localhost? I can't see how that could happen.

@ttulttul
Copy link
Author

"/localhost" is a bad example. In our case, we're using rewrite to insert the "site" name into a multi-tenant web app. The "localhost" might be "site1", "site2" etc.. We really do need the PATH_NAME variable to be left unmolested because it differs from the REQUEST_URI, which mod_rewrite does not modify.

@miyagawa
Copy link
Member

OK, yes, if you use rewrite to inject some custom variables into PATH_INFO that could be an issue.

@ttulttul
Copy link
Author

Here is the rewrite code - I don't think it's too unusual. This is just following the Catalyst Apache+FastCGI documentation:

    RewriteEngine On
    RewriteRule ^/?(.*) /usr/local/tcls-ui/script/fastcgi.pl/sitename/$1 [L]

@dairiki
Copy link

dairiki commented Oct 4, 2012

I'm not sure what the best solution for this is, but here's another vote for either moving the multiple-slash fix into middleware or otherwise providing some way to disable the "fix".

Here's my use case, if you're interested:

Legacy Mason app, which I've just converted to run under PSGI. This is running as a FastCGI service behind nginx.
Up until now we've been relying on nginx to add 'index.html' to requests for "directories". E.g. if the request is for /?q=foo, because we have an index index.html; directive in our nginx config, nginx rewrites (what eventually becomes) PATH_INFO to /index.html. REQUEST_URI meanwhile (correctly) remains /?q=foo. The double-slash-fix in Plack::Handler::FCGI ends up setting PATH_INFO back to / which causes havoc.

Anyhow, I do understand how restoring multiple slashes may be important for some apps, but to me, that seems like an edge case. There are legitimate reasons (e.g. apache's mod_rewrite or nginx's rewrite) that PATH_INFO may not be the same as REQUEST_URI. It was a big surprise to me when I figured out that the FSGI handler was totally ignoring the PATH_INFO.

@nmatasci
Copy link

There is a more serious issue because this fix makes it incompatible with HTTP/1.1 /RFC 2616 (http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html). In particular:

To allow for transition to absoluteURIs in all requests in future versions of HTTP, all HTTP/1.1 servers MUST accept the absoluteURI form in requests, even though HTTP/1.1 clients will only generate them in requests to proxies.

If the request contains an absolute URI, because of the fix, the $uri variable becomes "http://localhosthttp://example.com/index.html" and the PATH_INFO variable is then set to //example.com/index.html.

An easy fix is to test whether REQUEST_URI is an absolute path.

@dracos
Copy link

dracos commented Dec 15, 2016

As with @dairiki and others, this has just bitten me, here are the straightforward relevant lines from the Apache config:

RewriteRule ^/$         /static/front
...
RewriteRule ^(.*)$      /catalyst_app_fastcgi.cgi$1  [L]

When Apache sends the information to the app, PATH_INFO is /static/front and REQUEST_URI is / – but because of the code mentioned in this ticket, PATH_INFO is reset to /, and the app thinks it’s being asked for the front page. I can’t see anyway to get around this issue beyond replacing Plack::Handler::FCGI.

@mdhafen
Copy link

mdhafen commented Nov 7, 2017

I have another case where this code breaks things:
I'm using Plack::App::CGIBin on a legacy app, which uses Plack::App::File::locate_file() which looks for the file name in PATH_INFO. Because of this code resets PATH_INFO from REQUEST_URI the file name will never be there unless we mangle REQUEST_URI too, or make sure SCRIPT_NAME is empty (which I don't know how to do in Apache2).

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

7 participants