From ac2d146a7cb1409fd91af412cf127dc707e05fa2 Mon Sep 17 00:00:00 2001 From: Aristotle Pagaltzis Date: Sat, 16 Feb 2019 18:22:19 +0100 Subject: [PATCH 01/10] bail early on missing Accept-Encoding --- lib/Plack/Middleware/Deflater.pm | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Plack/Middleware/Deflater.pm b/lib/Plack/Middleware/Deflater.pm index 4cb0b03..5befa31 100644 --- a/lib/Plack/Middleware/Deflater.pm +++ b/lib/Plack/Middleware/Deflater.pm @@ -60,14 +60,14 @@ sub call { return if $content_type ne 'text/html'; } + return if not defined $env->{HTTP_ACCEPT_ENCODING}; + # TODO check quality my $encoding = 'identity'; - if ( defined $env->{HTTP_ACCEPT_ENCODING} ) { - for my $enc (qw(gzip deflate identity)) { - if ( $env->{HTTP_ACCEPT_ENCODING} =~ /\b$enc\b/ ) { - $encoding = $enc; - last; - } + for my $enc (qw(gzip deflate identity)) { + if ( $env->{HTTP_ACCEPT_ENCODING} =~ /\b$enc\b/ ) { + $encoding = $enc; + last; } } From 37c25639f40f3749dc11d4b08d24a5cb2dedf18d Mon Sep 17 00:00:00 2001 From: Aristotle Pagaltzis Date: Sat, 16 Feb 2019 18:24:37 +0100 Subject: [PATCH 02/10] bail early on identity encoding --- lib/Plack/Middleware/Deflater.pm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Plack/Middleware/Deflater.pm b/lib/Plack/Middleware/Deflater.pm index 5befa31..8608809 100644 --- a/lib/Plack/Middleware/Deflater.pm +++ b/lib/Plack/Middleware/Deflater.pm @@ -71,6 +71,8 @@ sub call { } } + return if $encoding eq 'identity'; + my $encoder; if ($encoding eq 'gzip' || $encoding eq 'deflate') { $encoder = Plack::Middleware::Deflater::Encoder->new($encoding); From 30fcbe1f918155a286a840e498aba78b30aa4905 Mon Sep 17 00:00:00 2001 From: Aristotle Pagaltzis Date: Sat, 16 Feb 2019 18:25:05 +0100 Subject: [PATCH 03/10] delete unreachable code Since $encoding is initialised to `identity` and only possibly assigned one of the values of the `for` loop, it can never have a value other than `gzip`, `deflate`, or `identity`. --- lib/Plack/Middleware/Deflater.pm | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/Plack/Middleware/Deflater.pm b/lib/Plack/Middleware/Deflater.pm index 8608809..c2f43ca 100644 --- a/lib/Plack/Middleware/Deflater.pm +++ b/lib/Plack/Middleware/Deflater.pm @@ -76,10 +76,6 @@ sub call { my $encoder; if ($encoding eq 'gzip' || $encoding eq 'deflate') { $encoder = Plack::Middleware::Deflater::Encoder->new($encoding); - } elsif ($encoding ne 'identity') { - my $msg = "An acceptable encoding for the requested resource is not found."; - @$res = (406, ['Content-Type' => 'text/plain'], [ $msg ]); - return; } if ($encoder) { From fda7a03e08df3b3b7381c555a1671e2f68d5e694 Mon Sep 17 00:00:00 2001 From: Aristotle Pagaltzis Date: Sat, 16 Feb 2019 18:26:55 +0100 Subject: [PATCH 04/10] delete superfluous conditional At this point, $encoding can never have a value other than `gzip` or `deflate`, so this condition will always be true. --- lib/Plack/Middleware/Deflater.pm | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/Plack/Middleware/Deflater.pm b/lib/Plack/Middleware/Deflater.pm index c2f43ca..6072a06 100644 --- a/lib/Plack/Middleware/Deflater.pm +++ b/lib/Plack/Middleware/Deflater.pm @@ -73,10 +73,7 @@ sub call { return if $encoding eq 'identity'; - my $encoder; - if ($encoding eq 'gzip' || $encoding eq 'deflate') { - $encoder = Plack::Middleware::Deflater::Encoder->new($encoding); - } + my $encoder = Plack::Middleware::Deflater::Encoder->new($encoding); if ($encoder) { $h->set('Content-Encoding' => $encoding); From 4f18296ecd1998dee2b052aee527f4eae31c4b06 Mon Sep 17 00:00:00 2001 From: Aristotle Pagaltzis Date: Sat, 16 Feb 2019 18:29:36 +0100 Subject: [PATCH 05/10] delete another superfluous conditional Since we are assigning to $encoder unconditionally, it is obviously silly to then check if we have an $encoder. --- lib/Plack/Middleware/Deflater.pm | 34 +++++++++++++++----------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/lib/Plack/Middleware/Deflater.pm b/lib/Plack/Middleware/Deflater.pm index 6072a06..5b33575 100644 --- a/lib/Plack/Middleware/Deflater.pm +++ b/lib/Plack/Middleware/Deflater.pm @@ -75,26 +75,24 @@ sub call { my $encoder = Plack::Middleware::Deflater::Encoder->new($encoding); - if ($encoder) { - $h->set('Content-Encoding' => $encoding); - $h->remove('Content-Length'); - - # normal response - if ( $res->[2] && ref($res->[2]) && ref($res->[2]) eq 'ARRAY' ) { - my $buf = ''; - foreach (@{$res->[2]}) { - $buf .= $encoder->print($_) if defined $_; - } - $buf .= $encoder->close(); - $res->[2] = [$buf]; - return; + $h->set('Content-Encoding' => $encoding); + $h->remove('Content-Length'); + + # normal response + if ( $res->[2] && ref($res->[2]) && ref($res->[2]) eq 'ARRAY' ) { + my $buf = ''; + foreach (@{$res->[2]}) { + $buf .= $encoder->print($_) if defined $_; } - - # delayed or stream - return sub { - $encoder->print(shift); - }; + $buf .= $encoder->close(); + $res->[2] = [$buf]; + return; } + + # delayed or stream + return sub { + $encoder->print(shift); + }; }); } From e89ac5c2e22e610088f7ae731baa4dde7f1c996e Mon Sep 17 00:00:00 2001 From: Aristotle Pagaltzis Date: Sat, 16 Feb 2019 18:39:32 +0100 Subject: [PATCH 06/10] simplify (and speed up) no-encoding case We always return a response, just not always an encoded one. All we really check for is, does the client accept one of the encodings we make, or does it not. So it is pointless to scan for the `identity` encoding: it is simply among the encodings we do not make. --- lib/Plack/Middleware/Deflater.pm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Plack/Middleware/Deflater.pm b/lib/Plack/Middleware/Deflater.pm index 5b33575..72b1c16 100644 --- a/lib/Plack/Middleware/Deflater.pm +++ b/lib/Plack/Middleware/Deflater.pm @@ -63,15 +63,15 @@ sub call { return if not defined $env->{HTTP_ACCEPT_ENCODING}; # TODO check quality - my $encoding = 'identity'; - for my $enc (qw(gzip deflate identity)) { + my $encoding; + for my $enc (qw(gzip deflate)) { if ( $env->{HTTP_ACCEPT_ENCODING} =~ /\b$enc\b/ ) { $encoding = $enc; last; } } - return if $encoding eq 'identity'; + return if not $encoding; my $encoder = Plack::Middleware::Deflater::Encoder->new($encoding); From a82bdce4e2151cd37c36ae5e9d092a472f5b4768 Mon Sep 17 00:00:00 2001 From: Aristotle Pagaltzis Date: Sat, 16 Feb 2019 18:47:34 +0100 Subject: [PATCH 07/10] delete redundant condition subexpressions The `ref` function handle undefined input and always produces a defined output so it is a waste of time to work up to the final check by testing each component subexpression first. --- lib/Plack/Middleware/Deflater.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Plack/Middleware/Deflater.pm b/lib/Plack/Middleware/Deflater.pm index 72b1c16..3907f65 100644 --- a/lib/Plack/Middleware/Deflater.pm +++ b/lib/Plack/Middleware/Deflater.pm @@ -79,7 +79,7 @@ sub call { $h->remove('Content-Length'); # normal response - if ( $res->[2] && ref($res->[2]) && ref($res->[2]) eq 'ARRAY' ) { + if ( ref($res->[2]) eq 'ARRAY' ) { my $buf = ''; foreach (@{$res->[2]}) { $buf .= $encoder->print($_) if defined $_; From 10e6ab9392469a35ef4631d32298fca377faa366 Mon Sep 17 00:00:00 2001 From: Aristotle Pagaltzis Date: Sat, 16 Feb 2019 18:59:09 +0100 Subject: [PATCH 08/10] handle multiple Cache-Control response headers --- lib/Plack/Middleware/Deflater.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Plack/Middleware/Deflater.pm b/lib/Plack/Middleware/Deflater.pm index 3907f65..e2c41c6 100644 --- a/lib/Plack/Middleware/Deflater.pm +++ b/lib/Plack/Middleware/Deflater.pm @@ -42,7 +42,7 @@ sub call { } if (Plack::Util::status_with_no_entity_body($res->[0]) or - $h->exists('Cache-Control') && $h->get('Cache-Control') =~ /\bno-transform\b/) { + grep /\bno-transform\b/, $h->get('Cache-Control') ) { return; } From d27019fbcc382b5db9b4e95edc252eb6c8374521 Mon Sep 17 00:00:00 2001 From: Aristotle Pagaltzis Date: Sat, 16 Feb 2019 18:59:28 +0100 Subject: [PATCH 09/10] handle multiple Vary response headers --- lib/Plack/Middleware/Deflater.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Plack/Middleware/Deflater.pm b/lib/Plack/Middleware/Deflater.pm index e2c41c6..c729937 100644 --- a/lib/Plack/Middleware/Deflater.pm +++ b/lib/Plack/Middleware/Deflater.pm @@ -46,7 +46,7 @@ sub call { return; } - my @vary = split /\s*,\s*/, ($h->get('Vary') || ''); + my @vary = map +( split /\s*,\s*/, $_ ), $h->get('Vary'); push @vary, 'Accept-Encoding'; push @vary, 'User-Agent' if $self->vary_user_agent; $h->set('Vary' => join(",", @vary)); From fd6aa09c9180fe01f4c7375a62b73ac7c8d90f9e Mon Sep 17 00:00:00 2001 From: Aristotle Pagaltzis Date: Sat, 16 Feb 2019 22:24:10 +0100 Subject: [PATCH 10/10] combine Accept-Encoding checks with no loop --- lib/Plack/Middleware/Deflater.pm | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/lib/Plack/Middleware/Deflater.pm b/lib/Plack/Middleware/Deflater.pm index c729937..09850c7 100644 --- a/lib/Plack/Middleware/Deflater.pm +++ b/lib/Plack/Middleware/Deflater.pm @@ -60,18 +60,10 @@ sub call { return if $content_type ne 'text/html'; } - return if not defined $env->{HTTP_ACCEPT_ENCODING}; - # TODO check quality - my $encoding; - for my $enc (qw(gzip deflate)) { - if ( $env->{HTTP_ACCEPT_ENCODING} =~ /\b$enc\b/ ) { - $encoding = $enc; - last; - } - } - - return if not $encoding; + my $ae = $env->{HTTP_ACCEPT_ENCODING} || return; + my $encoding = ($ae =~ /\b(gzip)\b/ || $ae =~ /\b(deflate)\b/) + ? $1 : return; my $encoder = Plack::Middleware::Deflater::Encoder->new($encoding);