浏览代码

Merge pull request #360 from Troglodyne-Internet-Widgets/openapi-routing

Hand tPSGI an OpenAPI document of the routes, rather than regexes
George S. Baugh 5 天之前
父节点
当前提交
3735564d37

+ 1 - 0
.pod_stopwords

@@ -230,6 +230,7 @@ NTP
 nullary
 OAuth
 OOM
+OpenAPI
 OpenAPIv
 openapiv
 overcommit

+ 9 - 0
design.md

@@ -68,6 +68,15 @@ builds the routing table out of them, then merges the static routes from
 Publishing a post creates a URL because the URL *is* a field on the post. There
 is no separate router config to keep in sync, and no rewrite rules.
 
+A key of the table is a path, or an OpenAPI path template such as
+`/sitemap/{map}.xml` or `/lib/{module+}`. Each variable of a template reaches the
+callback under its name, without the `+`. `TCMS::build_routes` turns the table
+into the OpenAPI 3 document that tPSGI routes by. A route's `parameters` are
+OpenAPI parameter objects, so tPSGI validates the request before the callback
+runs. Its flags, such as `auth` and `totp_exempt`, reach the callback as
+`x-tcms-auth` and so on in `$query->{dispatcher}`. The path of a post cannot
+hold a `{` or a `}`, because OpenAPI has no way to escape them.
+
 **Adding a content type does not require touching Perl.** A post type is a
 template that says how to display and edit it, plus a JSON sidecar that says what
 fields it has. Both are files in a directory. See [Post types](#post-types).

+ 61 - 26
lib/TCMS.pm

@@ -8,6 +8,7 @@ use Carp  qw{confess};
 use Clone qw{clone};
 
 use CGI::Cookie ();
+use List::Util  ();
 use URI();
 
 #Grab our custom routes
@@ -41,6 +42,12 @@ my sub _routes ( $data = {} ) {
     # XXX this is probably bad in the case of the specific content-typed routes, they should be mutex
     my %roots = $data->routes();
 
+    # OpenAPI has no escape for a brace, so such a path would be a template.
+    foreach my $href ( grep { m/[{}]/ } keys(%roots) ) {
+        WARN("Not routing $href: a path with a brace in it cannot be a route");
+        delete $roots{$href};
+    }
+
     # Setup the /secure/... versions for logged in users, except for already secured routes.
     foreach my $route ( keys(%roots) ) {
         next if index( $route, '/secure/' ) == 0;
@@ -96,7 +103,24 @@ my sub _routes ( $data = {} ) {
 
 # Troglodyne philosophy - simple as possible
 
-# Build routes
+=head2 build_routes() = HASH
+
+The OpenAPI 3 document that tPSGI routes by, made from the routing table that
+_routes() returns.  Each route becomes the operation of its method, under its
+path.  The operation's x-tpsgi-callback is the route's callback, wrapped in what
+every tCMS request needs.  C<data> becomes x-tpsgi-data, and C<auth>,
+C<nocache>, C<noindex>, C<nomap>, C<robot_name> and C<totp_exempt> become
+x-tcms-auth and so on, which the callback finds in C<< $query->{dispatcher} >>.
+C<parameters> are OpenAPI parameter objects, and each variable of a template
+that they leave out is a required string.  A route with no method is a GET.
+
+The C<default> route is not a path.  The wrapper calls it when there is no
+F<config/setup>.
+
+=cut
+
+my @tcms_keys = qw{auth nocache noindex nomap robot_name totp_exempt};
+
 sub build_routes {
     my $conf   = Trog::Config::get();
     my $data   = Trog::Data->new($conf);
@@ -104,20 +128,16 @@ sub build_routes {
 
     my $default_route = $routes{default}{callback};
 
-    # Transform 'method' / 'callback' to new scheme
-    my %routes_adj;
-    foreach my $k ( keys(%routes) ) {
+    my %paths;
+    foreach my $k ( sort grep { $_ ne 'default' } keys(%routes) ) {
         my $v = $routes{$k};
 
-        # Some routes are just pointers to other routes.
-        # In this case they might have already been transformed and need to be skipped.
-        if ( exists $v->{callbacks} ) {
-            $routes_adj{$k} = $v;
-            next;
-        }
+        my $method = lc( $v->{method} // 'GET' );
 
-        my %cb;
-        my $callback = delete $v->{callback};
+        # FlatFile and the tag index name the callback rather than hold it.
+        my $callback = $v->{callback};
+        next                      unless $callback;
+        $callback = \&{$callback} unless ref $callback;
 
         #XXX For now we will discard the tPSGI object.  We might want it later.
         my $cb_wrap = sub {    ## no critic (ProhibitSingleUseLexicalSubs) -- 71 lines, which would overcomplicate build_routes()
@@ -180,26 +200,36 @@ sub build_routes {
             $query->{to} = URI->new( $query->{to} // '' )->path() || $query->{to} if $query->{to};
 
             # Now that we have firmed up the actual routing, let's validate.
-            return $tpsgi->forbidden($query) if exists $query->{dispatcher}{auth} && !$active_user;
+            return $tpsgi->forbidden($query) if exists $query->{dispatcher}{'x-tcms-auth'} && !$active_user;
 
             if ( needs_enrolment( $active_user, $query->{dispatcher} ) ) {
                 INFO("$active_user has no TOTP enrolment; sending them to /totp");
                 return $tpsgi->see_also('/totp');
             }
 
-            no strict 'refs';
-            $callback->($query);
-            use strict;
+            return $callback->($query);
         };
 
-        #XXX todo support different callbacks per requested content-type
-        $cb{'*'} = $cb_wrap if $callback;
+        my @parameters = @{ $v->{parameters} // [] };
+        foreach my $variable ( $k =~ m/\{([^{}]+)\}/g ) {
+            next if List::Util::any { $_->{in} eq 'path' && $_->{name} eq $variable } @parameters;
+            push( @parameters, { name => $variable, in => 'path', required => true, schema => { type => 'string' } } );
+        }
 
-        $v->{callbacks} = \%cb;
-        $routes_adj{$k} = $v;
+        $paths{$k}{$method} = {
+            'x-tpsgi-callback' => $cb_wrap,
+            ( $v->{data} ? ( 'x-tpsgi-data' => $v->{data} ) : () ),
+            ( map { ( "x-tcms-$_" => $v->{$_} ) } grep { exists $v->{$_} } @tcms_keys ),
+            ( @parameters ? ( parameters => \@parameters ) : () ),
+            responses => { default => { description => 'The page, or an error page' } },
+        };
     }
 
-    return [%routes_adj];
+    return (
+        openapi => '3.0.3',
+        info    => { title => 'tCMS', version => '1' },
+        paths   => \%paths,
+    );
 }
 
 =head2 needs_enrolment($user, $dispatcher) = BOOL
@@ -214,13 +244,14 @@ route a logged out visitor can, is refused until they have enrolled.
 
 The enrollment page and the QR on it say so themselves, with totp_exempt, because
 the exemption belongs next to the route rather than in a list over here that the
-routing table knows nothing about.
+routing table knows nothing about.  $dispatcher is the operation that
+build_routes() made, so it holds them as x-tcms-auth and x-tcms-totp_exempt.
 
 =cut
 
 sub needs_enrolment ( $user, $dispatcher ) {
     return 0 unless $user;
-    return 0 if !$dispatcher || !$dispatcher->{auth} || $dispatcher->{totp_exempt};
+    return 0 if !$dispatcher || !$dispatcher->{'x-tcms-auth'} || $dispatcher->{'x-tcms-totp_exempt'};
     return Trog::Auth::has_totp($user) ? 0 : 1;
 }
 
@@ -233,7 +264,7 @@ sub generic_route ( $rname, $code, $title, $query ) {
     return Trog::Routes::HTML::index($query);
 }
 
-our @routes  = @{ build_routes() };
+our %openapi = build_routes();
 our %aliases = aliases();
 
 sub aliases {
@@ -255,8 +286,12 @@ sub robots ($query) {
     state $etag = "robots-" . time();
     my $routes = _routes();
 
-    # If there's a 'capture' route, we need to format it correctly.
-    state @banned = map { exists $routes->{$_}{robot_name} ? $routes->{$_}{robot_name} : $_ } grep { $routes->{$_}{noindex} } sort keys(%$routes);
+    # A template's variables can be anything, which robots.txt spells as *.
+    state @banned = map {
+        my $robot_name = $routes->{$_}{robot_name} // $_;
+        $robot_name =~ s/\{[^{}]*\}/*/g;
+        $robot_name
+    } grep { $routes->{$_}{noindex} } sort keys(%$routes);
 
     return Trog::Renderer->render(
         contenttype => 'text/plain',

+ 17 - 24
lib/Trog/Routes/HTML.pm

@@ -134,10 +134,9 @@ our %routes = (
         auth     => 1,
         callback => \&Trog::Routes::HTML::manual,
     },
-    '/lib/(.*)' => {
+    '/lib/{module+}' => {
         method   => 'GET',
         auth     => 1,
-        captures => ['module'],
         callback => \&Trog::Routes::HTML::manual,
     },
     '/password_reset' => {
@@ -175,15 +174,14 @@ our %routes = (
         noindex  => 1,
         nocache  => 1,
     },
-    '/guest/screenshot/(.*)/(.*)' => {
+
+    # 'guest', not 'domain': the router sets $query->{domain} to the request's
+    # own host *after* it applies the path parameters, so a variable by that
+    # name never survives to the callback.
+    '/guest/screenshot/{hypervisor}/{guest}' => {
         method   => 'GET',
         auth     => 1,
         callback => \&Trog::Routes::HTML::guest_screenshot,
-
-        # 'guest', not 'domain': the router sets $query->{domain} to the
-        # request's own host *after* it applies captures, so a capture by that
-        # name never survives to the callback.
-        captures => [qw{hypervisor guest}],
         noindex  => 1,
         nocache  => 1,
     },
@@ -201,15 +199,13 @@ our %routes = (
         noindex  => 1,
         nocache  => 1,
     },
-    '/guest/reprovision/log/(.*)' => {
-        method   => 'GET',
-        auth     => 1,
-        callback => \&Trog::Routes::HTML::guest_reprovision_log,
 
-        # 'guest', not 'domain', for the reason the screenshot route captures it
-        # that way -- the router overwrites $query->{domain} with the request's
-        # own host after applying captures.
-        captures   => [qw{guest}],
+    # 'guest', not 'domain', for the reason the screenshot route names it that
+    # way.
+    '/guest/reprovision/log/{guest}' => {
+        method     => 'GET',
+        auth       => 1,
+        callback   => \&Trog::Routes::HTML::guest_reprovision_log,
         robot_name => '/guest/reprovision/log/',
         noindex    => 1,
         nocache    => 1,
@@ -257,17 +253,15 @@ our %routes = (
         callback => \&Trog::Routes::HTML::sitemap,
         data     => { xml => 1, compressed => 1, map => 'static' },
     },
-    '/sitemap/(.*).xml' => {
+    '/sitemap/{map}.xml' => {
         method   => 'GET',
         callback => \&Trog::Routes::HTML::sitemap,
         data     => { xml => 1 },
-        captures => ['map'],
     },
-    '/sitemap/(.*).xml.gz' => {
+    '/sitemap/{map}.xml.gz' => {
         method   => 'GET',
         callback => \&Trog::Routes::HTML::sitemap,
         data     => { xml => 1, compressed => 1 },
-        captures => ['map'],
     },
     '/humans.txt' => {
         method   => 'GET',
@@ -279,10 +273,9 @@ our %routes = (
         callback => \&Trog::Routes::HTML::avatars,
         data     => { tag => ['about'] },
     },
-    '/avatar/(.*)' => {
+    '/avatar/{avatar_hash}' => {
         method   => 'GET',
         callback => \&Trog::Routes::HTML::libravatar,
-        captures => ['avatar_hash'],
         noindex  => 1,
         nocache  => 1,
     },
@@ -290,7 +283,7 @@ our %routes = (
         method   => 'GET',
         callback => \&Trog::Routes::HTML::icon,
     },
-    '/totp_qr/(.*)' => {
+    '/totp_qr/{file}' => {
         auth   => 1,
         method => 'GET',
 
@@ -2140,7 +2133,7 @@ sub sitemap ($query) {
 
         # Return the map of static routes
         $route_type = 'Static Routes';
-        @to_map     = grep { !defined $routes{$_}->{captures} && !$routes{$_}->{auth} && !$routes{$_}->{noindex} && !$routes{$_}->{nomap} } keys(%routes);
+        @to_map     = grep { CORE::index( $_, '{' ) < 0 && !$routes{$_}->{auth} && !$routes{$_}->{noindex} && !$routes{$_}->{nomap} } keys(%routes);
     }
     elsif ( !$query->{map} ) {
 

+ 26 - 30
lib/Trog/Routes/JSON.pm

@@ -6,65 +6,61 @@ use warnings FATAL => 'all';
 
 use Clone qw{clone};
 
-use Scalar::Util();
-
 use Trog::Utils();
 use Trog::Auth();
 use Trog::Routes::HTML();
 
 use Trog::Log::Metrics();
 
+# The parameters of a route are OpenAPI 3 parameter objects, which
+# TCMS::build_routes puts in the operation that tPSGI validates a request by.
 our %routes = (
     '/api/catalog' => {
-        method     => 'GET',
-        callback   => \&catalog,
-        parameters => {},
+        method   => 'GET',
+        callback => \&catalog,
     },
     '/api/webmanifest' => {
-        method     => 'GET',
-        callback   => \&webmanifest,
-        parameters => {},
+        method   => 'GET',
+        callback => \&webmanifest,
     },
     '/api/version' => {
-        method     => 'GET',
-        callback   => \&version,
-        parameters => {},
+        method   => 'GET',
+        callback => \&version,
     },
-    '/api/auth_change_request/(.*)' => {
+    '/api/auth_change_request/{token}' => {
         method     => 'GET',
         callback   => \&process_auth_change_request,
-        captures   => ['token'],
-        parameters => {
-            token => sub { my $tok = shift; $tok =~ m/\A[\da-f-]+\z/; },
-        },
+        parameters => [
+            {
+                name     => 'token',
+                in       => 'path',
+                required => true,
+
+                # What Trog::Utils::uuid() makes, ended by \z because perl's $ allows a final newline.
+                schema => { type => 'string', pattern => '^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z' },
+            },
+        ],
         noindex    => 1,
         robot_name => '/api/auth_change_request/*',
     },
     '/api/posts_of_form' => {
         method     => 'GET',
         auth       => 1,
-        parameters => {
+        parameters => [
 
             # Same shape series.json demands of child_form.
-            form => sub { $_[0] =~ m/^[\w-]+\.tx$/ },
-        },
+            { name => 'form', in => 'query', schema => { type => 'string', pattern => '^[A-Za-z0-9_-]+\.tx\z' } },
+        ],
         callback => \&posts_of_form,
         noindex  => 1,
     },
     '/api/requests_per' => {
         method     => 'GET',
         auth       => 1,
-        parameters => {
-            period => sub {
-                grep {
-                    my $valid = $_;
-                    List::Util::any { $_ eq $valid } @_
-                } qw{second minute hour day week month year};
-            },
-            num_periods => \&Scalar::Util::looks_like_number,
-            before      => \&Scalar::Util::looks_like_number,
-            code        => \&Scalar::Util::looks_like_number,
-        },
+        parameters => [
+            { name => 'period', in => 'query', schema => { type => 'string', enum => [qw{second minute hour day week month year}] } },
+            map { { name => $_, in => 'query', schema => { type => 'number' } } } qw{num_periods before code},
+        ],
         callback => \&requests_per,
     },
 );

+ 131 - 0
t/TCMS.t

@@ -0,0 +1,131 @@
+#!/usr/bin/env perl
+
+# The OpenAPI document that TCMS.pm hands tPSGI, made from tCMS's own routing
+# table.
+
+use 5.040;
+use re '/aasx';
+use warnings FATAL => 'all';
+
+use FindBin;
+use FindBin::libs;
+
+use Test::More;
+use Test::MockModule                   qw{strict};
+use JSON::MaybeXS                      ();
+use JSON::Validator::Schema::OpenAPIv3 ();
+
+our ( $REPO, $ROOT, $OLDCWD );
+
+BEGIN {
+    require Cwd;
+    require File::Copy;
+    require File::Path;
+    require File::Temp;
+
+    $REPO   = Cwd::abs_path("$FindBin::Bin/..");
+    $OLDCWD = Cwd::getcwd();
+    $ROOT   = File::Temp::tempdir( 'tcms-routes-XXXXXX', TMPDIR => 1, CLEANUP => 1 );
+
+    # TCMS.pm builds its document at file scope, against whatever cwd is
+    # current when it loads, and every path in tCMS is relative to that.
+    $ENV{HOME} = $ROOT;
+    File::Path::make_path( map { "$ROOT/$_" } qw{config schema data/files logs totp www/statics www/themes} );
+    File::Copy::copy( "$REPO/schema/$_", "$ROOT/schema/$_" ) or die $! for qw{auth.schema flatfile.schema log.schema};
+
+    open( my $fh, '>', "$ROOT/config/default.cfg" ) or die $!;
+    print {$fh} "[general]\n    data_model=FlatFile\n    hostname=routes.test.test\n";
+    close($fh) or die $!;
+
+    chdir($ROOT) or die "could not chdir to the sandbox: $!";
+}
+
+END {
+    chdir($OLDCWD) if $OLDCWD;
+}
+
+# The routes of posts, as the tag index of a FlatFile site gives them: by the
+# name of their callback.
+require_ok('Trog::SQLite::TagIndex') or BAIL_OUT("Can't load the tag index");
+my $index = Test::MockModule->new('Trog::SQLite::TagIndex');
+$index->redefine(
+    routes => sub {
+        return (
+            '/posts/p-one'   => { method => 'GET', callback => 'Trog::Routes::HTML::posts' },
+            '/posts/{curly}' => { method => 'GET', callback => 'Trog::Routes::HTML::posts' },
+        );
+    }
+);
+
+# What a post's route runs, which the routes of posts name.
+require_ok('Trog::Routes::HTML') or BAIL_OUT("Can't load the routes");
+my $html = Test::MockModule->new('Trog::Routes::HTML');
+$html->redefine( posts => sub { return [ 200, [], ['the post'] ] } );
+
+require_ok('TCMS') or BAIL_OUT("Can't load the dispatcher");
+
+my %openapi = do { no warnings 'once'; %TCMS::openapi };
+
+# Checked as JSON reads it, since tPSGI is not a prerequisite of tCMS: perl's
+# booleans become JSON's, and a callback becomes null.
+my $json   = JSON::MaybeXS->new( allow_unknown => 1, canonical => 1 );
+my $schema = JSON::Validator::Schema::OpenAPIv3->new( $json->decode( $json->encode( \%openapi ) ) );
+my $paths  = $openapi{paths};
+
+subtest 'the document' => sub {
+    is_deeply( $schema->errors, [], 'is valid OpenAPI 3' ) or diag explain $schema->errors;
+    ok( !exists $paths->{default},          'and has no path for the setup route, which is not one' );
+    ok( !( grep { m/[(]/ } keys(%$paths) ), 'nor any regex left' ) or diag explain [ grep { m/[(]/ } keys(%$paths) ];
+};
+
+subtest 'an operation' => sub {
+    my $manual = $paths->{'/lib/{module+}'}{get};
+    ok( $manual, 'the manual of a module is a GET of /lib/{module+}' );
+    is( ref $manual->{'x-tpsgi-callback'}, 'CODE', 'with a callback' );
+    is( $manual->{'x-tcms-auth'},          1,      'and its auth as x-tcms-auth' );
+    is_deeply(
+        $manual->{parameters},
+        [ { name => 'module+', in => 'path', required => true, schema => { type => 'string' } } ],
+        'and each variable of the template as a required path parameter'
+    );
+
+    my $totp = $paths->{'/totp'}{get};
+    is( $totp->{'x-tcms-totp_exempt'}, 1, 'totp_exempt is x-tcms-totp_exempt' );
+    is_deeply( $paths->{'/sitemap/{map}.xml'}{get}{'x-tpsgi-data'}, { xml => 1 }, 'and data is x-tpsgi-data' );
+    ok( $paths->{'/auth'}{post} && !$paths->{'/auth'}{get}, 'the method is the key of the operation' );
+};
+
+subtest 'the routes of posts' => sub {
+    is( ref $paths->{'/posts/p-one'}{get}{'x-tpsgi-callback'}, 'CODE', 'a post named by its callback is routed' );
+    ok( $paths->{'/secure/posts/p-one'}{get}, 'and so is its /secure copy' );
+
+    # The flag that says setup is done, or the default route answers instead.
+    open( my $fh, '>', 'config/setup' ) or die $!;
+    close($fh)                          or die $!;
+    my $res = $paths->{'/posts/p-one'}{get}{'x-tpsgi-callback'}->( { log_dir => "$ROOT/logs", verbose => 0 }, { route => '/posts/p-one', dispatcher => $paths->{'/posts/p-one'}{get} } );
+    is_deeply( $res->[2], ['the post'], 'and it runs the sub that the name names' );
+    ok( !exists $paths->{'/posts/{curly}'}, 'but a post whose path has a brace is not, since it would be a template' );
+};
+
+subtest 'validation' => sub {
+    my $token = '0f1e2d3c-4b5a-6978-8796-a5b4c3d2e1f0';
+    is_deeply( [ $schema->validate_request( [ get => '/api/auth_change_request/{token}' ], { path => { token => $token } } ) ], [], 'a token that is a UUID is taken' );
+    ok( scalar $schema->validate_request( [ get => '/api/auth_change_request/{token}' ], { path => { token => "$token|x" } } ), 'and one with more after it is not' );
+
+    is_deeply( [ $schema->validate_request( [ get => '/api/requests_per' ], { query => { period => 'week', num_periods => 2 } } ) ], [], 'a period that requests_per knows is taken' );
+    ok( scalar $schema->validate_request( [ get => '/api/requests_per' ], { query => { period      => 'fortnight' } } ), 'and one it does not is refused' );
+    ok( scalar $schema->validate_request( [ get => '/api/requests_per' ], { query => { num_periods => 'many' } } ),      'as is a count that is not a number' );
+};
+
+subtest 'robots' => sub {
+    my $banned;
+    my $renderer = Test::MockModule->new('Trog::Renderer');
+    $renderer->redefine( render => sub { my ( $class, %args ) = @_; $banned = $args{data}{banned}; return [ 200, [], [''] ] } );
+
+    TCMS::robots( { domain => 'routes.test.test' } );
+    ok( ( grep { $_ eq '/avatar/*' } @$banned ),               'a template with no robot_name is disallowed with * for its variable' );
+    ok( ( grep { $_ eq '/guest/reprovision/log/' } @$banned ), 'and one with a robot_name by that' );
+    ok( !( grep { m/[{}]/ } @$banned ),                        'and no template is written as it is' );
+};
+
+done_testing();

+ 6 - 4
t/Trog-Auth-totp-mandatory.t

@@ -71,9 +71,11 @@ subtest 'a user starts out unenrolled' => sub {
 };
 
 subtest 'and cannot go anywhere but /totp until they enrol' => sub {
-    my %page = ( auth => 1 );
+
+    # The operations that TCMS::build_routes makes of the routes.
+    my %page = ( 'x-tcms-auth' => 1 );
     my %public;
-    my %enrol = ( auth => 1, totp_exempt => 1 );
+    my %enrol = ( 'x-tcms-auth' => 1, 'x-tcms-totp_exempt' => 1 );
 
     is( TCMS::needs_enrolment( 'bob', \%page ),   1, 'a page behind a login is refused' );
     is( TCMS::needs_enrolment( 'bob', \%enrol ),  0, 'the enrolment page is not, or there would be nowhere to go' );
@@ -106,8 +108,8 @@ subtest 'and there is no way to turn it off' => sub {
     ok( !exists $Trog::Routes::HTML::routes{'/request_totp_clear'}, 'nor a route' );
 
     # The two the gate lets through, which is the whole of the exemption.
-    ok( $Trog::Routes::HTML::routes{'/totp'}{totp_exempt},         'the enrolment page says it is exempt' );
-    ok( $Trog::Routes::HTML::routes{'/totp_qr/(.*)'}{totp_exempt}, 'and so does the QR on it' );
+    ok( $Trog::Routes::HTML::routes{'/totp'}{totp_exempt},           'the enrolment page says it is exempt' );
+    ok( $Trog::Routes::HTML::routes{'/totp_qr/{file}'}{totp_exempt}, 'and so does the QR on it' );
     my @exempt = grep { $Trog::Routes::HTML::routes{$_}{totp_exempt} } keys(%Trog::Routes::HTML::routes);
     is( scalar(@exempt), 2, 'and nothing else is' );
 };

+ 1 - 2
t/Trog-DataSource-ProvisionedVirt.t

@@ -576,11 +576,10 @@ subtest 'the route' => sub {
 
     # The log route.  It lives in logs/ rather than under www/, and the output
     # of a provisioner is a fine place for a hostname or an IP plan to turn up.
-    my $logroute = $Trog::Routes::HTML::routes{'/guest/reprovision/log/(.*)'};
+    my $logroute = $Trog::Routes::HTML::routes{'/guest/reprovision/log/{guest}'};
     ok( $logroute, 'the log route is registered' );
     is( $logroute->{method}, 'GET', 'as a GET, since it only reads' );
     is( $logroute->{auth},   1,     'behind a login' );
-    is_deeply( $logroute->{captures}, ['guest'], "capturing 'guest', which survives routing" );
 
     $res = Trog::Routes::HTML::guest_reprovision_log( { user => 'bob', user_acls => ['public'], tpsgi => $tpsgi } );
     is( $res->[0], 403, 'a non-admin cannot read it' );

+ 10 - 7
t/Trog-DataSource-Virt.t

@@ -320,16 +320,19 @@ subtest 'the datasource hook only loads what it should' => sub {
 subtest 'the guest routes avoid the router\'s own query keys' => sub {
     require Trog::Routes::HTML;
 
-    # TPSGI applies a route's captures in extract_query, and *then* overwrites
-    # $query->{domain} with the request's own host.  A capture or form field by
-    # that name therefore never reaches the callback -- which silently aimed
-    # the screenshot and action routes at a guest named after the web host.
+    # TPSGI puts a route's path parameters in the query in extract_query, and
+    # *then* overwrites $query->{domain} with the request's own host.  A path
+    # variable or form field by that name therefore never reaches the callback
+    # -- which silently aimed the screenshot and action routes at a guest named
+    # after the web host.
     my @reserved = qw{domain route method scheme port user acls to};
 
     no warnings 'once';
-    my $captures = $Trog::Routes::HTML::routes{'/guest/screenshot/(.*)/(.*)'}{captures};
-    foreach my $capture (@$captures) {
-        ok( !( grep { $_ eq $capture } @reserved ), "the screenshot route's '$capture' capture is not a router key" );
+    my ($template) = grep { index( $_, '/guest/screenshot/' ) == 0 } keys(%Trog::Routes::HTML::routes);
+    my @variables = $template =~ m/\{([^{}]+)\}/g;
+    is_deeply( \@variables, [qw{hypervisor guest}], 'the screenshot route names the hypervisor and the guest' );
+    foreach my $variable (@variables) {
+        ok( !( grep { $_ eq $variable } @reserved ), "the screenshot route's '$variable' variable is not a router key" );
     }
 
     # The action form posts the same field names the handler reads, so check

+ 6 - 4
t/Trog-Routes-HTML-series-lifecycle.t

@@ -28,6 +28,7 @@ use File::Temp          ();
 use Time::HiRes         ();
 use URI::Escape         ();
 use JSON::MaybeXS       ();
+use JSON::Validator     ();
 use File::Slurper       ();
 use File::Slurper::Temp ();
 
@@ -1427,10 +1428,11 @@ subtest '/api/posts_of_form feeds the relation picker' => sub {
 
     # The route's own declaration is what keeps a malformed form out; check it
     # matches what series.json demands of child_form.
-    my $validator = $Trog::Routes::JSON::routes{'/api/posts_of_form'}{parameters}{form};
-    ok( $validator->('blog.tx'),                                 'the parameter validator accepts a form name' );
-    ok( !$validator->('../../etc/passwd'),                       'and rejects a path' );
-    ok( $Trog::Routes::JSON::routes{'/api/posts_of_form'}{auth}, 'the route requires a login' );
+    my ($form) = grep { $_->{name} eq 'form' } @{ $Trog::Routes::JSON::routes{'/api/posts_of_form'}{parameters} };
+    my $validator = JSON::Validator->new();
+    is_deeply( [ $validator->validate( 'blog.tx', $form->{schema} ) ], [], 'the parameter schema accepts a form name' );
+    ok( scalar $validator->validate( '../../etc/passwd', $form->{schema} ), 'and rejects a path' );
+    ok( $Trog::Routes::JSON::routes{'/api/posts_of_form'}{auth},            'the route requires a login' );
 };
 
 subtest 'a series resolves its relations for logged out visitors too' => sub {

+ 10 - 6
t/Trog-Routes-JSON.t

@@ -4,6 +4,7 @@ use re '/aasx';
 use warnings FATAL => 'all';
 
 use Test::More;
+use JSON::Validator ();
 
 use FindBin::libs;
 
@@ -11,13 +12,16 @@ require_ok('Trog::Routes::JSON') or BAIL_OUT("Can't find SUT");
 
 subtest 'the change request token is a uuid, and nothing else' => sub {
     no warnings 'once';
-    my $valid = $Trog::Routes::JSON::routes{'/api/auth_change_request/(.*)'}{parameters}{token};
+    my ($token) = @{ $Trog::Routes::JSON::routes{'/api/auth_change_request/{token}'}{parameters} };
+    my $jv      = JSON::Validator->new->schema( $token->{schema} );
+    my $valid   = sub ($value) { return !$jv->validate($value) };
 
-    ok( $valid->('5bed1745-4201-40dd-b5dd-65acc0316604'), 'a uuid is a token' );
-    ok( !$valid->(q{}),                                   'nothing is not' );
-    ok( !$valid->('x;drop table user'),                   'and neither is text with a hex digit in it' );
-    ok( !$valid->("5bed1745-4201\n"),                     'nor a token with something after it' );
-    ok( !$valid->('a|b'),                                 'nor a pipe, which the class used to hold' );
+    ok( $valid->('5bed1745-4201-40dd-b5dd-65acc0316604'),    'a uuid is a token' );
+    ok( !$valid->(q{}),                                      'nothing is not' );
+    ok( !$valid->('x;drop table user'),                      'and neither is text with a hex digit in it' );
+    ok( !$valid->("5bed1745-4201\n"),                        'nor a token with something after it' );
+    ok( !$valid->("5bed1745-4201-40dd-b5dd-65acc0316604\n"), 'nor a uuid with a newline after it' );
+    ok( !$valid->('a|b'),                                    'nor a pipe, which the class used to hold' );
 };
 
 done_testing();

+ 3 - 0
www/templates/html/components/405.tx

@@ -0,0 +1,3 @@
+<h1>405 Method Not Allowed</h1>
+<br /><br />
+This page does not take that kind of request.  See the SiteMap <a href="/sitemap">here</a> as to the valid ways to communicate with this website.

+ 3 - 0
www/templates/html/components/415.tx

@@ -0,0 +1,3 @@
+<h1>415 Unsupported Media Type</h1>
+<br /><br />
+This page does not take what you sent in that format.  See the SiteMap <a href="/sitemap">here</a> as to the valid ways to communicate with this website.