Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Show breakage for IIS FCGI. #284

Open
wants to merge 4 commits into from

4 participants

@bobtfish
Collaborator

From Catalyst's RT#76522, a combination of the code which
is supposedly for lighttpd multiple slases, and the IIS
fix Middleware don't play nicely together - meaning that
IIS doesn't work as expected with the FCGI handler.

I guess that the fix should become conditional on lighttpd,
or should be entirely broken out into middleware
(rather than the IIS6 middleware doing more tricks)..

Thoughts?

@KnowZero

What is needed to get this implemented? Because as-is I have to constantly modify Plack FCGI handler to make it work on any version of IIS.

@ttulttul

This breaks Apache+FastCGI as well... Maybe just move the fix down a few lines inside the lighttpd-specific block? Why would you ever "fix" something for lighttpd and then run that fix for all web servers?

@miyagawa
Owner

The comment could be confusing but the fix is meant to work with any web servers including Apache that munges multiple slashes into one.

@KnowZero

What about IIS? In IIS the PATH_INFO is generally identical to the SCRIPT_NAME so it ends up being destroyed completely.

@miyagawa
Owner

That's a separate issue for IIS and that's why the IIS fix middleware should exist, but generally if PATH_INFO is always identical to SCRIPT_NAME then it has to be destroyed (or swapped like lighttpd) so that you can dispatch requests correctly, while keeping the "base" URL in SCRIPT_NAME.

@KnowZero

The IIS middleware does not work due to that code. The only way I got the middleware to work was by applying this:

my @script_name = split(m!/!, $env->{PATH_INFO} eq '' ? $env->
{SCRIPT_NAME}:$env->{PATH_INFO} );

That said should Plack really destroy the PATH_INFO ENV variable without making a copy? Because effectively PATH_INFO is destroyed before it ever reaches the middleware.

@miyagawa
Owner

Yeah the problem is that the code runs on the handler so that middleware can't fix - there's a similar discussion in #254.

@miyagawa
Owner

I guess #254 #288 #299 are basically all the same issue.

Maybe, maybe "resurrecting the correct PATH_INFO from REQUEST_URI" bit can be moved out into its own middleware and now pushed to users responsibility to apply, if you want double slashes be factored out or do funky things with mod_rewrite and PATH_INFO like #299.

You guys can coordinate a fix to address all of them with proper tests, I'll be happy to merge.

@KnowZero

The code I posted runs on the middleware and just uses SCRIPT_NAME if PATH_INFO is blank.

But I think a cleaner solution would be to make a backup of the PATH_INFO in the handler and then let the middleware decide what to use.

Cause right now out of box this makes running IIS and FCGI at the same time impossible without manual modification.

@miyagawa
Owner

@KnowZero whatever a fix that solves all problems posted in these issues and makes everyone happy would be happily accepted and merged. I never intended to support IIS with Plack/FastCGI - it should be a community effort if you really want it.

Also, you can always fork FCGI and release it as FCGIIS.pm or whatever.

@KnowZero

I am willing to write a patch to make it work, I will write whatever I think will work for IIS and if ttulttul is willing to help on the Apache side then we can get it covered. To summarize what I want to do:

Handler:
$env->{PLACK_PATH_INFO}=$env->{PATH_INFO};

Middleware:
** RESTORE PATH_INFO from PLACk_PATH_INFO **

As for forking FCGI, and releasing it separately. I thought about that but I think it will cause too much confusion since there is already an iis6scriptfix (which should probably be renamed as this effects all versions of iis, not just 6)

@ttulttul

I unfortunately don't have time to write the Apache fix; I just manually patched up Catalyst::Engine for this one specific use case - it's legacy code for us and we're unlikely to ever deploy it again.. Still, this issue cost us four days (ouch). Sorry, guys.

bobtfish added some commits
@bobtfish bobtfish Update .gitignore to ignore MYMETA ba99e81
@bobtfish bobtfish Show breakage for IIS FCGI.
From Catalyst's RT#76522, a combination of the code which
is supposedly for lighttpd multiple slases, and the IIS
fix Middleware don't play nicely together - meaning that
IIS doesn't work as expected with the FCGI handler.

I guess that the fix should become conditional on lighttpd,
or should be entirely broken out into middleware
(rather than the IIS6 middleware doing more tricks)..
343d295
@bobtfish bobtfish Remove the multiple slashes fix from the FCGI handler into middleware.
This code was originally added in:

commit 5a22236
commit 2af4bb5

however causes a load of problems if applied unconditionally,
and so is removed from the core FCGI handler into a middleware here.
e4cf555
@bobtfish bobtfish Wrap SCRIPT_NAME in \Q \E modifiers in the apache registry handler as…
… that's more correct
706682b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 18, 2012
  1. @bobtfish
  2. @bobtfish

    Show breakage for IIS FCGI.

    bobtfish authored
    From Catalyst's RT#76522, a combination of the code which
    is supposedly for lighttpd multiple slases, and the IIS
    fix Middleware don't play nicely together - meaning that
    IIS doesn't work as expected with the FCGI handler.
    
    I guess that the fix should become conditional on lighttpd,
    or should be entirely broken out into middleware
    (rather than the IIS6 middleware doing more tricks)..
  3. @bobtfish

    Remove the multiple slashes fix from the FCGI handler into middleware.

    bobtfish authored
    This code was originally added in:
    
    commit 5a22236
    commit 2af4bb5
    
    however causes a load of problems if applied unconditionally,
    and so is removed from the core FCGI handler into a middleware here.
Commits on Jul 19, 2012
  1. @bobtfish
This page is out of date. Refresh to see the latest.
View
1  .gitignore
@@ -1,3 +1,4 @@
+MYMETA.*
META.yml
Makefile
inc/
View
2  lib/Plack/Handler/Apache2/Registry.pm
@@ -27,7 +27,7 @@ sub handler {
# Overriding
sub fixup_path {
my ($class, $r, $env) = @_;
- $env->{PATH_INFO} =~ s{^$env->{SCRIPT_NAME}}{};
+ $env->{PATH_INFO} =~ s/^\Q$env->{SCRIPT_NAME}\E//;
}
1;
View
5 lib/Plack/Handler/FCGI.pm
@@ -111,11 +111,6 @@ sub run {
delete $env->{HTTP_CONTENT_TYPE};
delete $env->{HTTP_CONTENT_LENGTH};
- # 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//;
-
if ($env->{SERVER_SOFTWARE} && $env->{SERVER_SOFTWARE} =~ m!lighttpd[-/]1\.(\d+\.\d+)!) {
no warnings;
if ($ENV{PLACK_ENV} eq 'development' && $1 < 4.23 && $env->{PATH_INFO} eq '') {
View
51 lib/Plack/Middleware/FixPathInfoMultipleSlashes.pm
@@ -0,0 +1,51 @@
+package Plack::Middleware::FixPathInfoMultipleSlashes;
+
+use strict;
+use parent 'Plack::Middleware';
+use URI::Escape;
+use URI;
+
+sub call {
+ my($self, $env) = @_;
+
+ # lighttpd and apache munge 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//;
+
+ return $self->app->($env);
+}
+
+1;
+
+__END__
+
+=head1 NAME
+
+Plack::Middleware::FixPathInfoMultipleSlashes - fixes wrong PATH_INFO that lighttpd and apache generate by munging multiple slashes to one
+
+=head1 SYNOPSIS
+
+ # in your app.psgi
+ use Plack::Builder;
+
+ builder {
+ enable "FixPathInfoMultipleSlashes";
+ $app;
+ };
+
+ # Or from the command line
+ plackup -s FCGI -e 'enable "FixPathInfoMultipleSlashes"' /path/to/app.psgi
+
+=head1 DESCRIPTION
+
+This middleware fixes wrong C<PATH_INFO> set by lighttpd and apache which munge multiple slashes to a single slash.
+
+This middleware is not applied automatically, as it can break use of mod_rewrite when prepending things to the C<PATH_INFO>.
+
+=head1 AUTHORS
+
+Tomas Doran, Tatsuhiko Miyagawa, cho45
+
+=cut
+
View
5 lib/Plack/Middleware/IIS6ScriptNameFix.pm
@@ -2,12 +2,15 @@ package Plack::Middleware::IIS6ScriptNameFix;
use strict;
use parent 'Plack::Middleware';
+use URI::Escape;
+use URI;
sub call {
my($self, $env) = @_;
if ($env->{SERVER_SOFTWARE} && $env->{SERVER_SOFTWARE} =~ /IIS\/[6-9]\.[0-9]/) {
- my @script_name = split(m!/!, $env->{PATH_INFO});
+ my $uri = URI->new("http://localhost" . $env->{REQUEST_URI});
+ my @script_name = split(m!/!, uri_unescape($uri->path));
my @path_translated = split(m!/|\\\\?!, $env->{PATH_TRANSLATED});
my @path_info;
View
19 lib/Plack/Test/Suite.pm
@@ -710,25 +710,6 @@ our @TEST = (
];
},
],
- [
- 'repeated slashes',
- sub {
- my $cb = shift;
- my $res = $cb->(GET "http://127.0.0.1//foo///bar/baz");
- is $res->code, 200;
- is $res->message, 'OK';
- is $res->header('content_type'), 'text/plain';
- is $res->content, '//foo///bar/baz';
- },
- sub {
- my $env = shift;
- return [
- 200,
- [ 'Content-Type' => 'text/plain', ],
- [ $env->{PATH_INFO} ],
- ];
- },
- ],
);
sub runtests {
View
63 t/Plack-Middleware/fcgi_path_info_multipleslashes_fix.t
@@ -0,0 +1,63 @@
+use strict;
+use warnings;
+use Test::More;
+
+use Plack::Middleware::FixPathInfoMultipleSlashes;
+use URI::Escape;
+use URI;
+
+my %env = (
+ 'psgi.multiprocess' => 1,
+ 'SCRIPT_NAME' => '/fastcgi',
+ 'PATH_INFO' => '/foo/bar/baz',
+ 'REQUEST_METHOD' => 'GET',
+ 'psgi.multithread' => '',
+ 'SCRIPT_FILENAME' => '/var/folders/9y/f64sy2xx3vnb8ddv4w1g498m0000gn/T/utzt7PlANx/fastcgi',
+ 'SERVER_SOFTWARE' => 'lighttpd/1.4.30',
+ 'HTTP_TE' => 'deflate,gzip;q=0.3',
+ 'REMOTE_PORT' => '54624',
+ 'QUERY_STRING' => '',
+ 'HTTP_USER_AGENT' => 'libwww-perl/6.04',
+ 'FCGI_ROLE' => 'RESPONDER',
+ 'psgi.streaming' => 1,
+ 'GATEWAY_INTERFACE' => 'CGI/1.1',
+ 'psgi.version' => [
+ 1,
+ 1
+ ],
+ 'DOCUMENT_ROOT' => '/var/folders/9y/f64sy2xx3vnb8ddv4w1g498m0000gn/T/utzt7PlANx/',
+ 'psgi.run_once' => '',
+ 'PATH_TRANSLATED' => '/var/folders/9y/f64sy2xx3vnb8ddv4w1g498m0000gn/T/utzt7PlANx//foo/bar/baz',
+ 'SERVER_NAME' => '127.0.0.1',
+ 'HTTP_CONNECTION' => 'TE, close',
+ 'SERVER_PORT' => '50992',
+ 'REDIRECT_STATUS' => '200',
+ 'REMOTE_ADDR' => '127.0.0.1',
+ 'SERVER_PROTOCOL' => 'HTTP/1.1',
+ 'REQUEST_URI' => '/fastcgi//foo///bar/baz',
+ 'psgi.nonblocking' => '',
+ 'SERVER_ADDR' => '127.0.0.1',
+ 'psgi.url_scheme' => 'http',
+ 'psgix.harakiri' => 1,
+ 'HTTP_X_PLACK_TEST' => '35',
+ 'HTTP_HOST' => '127.0.0.1:50992',
+);
+
+sub test_fix {
+ my ($input_env) = @_;
+
+ my $mangled_env;
+ Plack::Middleware::FixPathInfoMultipleSlashes->wrap(sub {
+ my ($env) = @_;
+ $mangled_env = $env;
+ return [ 200, ['Content-Type' => 'text/plain'], [''] ];
+ })->($input_env);
+
+ return $mangled_env;
+}
+
+my $fixed_env = test_fix({ %env });
+
+is($fixed_env->{PATH_INFO}, '//foo///bar/baz', 'check PATH_INFO');
+
+done_testing;
View
14 t/Plack-Middleware/iis6_script_name_fix.t
@@ -3,6 +3,8 @@ use warnings;
use Test::More;
use Plack::Middleware::IIS6ScriptNameFix;
+use URI::Escape;
+use URI;
my %env = (
'SCRIPT_NAME' => '/koo/blurb',
@@ -65,4 +67,16 @@ my $fixed_env = test_fix({ %env });
is($fixed_env->{PATH_INFO}, '//blurb', 'check PATH_INFO');
is($fixed_env->{SCRIPT_NAME}, '/koo', 'check SCRIPT_NAME');
+# From Plack::Handler::FCGI:
+## 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//;
+
+# And re-test!
+$fixed_env = test_fix({ %env });
+
+is($fixed_env->{PATH_INFO}, '//blurb', 'check PATH_INFO');
+is($fixed_env->{SCRIPT_NAME}, '/koo', 'check SCRIPT_NAME');
+
done_testing;
Something went wrong with that request. Please try again.