فهرست منبع

Hand tPSGI an OpenAPI document of the routes, rather than regexes

tPSGI now routes by an OpenAPI 3 document that a router exports as
%openapi, and refuses a router that still has @routes.  build_routes
flattened the routing hash into such an array, so the regex routes were
tried in hash order, and every literal path was a regex as well.

The routing hashes stay the source, because robots.txt, the sitemap and
the catalog read them.  Their regex keys are now templates: /lib/{module+},
/guest/screenshot/{hypervisor}/{guest}, /guest/reprovision/log/{guest},
/sitemap/{map}.xml and .xml.gz, /avatar/{avatar_hash}, /totp_qr/{file} and
/api/auth_change_request/{token}.  The variables replace captures, and keep
the names that avoid the keys the router sets itself.  build_routes makes
one operation per route: the method is its key, the wrapped callback is
x-tpsgi-callback, data is x-tpsgi-data, and the route's flags are x-tcms-*.
The wrapper and needs_enrolment read x-tcms-auth and x-tcms-totp_exempt from
the dispatcher.  The default route is not a path, and stays out.

The validator coderefs of the JSON routes are JSON Schema now.  The token
of an auth change request must be a UUID, which Trog::Utils::uuid() makes.
The old check had no anchors and a class that took a |, so it took any
string with one hex digit in it.  The catalog clones these routes, and it
died encoding the coderefs.  Schemas encode, so it answers again.

The paths of posts are literal paths.  OpenAPI has no escape for a brace,
so a post whose path has one is left out, with a warning.  The tag index of
a FlatFile site names each callback, and the wrapper called that name as a
symbolic reference.  It is resolved to the sub now.  robots.txt wrote a
regex route as the regex.  It writes * for each variable of a template that
has no robot_name.

tPSGI answers a path with a method that it has no route for with a 405,
and a body of the wrong media type with a 415.  generic_route renders the
page of the code, so both get a page of their own.

t/TCMS.t checks the document against the OpenAPI schema and each of these
rules.  Each rule was broken once on purpose, and the test failed: the brace
rule, the *, the path parameters, the x-tcms- names, the named callback, the
UUID pattern, the period enum and the default route.

Two problems stay, and are not fixed here.  A SQLite local_href keeps its
%20 while PATH_INFO is decoded, so such a post does not match.  The wrapper
asks whether x-tcms-auth exists, so auth => 0 still needs a login.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
troglodyne-bot 1 هفته پیش
والد
کامیت
45a4da7578

+ 1 - 0
Makefile.PL

@@ -123,6 +123,7 @@ WriteMakefile(
         'Sys::Hostname::FQDN'                 => '0',
         'List::Util'                          => '0',
         'Mojo::File'                          => '0',
+        'Mojo::JSON'                          => '0',
         'POSIX'                               => '0',
         'Ref::Util'                           => '0',
         'Pod::Html'                           => '0',

+ 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

@@ -9,6 +9,7 @@ use Clone qw{clone};
 
 use CGI::Cookie ();
 use URI();
+use Mojo::JSON ();
 
 #Grab our custom routes
 use FindBin::libs;
@@ -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 grep { $_->{in} eq 'path' && $_->{name} eq $variable } @parameters;
+            push( @parameters, { name => $variable, in => 'path', required => Mojo::JSON::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} ) {
 

+ 27 - 29
lib/Trog/Routes/JSON.pm

@@ -6,7 +6,7 @@ use warnings FATAL => 'all';
 
 use Clone qw{clone};
 
-use Scalar::Util();
+use Mojo::JSON ();
 
 use Trog::Utils();
 use Trog::Auth();
@@ -14,57 +14,55 @@ 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 => Mojo::JSON::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,
     },
 );

+ 125 - 0
t/TCMS.t

@@ -0,0 +1,125 @@
+#!/usr/bin/env perl
+
+# The OpenAPI document that TCMS.pm hands tPSGI, made from tCMS's own routing
+# table.
+
+use 5.040;
+use warnings;
+
+use FindBin;
+use lib "$FindBin::Bin/../lib";
+
+use Test::More;
+use Test::MockModule                   qw{strict};
+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;
+
+    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 = %TCMS::openapi;
+my $schema  = JSON::Validator::Schema::OpenAPIv3->new( \%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 => Mojo::JSON::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;
+    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 {

+ 5 - 1
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,12 +12,15 @@ 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->("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' );
 };
 

+ 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.