Skip to content

Commit

Permalink
Use die() instead of croak() since we don't need an extra stack for R…
Browse files Browse the repository at this point in the history
…EPL etc. and caller is not a useful information anyway.

Added a note for DEBUGGING to use REPL to investigate response values.
  • Loading branch information
miyagawa committed Jun 8, 2011
1 parent 86d1a70 commit 6aaea65
Showing 1 changed file with 53 additions and 30 deletions.
83 changes: 53 additions & 30 deletions lib/Plack/Middleware/Lint.pm
Expand Up @@ -10,7 +10,7 @@ sub wrap {
my($self, $app) = @_;

unless (ref $app eq 'CODE' or overload::Method($app, '&{}')) {
Carp::croak("PSGI app should be a code reference: ", (defined $app ? $app : "undef"));
die("PSGI app should be a code reference: ", (defined $app ? $app : "undef"));
}

$self->SUPER::wrap($app);
Expand All @@ -28,58 +28,58 @@ sub call {
sub validate_env {
my ($self, $env) = @_;
unless ($env->{'REQUEST_METHOD'}) {
Carp::croak('Missing env param: REQUEST_METHOD');
die('Missing env param: REQUEST_METHOD');
}
unless ($env->{'REQUEST_METHOD'} =~ /^[A-Z]+$/) {
Carp::croak("Invalid env param: REQUEST_METHOD($env->{REQUEST_METHOD})");
die("Invalid env param: REQUEST_METHOD($env->{REQUEST_METHOD})");
}
unless (defined($env->{'SCRIPT_NAME'})) { # allows empty string
Carp::croak('Missing mandatory env param: SCRIPT_NAME');
die('Missing mandatory env param: SCRIPT_NAME');
}
unless (defined($env->{'PATH_INFO'})) { # allows empty string
Carp::croak('Missing mandatory env param: PATH_INFO');
die('Missing mandatory env param: PATH_INFO');
}
unless (defined($env->{'SERVER_NAME'})) {
Carp::croak('Missing mandatory env param: SERVER_NAME');
die('Missing mandatory env param: SERVER_NAME');
}
unless ($env->{'SERVER_NAME'} ne '') {
Carp::croak('SERVER_NAME must not be empty string');
die('SERVER_NAME must not be empty string');
}
unless (defined($env->{'SERVER_PORT'})) {
Carp::croak('Missing mandatory env param: SERVER_PORT');
die('Missing mandatory env param: SERVER_PORT');
}
unless ($env->{'SERVER_PORT'} ne '') {
Carp::croak('SERVER_PORT must not be empty string');
die('SERVER_PORT must not be empty string');
}
unless (!defined($env->{'SERVER_PROTOCOL'}) || $env->{'SERVER_PROTOCOL'} =~ m{^HTTP/1.\d$}) {
Carp::croak('Invalid SERVER_PROTOCOL');
die('Invalid SERVER_PROTOCOL');
}
for my $param (qw/version url_scheme input errors multithread multiprocess/) {
unless (exists $env->{"psgi.$param"}) {
Carp::croak("Missing psgi.$param");
die("Missing psgi.$param");
}
}
unless (ref($env->{'psgi.version'}) eq 'ARRAY') {
Carp::croak('psgi.version should be ArrayRef');
die('psgi.version should be ArrayRef');
}
unless (scalar(@{$env->{'psgi.version'}}) == 2) {
Carp::croak('psgi.version should contain 2 elements');
die('psgi.version should contain 2 elements');
}
unless ($env->{'psgi.url_scheme'} =~ /^https?$/) {
Carp::croak('psgi.version should be "http" or "https"');
die('psgi.version should be "http" or "https"');
}
if ($env->{"psgi.version"}->[1] == 1) { # 1.1
for my $param (qw(streaming nonblocking run_once)) {
unless (exists $env->{"psgi.$param"}) {
Carp::croak("Missing psgi.$param");
die("Missing psgi.$param");
}
}
}
if ($env->{HTTP_CONTENT_TYPE}) {
Carp::croak('HTTP_CONTENT_TYPE should not exist');
die('HTTP_CONTENT_TYPE should not exist');
}
if ($env->{HTTP_CONTENT_LENGTH}) {
Carp::croak('HTTP_CONTENT_LENGTH should not exist');
die('HTTP_CONTENT_LENGTH should not exist');
}
}

Expand All @@ -94,45 +94,43 @@ sub is_possibly_fh {
sub validate_res {
my ($self, $res, $streaming) = @_;

my $croak = $streaming ? \&Carp::confess : \&Carp::croak;

unless (ref($res) and ref($res) eq 'ARRAY' || ref($res) eq 'CODE') {
$croak->('Response should be array ref or code ref');
die('Response should be array ref or code ref');
}

if (ref $res eq 'CODE') {
return $self->response_cb($res, sub { $self->validate_res(@_, 1) });
}

unless (@$res == 3 || ($streaming && @$res == 2)) {
$croak->('Response needs to be 3 element array, or 2 element in streaming');
die('Response needs to be 3 element array, or 2 element in streaming');
}

unless ($res->[0] =~ /^\d+$/ && $res->[0] >= 100) {
$croak->('Status code needs to be an integer greater than or equal to 100');
die('Status code needs to be an integer greater than or equal to 100x');
}

unless (ref $res->[1] eq 'ARRAY') {
$croak->('Headers needs to be an array ref');
die('Headers needs to be an array ref');
}

my @copy = @{$res->[1]};
unless (@copy % 2 == 0) {
$croak->('The number of response headers needs to be even, not odd');
die('The number of response headers needs to be even, not odd');
}

while(my($key, $val) = splice(@copy, 0, 2)) {
if (lc $key eq 'status') {
$croak->('Response headers MUST NOT contain a key named Status');
die('Response headers MUST NOT contain a key named Status');
}
if ($key =~ /[:\r\n]|[-_]$/) {
$croak->('Response headers MUST NOT contain a key with : or newlines, or that end in - or _');
die('Response headers MUST NOT contain a key with : or newlines, or that end in - or _');
}
unless ($key =~ /^[a-zA-Z][0-9a-zA-Z\-_]*$/) {
$croak->('Response headers MUST consist only of letters, digits, _ or - and MUST start with a letter');
die('Response headers MUST consist only of letters, digits, _ or - and MUST start with a letter');
}
if ($val =~ /[\000-\037]/) {
$croak->('Response headers MUST NOT contain characters below octal \037');
die('Response headers MUST NOT contain characters below octal \037');
}
}

Expand All @@ -142,11 +140,11 @@ sub validate_res {
Plack::Util::is_real_fh($res->[2]) ||
is_possibly_fh($res->[2]) ||
(blessed($res->[2]) && $res->[2]->can('getline'))) {
$croak->('Body should be an array ref or filehandle');
die('Body should be an array ref or filehandle');
}

if (ref $res->[2] eq 'ARRAY' && grep _is_really_utf8($_), @{$res->[2]}) {
$croak->('Body must be bytes and should not contain wide characters (UTF-8 strings).');
die('Body must be bytes and should not contain wide characters (UTF-8 strings).');
}

return $res;
Expand Down Expand Up @@ -187,6 +185,31 @@ web server that implements the PSGI interface.
This middleware is enabled by default when you run plackup or other
launcher tools with the default environment I<development> value.
=head1 DEBUGGING
Because of how this middleware works, it may not be easy to debug Lint
errors when you encounter one, unless you're writing a PSGI web server
or a framework.
For example, when you're an application developer (user of some
framework) and see errors like:
Body should be an array ref or filehandle at lib/Plack/Middleware/Lint.pm line XXXX
there's no clue about which line of I<your application> produces that
error.
We're aware of the issue, and have a plan to spit out more helpful
errors to diagnose the issue. But until then, currently there are some
workarounds to make this easier. For now, the easiest one would be to
enable L<Plack::Middleware::REPL> outside of the Lint middleware,
like:
plackup -e 'enable "REPL"; enable "Lint"' app.psgi
so that the Lint errors are caught by the REPL shell, where you can
inspect all the variables in the response.
=head1 AUTHOR
Tatsuhiko Miyagawa
Expand Down

0 comments on commit 6aaea65

Please sign in to comment.