Fix URLs for needles in subdirectories (POO #58959)

As discussed in the POO, this was broken by PR #2410 commit
36aa974 - it assumes you can always find a needle simply by
sticking the needle filename on the end of a `needledir` call,
but you can't, needles are allowed to be in subdirectories of
needledir. This should hopefully fix that without breaking
the custom run case by using the *whole* of the JSON file path -
we just figure out the subdirectory component from it. This works
for me in the 'needle is in a subdirectory of the normal needle
dir' case, but I didn't test it in the custom run case.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

lib/OpenQA/Utils.pm

@@ -158,9 +158,9 @@ sub testcasedir {
 sub is_in_tests {
     my ($file) = @_;
 
-    $file = File::Spec->rel2abs($file);
+    $file = abs_path($file) // '';
     # at least tests use a relative $prjdir, so it needs to be converted to absolute path as well
-    my $abs_projdir = File::Spec->rel2abs($prjdir);
+    my $abs_projdir = abs_path($prjdir);
     return index($file, catdir($abs_projdir, 'share', 'tests')) == 0
       || index($file, catdir($abs_projdir, 'tests')) == 0;
 }

lib/OpenQA/WebAPI/Controller/File.pm

@@ -35,15 +35,34 @@ sub needle {
     my $version  = $self->param('version') || '';
     my $jsonfile = $self->param('jsonfile') || '';
 
+    # locate the needle in the needle directory for the given distri and version
+    my $needledir = needledir($distri, $version);
+
     # make sure the directory of the file parameter is a real subdir of testcasedir before
-    # applying it as needledir to prevent access outside of the zoo
+    # using it to find needle subdirectory, to prevent access outside of the zoo
     if ($jsonfile && !is_in_tests($jsonfile)) {
         warn "$jsonfile is not in a subdir of $prjdir/share/tests or $prjdir/tests";
         return $self->render(text => 'Forbidden', status => 403);
     }
-
-    # locate the needle in the needle directory for the given distri and version
-    push(@{($self->{static} = Mojolicious::Static->new)->paths}, needledir($distri, $version));
+    # we need to handle the needle being in a subdirectory - we cannot assume it is always just
+    # '$needledir/$name.$format'. figure out subdirectory elements from the JSON file path
+    # Note this means you cannot just browse to /needles/distri/subdir/needle.png;
+    # you can only find needles in subdirectories by passing the jsonfile parameter
+    my ($dummy1, $path, $dummy2) = fileparse($jsonfile);
+    # drop the trailing / from $path
+    $path = substr($path, 0, -1);
+    if (index($path, '/needles') != -1) {
+        # we got something like /var/lib/openqa/share/tests/distri/needles/(subdir)/needle.json
+        my @elems = split('/needles', $path, 2);
+        if (defined $elems[1]) {
+            $needledir .= $elems[1];
+        }
+    }
+    elsif ($path ne '.') {
+        # we got something like subdir/needle.json, $path will be "subdir"
+        $needledir .= "/$path";
+    }
+    push(@{($self->{static} = Mojolicious::Static->new)->paths}, $needledir);
 
     # name is an URL parameter and can't contain slashes, so it should be safe
     return $self->serve_static_($name . $format);

t/ui/07-file.t

@@ -17,6 +17,7 @@
 use Mojo::Base -strict;
 
 use FindBin;
+use File::Spec;
 use lib "$FindBin::Bin/../lib";
 use Test::More;
 use Test::Mojo;
@@ -63,25 +64,51 @@ sub write_file {
 
 subtest 'needle download' => sub {
     # clean leftovers from previous run
-    my $needle_dir = Mojo::File->new('t/data/openqa/share/tests/opensuse/needles');
+    my $needle_path = 't/data/openqa/share/tests/opensuse/needles';
+    my $abs_needle_path = File::Spec->rel2abs($needle_path);
+    my $needle_dir = Mojo::File->new($needle_path);
     $needle_dir->remove_tree();
 
     $t->get_ok('/needles/opensuse/inst-timezone-text.png')->status_is(404, '404 if image not present');
     $t->get_ok('/needles/1/image')->status_is(404, '404 if image not present');
     $t->get_ok('/needles/1/json')->status_is(404, '404 if json not present');
 
     # create fake json file and image
-
     $needle_dir->make_path();
     my $json
       = '{"area" : [{"height": 217, "type": "match", "width": 384, "xpos": 0, "ypos": 0},{"height": 60, "type": "exclude", "width": 160, "xpos": 175, "ypos": 45}], "tags": ["inst-timezone"]}';
     write_file("$needle_dir/inst-timezone-text.png",  "png\n");
     write_file("$needle_dir/inst-timezone-text.json", $json);
 
+    # and another, in a subdirectory, to test that
+    my $needle_subdir = Mojo::File->new('t/data/openqa/share/tests/opensuse/needles/subdirectory');
+    $needle_subdir->make_path();
+    my $json2
+      = '{"area" : [{"height": 217, "type": "match", "width": 384, "xpos": 0, "ypos": 0},{"height": 60, "type": "exclude", "width": 160, "xpos": 175, "ypos": 45}], "tags": ["inst-subdirectory"]}';
+    write_file("$needle_subdir/inst-subdirectory.png",  "png\n");
+    write_file("$needle_subdir/inst-subdirectory.json", $json2);
+
     $t->get_ok('/needles/opensuse/inst-timezone-text.png')->status_is(200)->content_type_is('image/png')
       ->content_is("png\n");
     $t->get_ok('/needles/1/image')->status_is(200)->content_type_is('image/png')->content_is("png\n");
     $t->get_ok('/needles/1/json')->status_is(200)->content_type_is('application/json;charset=UTF-8')->content_is($json);
+
+    # arguably this should work and be tested, but does not work now because
+    # of how we do routing:
+    #$t->get_ok('/needles/opensuse/subdirectory/inst-subdirectory.png')
+
+    # currently you can only find a needle in a subdirectory by passing the
+    # jsonfile query parameter like this:
+    $t->get_ok("/needles/opensuse/inst-subdirectory.png?jsonfile=$needle_path/subdirectory/inst-subdirectory.json")->status_is(200)->content_type_is('image/png')
+      ->content_is("png\n");
+    # also test with jsonfile as absolute path (as usual in production)
+    $t->get_ok("/needles/opensuse/inst-subdirectory.png?jsonfile=$abs_needle_path/subdirectory/inst-subdirectory.json")->status_is(200)->content_type_is('image/png')
+      ->content_is("png\n");
+
+    # getting needle image and json by ID also does not work for needles
+    # in subdirectories, but arguably should do and should be tested:
+    #$t->get_ok('/needles/2/image')->status_is(200)->content_type_is('image/png')->content_is("png\n");
+    #$t->get_ok('/needles/2/json')->status_is(200)->content_type_is('application/json;charset=UTF-8')->content_is($json2);
 };
 
 

t/ui/12-needle-edit.t

@@ -617,6 +617,11 @@ subtest '(created) needles can be accessed over API' => sub {
           ->status_is(403, 'access to files outside the test directory not granted');
     };
     map { like($_, qr/is not in a subdir of/, 'expected warning') } @warnings;
+    @warnings = warnings {
+        $t->get_ok('/needles/opensuse/test-newneedle.png?jsonfile=t/data/openqa/share/tests/opensuse/needles/../../../../try/to/break_out.json')
+          ->status_is(403, 'access to files outside the test directory not granted');
+    };
+    map { like($_, qr/is not in a subdir of/, 'expected warning') } @warnings;
 
     my $tmp_dir = 't/tmp_needles';
     File::Path::rmtree($tmp_dir);