Skip to content
This repository

Stop load_psgi from searching .psgi files from @INC #343

Merged
merged 4 commits into from over 1 year ago

4 participants

Tatsuhiko Miyagawa Jesse Luehrs Tomas Doran KnowZero
Tatsuhiko Miyagawa
Owner

If you have foo.psgi in any of your @INC paths, running plackup foo.psgi used to try to load the one in there first rather than the current directory, which is listed as the last entry of include paths with usual perl.

This would confuse users potentially. With this change load_psgi function in Plack::Util does a little more DWIMmery to make it into ./foo.psgi unless the given path begins with / (slash).

Arguably this might break some users, who uses this feature for their CPAN modules to install PSGI scripts into @INC expecting it to be picked up by plackup foo.psgi. That will stop working, although we never recommended it, and if they have a module (not a script) like MyApp::PSGI into include paths that will keep working.

miyagawa and others added some commits
Tatsuhiko Miyagawa miyagawa Stop load_psgi() from loading files from @INC if it's given a filenam…
…e args.

This might break users who installs `foo.psgi` into @INC directories
and expect `plackup foo.psgi` to load them from there - it's been a
mix of documented (loading class names like MyApp::PSGI) and
undocumented feature anyways.
e78bbf0
Tatsuhiko Miyagawa miyagawa Documentation 990d956
Tatsuhiko Miyagawa miyagawa Add with git add -f (inc/ matches with my gitignore) f5b8faa
Jesse Luehrs doy explicitly exclude t/Plack-Util/inc/ from gitignore f579a39
Tatsuhiko Miyagawa
Owner

@plack/core care to take a look?

Jesse Luehrs
Owner
doy commented
Tomas Doran
Collaborator

+1 here, and -1 for pushing . onto @INC - this just adds more magic..

Tatsuhiko Miyagawa miyagawa merged commit 63882c6 into from
KnowZero

This change breaks Catalyst on Windows :(

C:\Sites\Test\script>perl64 test_server.pl
Error while loading ./C:\Sites\Test\test.psgi: No such file or directory at (eval 1307) line 4, line 1000.

Using ActiveState Perl 5.16 64bit on Windows 2008R2 64bit and IIS 7.5 and Plack 1.0007 and Catalyst 5.90016.

Is it possible to change line 128 of lib/Plack/Util.pm to:

$file =~ m!^(?:/|[a-zA-Z]\:)! ? $file : "./$file";
Tatsuhiko Miyagawa
Owner

What's the content of test_server.pl?

KnowZero

The default Catalyst generates:

BEGIN {

$ENV{CATALYST_SCRIPT_GEN} = 40;

}

use Catalyst::ScriptRunner;

Catalyst::ScriptRunner->run('Test', 'Server');

1;

KnowZero

The problem is unlike Linux, Windows has drive letters and "./C:\" is an invalid path.

Tatsuhiko Miyagawa miyagawa referenced this pull request from a commit
Tatsuhiko Miyagawa miyagawa Checking in changes prior to tagging of version 1.0008.
Changelog diff is:

diff --git a/Changes b/Changes
index 76d3cd1..6e958c6 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,9 @@
 Go to http://github.com/plack/Plack/issues for the roadmap and known issues.

+1.0008  Mon Oct 22 18:52:29 PDT 2012
+    [BUG FIXES]
+        - Allow drive letters for absolute paths for plackup and load_psgi #343
+
 1.0007  Sat Oct 20 23:20:20 PDT 2012
     [IMPROVEMENTS]
         - Fix test failures with HTTP::Message 6.06. #345
f863ec5
Tatsuhiko Miyagawa miyagawa referenced this pull request from a commit
Tatsuhiko Miyagawa miyagawa Checking in changes prior to tagging of version 1.0009.
Changelog diff is:

diff --git a/Changes b/Changes
index 6e958c6..1823d91 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,9 @@
 Go to http://github.com/plack/Plack/issues for the roadmap and known issues.

+1.0009  Tue Oct 23 00:57:16 PDT 2012
+    [BUG FIXES]
+        - Correct fix to address drive letters for Win32
+
 1.0008  Mon Oct 22 18:52:29 PDT 2012
     [BUG FIXES]
         - Allow drive letters for absolute paths for plackup and load_psgi #343
8c6c331
Christian Walde wchristian referenced this pull request from a commit in wchristian/Plack
Tatsuhiko Miyagawa miyagawa Checking in changes prior to tagging of version 1.0006.
Changelog diff is:

diff --git a/Changes b/Changes
index f7ced00..13330ff 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,17 @@
 Go to http://github.com/plack/Plack/issues for the roadmap and known issues.

+1.0006  Thu Oct 18 16:06:15 PDT 2012
+    [INCOMPATIBLE CHANGES]
+        - plackup foo.psgi will not search the file in @INC anymore before the current directory
+          See plack#343 for details (miyagawa)
+
+    [NEW FEATURES]
+        - plackup --path /foo will mount the application under /path (mattn)
+
+    [BUG FIXES]
+        - AccessLog: Fix the timezon offset for certain timezones
+        - ErrorDocument: support streaming interface
+
 1.0005  Tue Oct  9 13:33:47 PDT 2012
     [NEW FEATURES]
         - Support psgix.cleanup handlers in Apache2 (avar)
ecb3a36
Jonathan Perkin jperkin referenced this pull request from a commit in joyent/pkgsrc
wen Update to 1.0015
Upstream changes:
1.0015 Thu Jan 10 15:19:17 PST 2013
    [BUG FIXES]
        - Fixed Lint complaining about Latin-1 range characters stored internally with
          utf8 flag on (Mark Fowler)
        - HTTP::Message::PSGI::res_from_psgi now always returns empty string
          for an empty response body, so streamed responses are consistent with
          non-streamed (ether)

1.0014 Mon Dec  3 10:27:43 PST 2012
    [BUG FIXES]
        - Fixed Hash order in tests for perl 5.17 (doy)
        - Fixed StackTrace tests to run with Devel::StackTrace

    [IMPROVEMENTS]
        - Plack::Middleware::AccessLog can now log the worker pid and server
          port (ether)

1.0013  Wed Nov 14 19:46:49 PST 2012
    [BUG FIXES]
        - Make sure psgi.input is seeked even when the input is buffered (Getty, leedo)
        - Delete invalid (empty) CONTENT_LENGTH and CONTENT_TYPE in FCGI (Getty, leedo)

1.0012  Wed Nov 14 12:00:17 PST 2012
    [IMPROVEMENTS]
        - Make conditional middleware work with initialization without an app (doy)
        - Added force option to BufferedStreaming

1.0011  Sun Nov 11 11:05:30 PST 2012
    [BUG FIXES]
        - Fix bad Content-Length that could be caused with mod_perl (avar)
        - Allow an empty PATH_INFO in Lint per PSGI spec

1.0010  Fri Nov  2 13:30:50 PDT 2012
    [IMPROVEMENTS]
        - Added vim .swp files to the default ignore list in Restarter
        - Check if PATH_INFO begins with / in Lint

1.0009  Tue Oct 23 00:57:16 PDT 2012
    [BUG FIXES]
        - Correct fix to address drive letters for Win32

1.0008  Mon Oct 22 18:52:29 PDT 2012
    [BUG FIXES]
        - Allow drive letters for absolute paths for plackup and load_psgi #343

1.0007  Sat Oct 20 23:20:20 PDT 2012
    [IMPROVEMENTS]
        - Fix test failures with HTTP::Message 6.06. #345
        - relaxed plackup -R ignore files and directoris. #260

1.0006  Thu Oct 18 16:06:15 PDT 2012
    [INCOMPATIBLE CHANGES]
        - plackup foo.psgi will not search the file in @INC anymore before the current directory
          See plack/Plack#343 for details (miyagawa)

    [NEW FEATURES]
        - plackup --path /foo will mount the application under /path (mattn)

    [BUG FIXES]
        - AccessLog: Fix the timezon offset for certain timezones
        - ErrorDocument: support streaming interface

1.0005  Tue Oct  9 13:33:47 PDT 2012
    [NEW FEATURES]
        - Support psgix.cleanup handlers in Apache2 (avar)
        - Added REMOTE_PORT environment variable to HTTP::Server::PSGI (dex4er)

    [IMPROVEMENTS]
        - Documentation fix for multiple cookie values (miyagawa)
        - Delete MOD_PERL environment variable for better compatibilities (avar)
        - Split out Plack::TempBuffer as a standalone Stream::Buffered module (doy)
        - Bump Test::TCP dep

1.0004  Thu Sep 20 08:36:11 JST 2012
    [NEW FEATURES]
        - Added psgix.harakiri support in HTTP::Server::PSGI

    [IMPROVEMENTS]
        - Preload TempBuffer modules (avar)
        - Documentation fixes (autarch)
2b4dcbf
Jonathan Perkin jperkin referenced this pull request from a commit in joyent/pkgsrc
wen Update to 1.0015
Upstream changes:
1.0015 Thu Jan 10 15:19:17 PST 2013
    [BUG FIXES]
        - Fixed Lint complaining about Latin-1 range characters stored internally with
          utf8 flag on (Mark Fowler)
        - HTTP::Message::PSGI::res_from_psgi now always returns empty string
          for an empty response body, so streamed responses are consistent with
          non-streamed (ether)

1.0014 Mon Dec  3 10:27:43 PST 2012
    [BUG FIXES]
        - Fixed Hash order in tests for perl 5.17 (doy)
        - Fixed StackTrace tests to run with Devel::StackTrace

    [IMPROVEMENTS]
        - Plack::Middleware::AccessLog can now log the worker pid and server
          port (ether)

1.0013  Wed Nov 14 19:46:49 PST 2012
    [BUG FIXES]
        - Make sure psgi.input is seeked even when the input is buffered (Getty, leedo)
        - Delete invalid (empty) CONTENT_LENGTH and CONTENT_TYPE in FCGI (Getty, leedo)

1.0012  Wed Nov 14 12:00:17 PST 2012
    [IMPROVEMENTS]
        - Make conditional middleware work with initialization without an app (doy)
        - Added force option to BufferedStreaming

1.0011  Sun Nov 11 11:05:30 PST 2012
    [BUG FIXES]
        - Fix bad Content-Length that could be caused with mod_perl (avar)
        - Allow an empty PATH_INFO in Lint per PSGI spec

1.0010  Fri Nov  2 13:30:50 PDT 2012
    [IMPROVEMENTS]
        - Added vim .swp files to the default ignore list in Restarter
        - Check if PATH_INFO begins with / in Lint

1.0009  Tue Oct 23 00:57:16 PDT 2012
    [BUG FIXES]
        - Correct fix to address drive letters for Win32

1.0008  Mon Oct 22 18:52:29 PDT 2012
    [BUG FIXES]
        - Allow drive letters for absolute paths for plackup and load_psgi #343

1.0007  Sat Oct 20 23:20:20 PDT 2012
    [IMPROVEMENTS]
        - Fix test failures with HTTP::Message 6.06. #345
        - relaxed plackup -R ignore files and directoris. #260

1.0006  Thu Oct 18 16:06:15 PDT 2012
    [INCOMPATIBLE CHANGES]
        - plackup foo.psgi will not search the file in @INC anymore before the current directory
          See plack/Plack#343 for details (miyagawa)

    [NEW FEATURES]
        - plackup --path /foo will mount the application under /path (mattn)

    [BUG FIXES]
        - AccessLog: Fix the timezon offset for certain timezones
        - ErrorDocument: support streaming interface

1.0005  Tue Oct  9 13:33:47 PDT 2012
    [NEW FEATURES]
        - Support psgix.cleanup handlers in Apache2 (avar)
        - Added REMOTE_PORT environment variable to HTTP::Server::PSGI (dex4er)

    [IMPROVEMENTS]
        - Documentation fix for multiple cookie values (miyagawa)
        - Delete MOD_PERL environment variable for better compatibilities (avar)
        - Split out Plack::TempBuffer as a standalone Stream::Buffered module (doy)
        - Bump Test::TCP dep

1.0004  Thu Sep 20 08:36:11 JST 2012
    [NEW FEATURES]
        - Added psgix.harakiri support in HTTP::Server::PSGI

    [IMPROVEMENTS]
        - Preload TempBuffer modules (avar)
        - Documentation fixes (autarch)
ffc4717
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 4 unique commits by 2 authors.

Oct 16, 2012
Tatsuhiko Miyagawa miyagawa Stop load_psgi() from loading files from @INC if it's given a filenam…
…e args.

This might break users who installs `foo.psgi` into @INC directories
and expect `plackup foo.psgi` to load them from there - it's been a
mix of documented (loading class names like MyApp::PSGI) and
undocumented feature anyways.
e78bbf0
Tatsuhiko Miyagawa miyagawa Documentation 990d956
Tatsuhiko Miyagawa miyagawa Add with git add -f (inc/ matches with my gitignore) f5b8faa
Jesse Luehrs doy explicitly exclude t/Plack-Util/inc/ from gitignore f579a39
This page is out of date. Refresh to see the latest.
1  .gitignore
... ... @@ -1,6 +1,7 @@
1 1 META.yml
2 2 Makefile
3 3 inc/
  4 +!t/Plack-Util/inc/
4 5 pm_to_blib
5 6 blib
6 7 *~
27 lib/Plack/Util.pm
@@ -123,12 +123,17 @@ package Plack::Sandbox::%s;
123 123 END_EVAL
124 124 }
125 125
  126 +sub _relativize {
  127 + my $file = shift;
  128 + $file =~ m!^/! ? $file : "./$file";
  129 +}
  130 +
126 131 sub load_psgi {
127 132 my $stuff = shift;
128 133
129 134 local $ENV{PLACK_ENV} = $ENV{PLACK_ENV} || 'development';
130 135
131   - my $file = $stuff =~ /^[a-zA-Z0-9\_\:]+$/ ? class_to_file($stuff) : $stuff;
  136 + my $file = $stuff =~ /^[a-zA-Z0-9\_\:]+$/ ? class_to_file($stuff) : _relativize($stuff);
132 137 my $app = _load_sandbox($file);
133 138 die "Error while loading $file: $@" if $@;
134 139
@@ -430,19 +435,25 @@ require the file to get PSGI application handler. If the file can't be
430 435 loaded (e.g. file doesn't exist or has a perl syntax error), it will
431 436 throw an exception.
432 437
  438 +Since version 1.0006, this function would not load PSGI files from
  439 +include paths (C<@INC>) unless it looks like a class name that only
  440 +consists of C<[A-Za-z0-9_:]>. For example:
  441 +
  442 + Plack::Util::load_psgi("app.psgi"); # ./app.psgi
  443 + Plack::Util::load_psgi("/path/to/app.psgi"); # /path/to/app.psgi
  444 + Plack::Util::load_psgi("MyApp::PSGI"); # MyApp/PSGI.pm from @INC
  445 +
433 446 B<Security>: If you give this function a class name or module name
434 447 that is loadable from your system, it will load the module. This could
435 448 lead to a security hole:
436 449
437   - my $psgi = ...; # user-input: consider "Moose.pm"
438   - $app = Plack::Util::load_psgi($psgi); # this does 'require "Moose.pm"'!
  450 + my $psgi = ...; # user-input: consider "Moose"
  451 + $app = Plack::Util::load_psgi($psgi); # this would lead to 'require "Moose.pm"'!
439 452
440 453 Generally speaking, passing an external input to this function is
441   -considered very insecure. But if you really want to do that, be sure
442   -to validate the argument passed to this function. Also, if you do not
443   -want to accept an arbitrary class name but only load from a file path,
444   -make sure that the argument C<$psgi_file_or_class> begins with C</> so
445   -that Perl's built-in do function won't search the include path.
  454 +considered very insecure. If you really want to do that, validate that
  455 +a given file name contains dots (like C<foo.psgi>) and also turn it
  456 +into a full path in your caller's code.
446 457
447 458 =item run_app
448 459
1  t/Plack-Util/inc/hello.psgi
... ... @@ -0,0 +1 @@
  1 +die "Do not load this file";
15 t/Plack-Util/load.t
@@ -52,4 +52,19 @@ use Test::More;
52 52 }
53 53 }
54 54
  55 +{
  56 + require Cwd;
  57 + my $cwd = Cwd::cwd();
  58 +
  59 + chdir "t/Plack-Util";
  60 + local @INC = ("./inc", @INC);
  61 + my $app = Plack::Util::load_psgi("hello.psgi");
  62 + ok $app;
  63 + test_psgi $app, sub {
  64 + is $_[0]->(GET "/")->content, "Hello";
  65 + };
  66 +
  67 + chdir $cwd;
  68 +}
  69 +
55 70 done_testing;

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.