Browse Source

Log a 416 only for the request that gets one

serve_ranges() wrote "GET 416" to the log for each range before it checked
the range, so every satisfiable range request was logged as refused too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
troglodyne-bot 1 week ago
parent
commit
995e250fd3
2 changed files with 26 additions and 2 deletions
  1. 4 2
      lib/TPSGI.pm
  2. 22 0
      t/TPSGI.t

+ 4 - 2
lib/TPSGI.pm

@@ -474,8 +474,10 @@ sub serve_ranges {
     # Calculate the content-length up-front.  We have to fix unspecified lengths first, and reject bad requests.
     foreach my $range (@$ranges) {
         $range->[1] //= $sz - 1;
-        $self->INFO("GET 416 $fullpath");
-        return [ 416, [%headers], ["Requested range not satisfiable"] ] if $range->[0] > $sz || $range->[0] < 0 || $range->[1] < 0 || $range->[0] > $range->[1];
+        if ( $range->[0] > $sz || $range->[0] < 0 || $range->[1] < 0 || $range->[0] > $range->[1] ) {
+            $self->INFO("GET 416 $fullpath");
+            return [ 416, [%headers], ["Requested range not satisfiable"] ];
+        }
     }
     $headers{'Content-Length'} = List::Util::sum( map { $_->[1] + 1 - $_->[0] } @$ranges );
 

+ 22 - 0
t/TPSGI.t

@@ -7,6 +7,7 @@ use Test2::V0;
 use Test2::Plugin::NoWarnings;
 
 use File::Temp qw{tempdir};
+use Mojo::File ();
 use Test::MockModule;
 
 use FindBin::libs;
@@ -119,6 +120,27 @@ subtest 'app, when the request dies' => sub {
     like( \@Test::Logger::Capture::lines, bag { item qr/Request[ ]failed:[ ]Can't[ ]locate[ ]Bogus\/Dep\.pm/xs; etc() }, 'and so does the log, not just that a 500 happened' );
 };
 
+subtest 'serve_ranges' => sub {
+    local @Test::Logger::Capture::lines;
+    my $tpsgi = bless( { %options, ip => '0.0.0.0' }, 'TPSGI' );
+
+    my $file = "$dir/ten-bytes";
+    Mojo::File->new($file)->spew('0123456789');
+    my %args = ( size => 10, headers => { 'Content-type' => 'text/plain' }, fullpath => '/ten-bytes' );
+
+    open( my $fh, '<', $file ) or die "Cannot read $file: $!";
+    my $res = $tpsgi->serve_ranges( $fh, [ [ 0, 4 ] ], %args );
+    is( ref $res,                                                 'CODE', 'a range inside the file is streamed' );
+    is( [ grep { /GET[ ]416/xs } @Test::Logger::Capture::lines ], [],     'and not logged as a 416' );
+    close($fh) or die "Cannot close: $!";
+
+    open( $fh, '<', $file ) or die "Cannot read $file: $!";
+    $res = $tpsgi->serve_ranges( $fh, [ [ 0, 4 ], [ 20, 25 ] ], %args );
+    is( $res->[0], 416, 'a range past the end is refused' );
+    like( \@Test::Logger::Capture::lines, bag { item qr/GET[ ]416[ ]\/ten-bytes/xs; etc() }, 'and logged as a 416' );
+    close($fh) or die "Cannot close: $!";
+};
+
 # A router's pattern is matched as the router wrote it, whatever flags this
 # distribution's own regexes carry.
 subtest 'extract_query' => sub {