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 B6C031F46C for ; Wed, 18 Dec 2019 03:36:46 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 2/6] viewvcs: flesh out some functionality and test Date: Wed, 18 Dec 2019 03:36:41 +0000 Message-Id: <20191218033645.5037-3-e@80x24.org> In-Reply-To: <20191218033645.5037-1-e@80x24.org> References: <20191218033645.5037-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Expose MAX_SIZE via "our" will make it possible to use in tests, and configure, later. Additionally, returning HTTP 500 code for big files is not an Internal Server Error, just a memory limit... Some browsers won't show our HTML response with the link to the raw file in case of errors, either, so we'll return 200 to ensure users can use the link to access the raw blob. Finally, throw in some tests to the existing solver_git testcase, since that was incomplete and was pointlessly loading Plack modules without testing PSGI. --- lib/PublicInbox/ViewVCS.pm | 8 ++-- t/solver_git.t | 77 +++++++++++++++++++++++++++++++++++--- 2 files changed, 76 insertions(+), 9 deletions(-) diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm index 369afe93..842c873c 100644 --- a/lib/PublicInbox/ViewVCS.pm +++ b/lib/PublicInbox/ViewVCS.pm @@ -28,7 +28,7 @@ my $hl = eval { }; my %QP_MAP = ( A => 'oid_a', B => 'oid_b', a => 'path_a', b => 'path_b' ); -my $max_size = 1024 * 1024; # TODO: configurable +our $MAX_SIZE = 1024 * 1024; # TODO: configurable my $BIN_DETECT = 8000; # same as git sub html_page ($$$) { @@ -76,7 +76,7 @@ sub stream_large_blob ($$$$) { sub show_other ($$$$) { my ($ctx, $res, $logref, $fn) = @_; my ($git, $oid, $type, $size) = @$res; - if ($size > $max_size) { + if ($size > $MAX_SIZE) { $$logref = "$oid is too big to show\n" . $$logref; return html_page($ctx, 200, $logref); } @@ -122,11 +122,11 @@ sub solve_result { return show_other($ctx, $res, \$log, $fn) if $type ne 'blob'; my $path = to_filename($di->{path_b} || $hints->{path_b} || 'blob'); my $raw_link = "(raw)"; - if ($size > $max_size) { + if ($size > $MAX_SIZE) { return stream_large_blob($ctx, $res, \$log, $fn) if defined $fn; $log = "
Too big to show, download available\n" .
 			"$oid $type $size bytes $raw_link
" . $log; - return html_page($ctx, 500, \$log); + return html_page($ctx, 200, \$log); } my $blob = $git->cat_file($oid); diff --git a/t/solver_git.t b/t/solver_git.t index 9bda157d..6bac17ea 100644 --- a/t/solver_git.t +++ b/t/solver_git.t @@ -6,9 +6,9 @@ use Test::More; use Cwd qw(abs_path); require './t/common.perl'; require_git(2.6); +use PublicInbox::Spawn qw(spawn); -my @mods = qw(DBD::SQLite Search::Xapian HTTP::Request::Common Plack::Test - URI::Escape Plack::Builder); +my @mods = qw(DBD::SQLite Search::Xapian); foreach my $mod (@mods) { eval "require $mod"; plan skip_all => "$mod missing for $0" if $@; @@ -19,7 +19,7 @@ plan skip_all => "$0 must be run from a git working tree" if $?; # needed for alternates, and --absolute-git-dir is only in git 2.13+ $git_dir = abs_path($git_dir); -use_ok "PublicInbox::$_" for (qw(Inbox V2Writable MIME Git SolverGit)); +use_ok "PublicInbox::$_" for (qw(Inbox V2Writable MIME Git SolverGit WWW)); my ($inboxdir, $for_destroy) = tmpdir(); my $opts = { @@ -40,10 +40,10 @@ sub deliver_patch ($) { } deliver_patch('t/solve/0001-simple-mod.patch'); - +my $v1_0_0_tag = 'cb7c42b1e15577ed2215356a2bf925aef59cdd8d'; my $git = PublicInbox::Git->new($git_dir); is('public-inbox 1.0.0', - $git->commit_title('cb7c42b1e15577ed2215356a2bf925aef59cdd8d'), + $git->commit_title($v1_0_0_tag), 'commit_title works on 1.0.0'); is(undef, $git->commit_title('impossible'), 'undef on impossible object'); @@ -113,4 +113,71 @@ my $hinted = $res; shift @$res; shift @$hinted; is_deeply($res, $hinted, 'hints work (or did not hurt :P'); +my @psgi = qw(HTTP::Request::Common Plack::Test URI::Escape Plack::Builder); +SKIP: { + my @missing; + for my $mod (@psgi) { + eval("require $mod") or push(@missing, $mod); + } + skip("missing: ".join(', ', @missing), 7 + scalar(@psgi)) if @missing; + use_ok($_) for @psgi; + my $binfoo = "$inboxdir/binfoo.git"; + system(qw(git init --bare -q), $binfoo) == 0 or die "git init: $?"; + require_ok 'PublicInbox::ViewVCS'; + my $big_size = do { + no warnings 'once'; + $PublicInbox::ViewVCS::MAX_SIZE + 1; + }; + my %bin = (big => $big_size, small => 1); + my %oid; # (small|big) => OID + my $cmd = [ qw(git hash-object -w --stdin) ]; + my $env = { GIT_DIR => $binfoo }; + while (my ($label, $size) = each %bin) { + pipe(my ($rout, $wout)) or die; + pipe(my ($rin, $win)) or die; + my $rdr = { 0 => fileno($rin), 1 => fileno($wout) }; + my $pid = spawn($cmd , $env, $rdr); + $wout = $rin = undef; + print { $win } ("\0" x $size) or die; + close $win or die; + chomp($oid{$label} = <$rout>); + } + + # ensure the PSGI frontend (ViewVCS) works: + my $name = $ibx->{name}; + my $cfgpfx = "publicinbox.$name"; + my $cfg = PublicInbox::Config->new(\<{address}; +$cfgpfx.inboxdir=$inboxdir +$cfgpfx.coderepo=public-inbox +$cfgpfx.coderepo=binfoo +coderepo.public-inbox.dir=$git_dir +coderepo.public-inbox.cgiturl=http://example.com/public-inbox +coderepo.binfoo.dir=$binfoo +coderepo.binfoo.cgiturl=http://example.com/binfoo +EOF + my $www = PublicInbox::WWW->new($cfg); + test_psgi(sub { $www->call(@_) }, sub { + my ($cb) = @_; + my $res = $cb->(GET("/$name/3435775/s/")); + is($res->code, 200, 'success with existing blob'); + + $res = $cb->(GET("/$name/".('0'x40).'/s/')); + is($res->code, 404, 'failure with null OID'); + + $res = $cb->(GET("/$name/$v1_0_0_tag/s/")); + is($res->code, 200, 'shows commit'); + while (my ($label, $size) = each %bin) { + $res = $cb->(GET("/$name/$oid{$label}/s/")); + is($res->code, 200, "$label binary file"); + ok(index($res->content, "blob $size bytes") >= 0, + "showed $label binary blob size"); + $res = $cb->(GET("/$name/$oid{$label}/s/raw")); + is($res->code, 200, "$label raw binary download"); + is($res->content, "\0" x $size, + "$label content matches"); + } + }); +} + done_testing();