From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 632AA1F47A for ; Wed, 1 Jan 2020 10:39:00 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 3/6] wwwstatic: move r(...) functions here Date: Wed, 1 Jan 2020 10:38:56 +0000 Message-Id: <20200101103859.15401-4-e@80x24.org> In-Reply-To: <20200101103859.15401-1-e@80x24.org> References: <20200101103859.15401-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Remove redundant "r" functions for generating short error responses. These responses will no longer be cached by clients, which is probably a good thing since most errors ought to be transient, anyways. This also fixes error responses for our cgit wrapper when static files are missing. --- lib/PublicInbox/Cgit.pm | 3 +-- lib/PublicInbox/GitHTTPBackend.pm | 19 +++---------------- lib/PublicInbox/WWW.pm | 8 +++----- lib/PublicInbox/WwwHighlight.pm | 9 +-------- lib/PublicInbox/WwwStatic.pm | 28 ++++++++++++++++++++++------ 5 files changed, 30 insertions(+), 37 deletions(-) diff --git a/lib/PublicInbox/Cgit.pm b/lib/PublicInbox/Cgit.pm index 36239438..c0b1a73b 100644 --- a/lib/PublicInbox/Cgit.pm +++ b/lib/PublicInbox/Cgit.pm @@ -10,13 +10,12 @@ use strict; use PublicInbox::GitHTTPBackend; use PublicInbox::Git; # not bothering with Exporter for a one-off -*r = *PublicInbox::GitHTTPBackend::r; *input_prepare = *PublicInbox::GitHTTPBackend::input_prepare; *parse_cgi_headers = *PublicInbox::GitHTTPBackend::parse_cgi_headers; *serve = *PublicInbox::GitHTTPBackend::serve; use warnings; use PublicInbox::Qspawn; -use PublicInbox::WwwStatic; +use PublicInbox::WwwStatic qw(r); use Plack::MIME; sub locate_cgit ($) { diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm index 8883ec34..d1132fb7 100644 --- a/lib/PublicInbox/GitHTTPBackend.pm +++ b/lib/PublicInbox/GitHTTPBackend.pm @@ -9,10 +9,9 @@ use warnings; use Fcntl qw(:seek); use IO::Handle; use HTTP::Date qw(time2str); -use HTTP::Status qw(status_message); use PublicInbox::Qspawn; use PublicInbox::Tmpfile; -use PublicInbox::WwwStatic; +use PublicInbox::WwwStatic qw(r @NO_CACHE); # 32 is same as the git-daemon connection limit my $default_limiter = PublicInbox::Qspawn::Limiter->new(32); @@ -32,18 +31,6 @@ our $ANY = join('|', @binary, @text, 'git-upload-pack'); my $BIN = join('|', @binary); my $TEXT = join('|', @text); -my @no_cache = ('Expires', 'Fri, 01 Jan 1980 00:00:00 GMT', - 'Pragma', 'no-cache', - 'Cache-Control', 'no-cache, max-age=0, must-revalidate'); - -sub r ($;$) { - my ($code, $msg) = @_; - $msg ||= status_message($code); - my $len = length($msg); - [ $code, [qw(Content-Type text/plain Content-Length), $len, @no_cache], - [$msg] ] -} - sub serve { my ($env, $git, $path) = @_; @@ -88,12 +75,12 @@ sub serve_dumb { cache_one_year($h); } elsif ($path =~ /\A(?:$TEXT)\z/o) { $type = 'text/plain'; - push @$h, @no_cache; + push @$h, @NO_CACHE; } else { return r(404); } $path = "$git->{git_dir}/$path"; - PublicInbox::WwwStatic::response($env, $h, $path, $type) // r(404); + PublicInbox::WwwStatic::response($env, $h, $path, $type); } sub git_parse_hdr { # {parse_hdr} for Qspawn diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm index 13b66ee6..99f9f1dc 100644 --- a/lib/PublicInbox/WWW.pm +++ b/lib/PublicInbox/WWW.pm @@ -22,6 +22,7 @@ use PublicInbox::MID qw(mid_escape); require PublicInbox::Git; use PublicInbox::GitHTTPBackend; use PublicInbox::UserContent; +use PublicInbox::WwwStatic qw(r); # TODO: consider a routing tree now that we have more endpoints: our $INBOX_RE = qr!\A/([\w\-][\w\.\-]*)!; @@ -83,7 +84,7 @@ sub call { } } elsif ($method !~ /\AGET|HEAD\z/) { - return r(405, 'Method Not Allowed'); + return r(405); } # top-level indices and feeds @@ -176,12 +177,9 @@ sub r404 { require PublicInbox::ExtMsg; return PublicInbox::ExtMsg::ext_msg($ctx); } - r(404, 'Not Found'); + r(404); } -# simple response for errors -sub r { [ $_[0], ['Content-Type' => 'text/plain'], [ join(' ', @_, "\n") ] ] } - sub news_cgit_fallback ($) { my ($ctx) = @_; my $www = $ctx->{www}; diff --git a/lib/PublicInbox/WwwHighlight.pm b/lib/PublicInbox/WwwHighlight.pm index bc349f8a..6312edae 100644 --- a/lib/PublicInbox/WwwHighlight.pm +++ b/lib/PublicInbox/WwwHighlight.pm @@ -22,22 +22,15 @@ package PublicInbox::WwwHighlight; use strict; use warnings; use bytes (); # only for bytes::length -use HTTP::Status qw(status_message); use parent qw(PublicInbox::HlMod); use PublicInbox::Linkify qw(); use PublicInbox::Hval qw(ascii_html); +use PublicInbox::WwwStatic qw(r); # TODO: support highlight(1) for distros which don't package the # SWIG extension. Also, there may be admins who don't want to # have ugly SWIG-generated code in a long-lived Perl process. -sub r ($) { - my ($code) = @_; - my $msg = status_message($code); - my $len = length($msg); - [ $code, [qw(Content-Type text/plain Content-Length), $len], [$msg] ] -} - # another slurp API hogging up all my memory :< # This is capped by whatever the PSGI server allows, # $ENV{GIT_HTTP_MAX_REQUEST_BUFFER} for PublicInbox::HTTP (10 MB) diff --git a/lib/PublicInbox/WwwStatic.pm b/lib/PublicInbox/WwwStatic.pm index b8efcf62..c605e64f 100644 --- a/lib/PublicInbox/WwwStatic.pm +++ b/lib/PublicInbox/WwwStatic.pm @@ -3,8 +3,23 @@ package PublicInbox::WwwStatic; use strict; +use parent qw(Exporter); use Fcntl qw(:seek); use HTTP::Date qw(time2str); +use HTTP::Status qw(status_message); +our @EXPORT_OK = qw(@NO_CACHE r); + +our @NO_CACHE = ('Expires', 'Fri, 01 Jan 1980 00:00:00 GMT', + 'Pragma', 'no-cache', + 'Cache-Control', 'no-cache, max-age=0, must-revalidate'); + +sub r ($;$) { + my ($code, $msg) = @_; + $msg ||= status_message($code); + [ $code, [ qw(Content-Type text/plain), 'Content-Length', length($msg), + @NO_CACHE ], + [ $msg ] ] +} sub prepare_range { my ($env, $in, $h, $beg, $end, $size) = @_; @@ -36,7 +51,7 @@ sub prepare_range { if ($len <= 0) { $code = 416; } else { - sysseek($in, $beg, SEEK_SET) or return [ 500, [], [] ]; + sysseek($in, $beg, SEEK_SET) or return r(500); push @$h, qw(Accept-Ranges bytes Content-Range); push @$h, "bytes $beg-$end/$size"; @@ -44,12 +59,16 @@ sub prepare_range { $env->{'psgix.no-compress'} = 1; } } + if ($code == 416) { + push @$h, 'Content-Range', "bytes */$size"; + return [ 416, $h, [] ]; + } ($code, $len); } sub response { my ($env, $h, $path, $type) = @_; - return unless -f $path && -r _; # just in case it's a FIFO :P + return r(404) unless -f $path && -r _; # just in case it's a FIFO :P open my $in, '<', $path or return; my $size = -s $in; @@ -64,10 +83,7 @@ sub response { push @$h, 'Content-Type', $type; if (($env->{HTTP_RANGE} || '') =~ /\bbytes=([0-9]*)-([0-9]*)\z/) { ($code, $len) = prepare_range($env, $in, $h, $1, $2, $size); - if ($code == 416) { - push @$h, 'Content-Range', "bytes */$size"; - return [ 416, $h, [] ]; - } + return $code if ref($code); } push @$h, 'Content-Length', $len, 'Last-Modified', $mtime; my $body = bless {