Invalid REQUEST_METHOD should not die but return 405 METHOD NOT ALLOWED

[mrdvt92]

https://github.com/plack/Plack/blob/7d4011216825529dadbbb304afd2477fcc4b28e3/lib/Plack/Middleware/Lint.pm#L33

With curl I can pass an empty string as well as a lower case method so the default app should not die if a client can create it.

$ curl -vvv -i -X '' 'http://127.0.0.1:5000/'
$ curl -vvv -i -X lower 'http://127.0.0.1:5000/'
$ curl -vvv -i -X 0 'http://127.0.0.1:5000/'

I think this should work and would be a better default behavior.

diff --git a/lib/Plack/Middleware/Lint.pm b/lib/Plack/Middleware/Lint.pm
index eb3deee..98a7f97 100644
--- a/lib/Plack/Middleware/Lint.pm
+++ b/lib/Plack/Middleware/Lint.pm
@@ -28,10 +28,10 @@ sub call {
 sub validate_env {
     my ($self, $env) = @_;
     unless ($env->{REQUEST_METHOD}) {
-        die('Missing env param: REQUEST_METHOD');
+        return [405 => ['Content-Type' => 'text/plain'] => ['Method Not Allowed']];
     }
     unless ($env->{REQUEST_METHOD} =~ /^[A-Z]+$/) {
-        die("Invalid env param: REQUEST_METHOD($env->{REQUEST_METHOD})");
+        return [405 => ['Content-Type' => 'text/plain'] => ['Method Not Allowed']];
     }
     unless (defined($env->{SCRIPT_NAME})) { # allows empty string
         die('Missing mandatory env param: SCRIPT_NAME');

[miyagawa]

PSGI specification requires REQUEST_METHOD to be set, so empty string should be disallowed by the server. The purpose of the Lint middleware is to catch implementation problems like this in the development environment.

I assume many existing PSGI server implementation may not implement this properly, at least for lower-case request method, and that suggests to me that it can actually be patched by writing another piece of middleware. Handling this as 405 in Lint doesn't sound right because this middleware is not meant to be enabled in production, and that'd cause these requests will be handled differently in dev vs production.

[mrdvt92]

to catch implementation problems like this in the development environment

That what I needed. Searching the code base a bit.

    $ENV{PLACK_ENV} ||= $self->{env} || 'development';
    if ($ENV{PLACK_ENV} eq 'development') {
        $app = $self->prepare_devel($app);
    }

So, PLACK_ENV is REQUIRED to to be set in a production environment or the Lint logic defaults to 'development' and will expose the architecture of the underlying web services. I think this is the actual bug!

So, I can call my environments something besides 'development' and die in my code if PLACK_ENV is not set. I still think it is bad form to have a different behavior between different environments.

[miyagawa]

You can add --no-default-middleware to plackup and the Lint middleware is not set.

That what I needed. Searching the code base a bit.

You don't need to search for a code. It's documented in perldoc plackup:

    -E, --env, the "PLACK_ENV" environment variable.
        Specifies the environment option. Setting this value with "-E" or
        "--env" also writes to the "PLACK_ENV" environment variable. This
        allows applications or frameworks to tell which environment setting
        the application is running on.

          # These two are the same
          plackup -E deployment
          env PLACK_ENV=deployment plackup

        Common values are "development", "deployment", and "test". The
        default value is "development", which causes "plackup" to load the
        middleware components: *AccessLog*, *StackTrace*, and *Lint* unless
        "--no-default-middleware" is set.

    --no-default-middleware
        This prevents loading the default middleware stack even when Plack
        environment (i.e. "-E" or "PLACK_ENV") is set to "development".