Эх сурвалжийг харах

Die when a template reads a variable that its caller did not pass

Xslate renders a variable that is not in its data as nothing, and does so
whether the variable is optional or the caller forgot it.  So a template and
its caller could disagree about the template's variables and nothing said so.
sitemap() built a warning for the sitemap page and never passed it, and
posts.tx and the forms read an app that nothing has set.

Now render() locks the keys of a copy of the data, and Xslate dies on a read
of any other key, inside an if as well as in a print.  A variable passed as
undef still renders as nothing, so a caller says which variables it leaves
empty.  The copy is sized before it is filled, because perl checks the keys
of a locked hash only once it has buckets, and an empty hash has none.
Setting a variable in a template is unaffected: Kolon already forbids it,
and my, for and macro bind lexicals.  An include that passes variables
needs them in the data, which posts.tx does with post.

The callers now pass what their templates read.  index() passes the
variables of index.tx and embed.tx that a route may leave unset.  sitemap()
passes its warning, posts() passes app as undef, and rss_style() passes lang.
login.tx and notconfigured.tx take the Header and the Footer as components,
as index.tx does, rather than including header.tx into their own data.  The
Header component takes a fixed list of arguments and dies on any other, and
CategoryBar defaults user.  Their callers spread %$query before their own
keys rather than after, so that a key they set is the one rendered.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
troglodyne-bot 3 өдөр өмнө
parent
commit
2b7a282f74

+ 1 - 0
lib/Trog/Component/CategoryBar.pm

@@ -23,6 +23,7 @@ decides whether the links point at /secure.
 
 sub render (%args) {
     $args{categories} //= [];
+    $args{user}       //= undef;
     return Trog::Component::render_template( 'categories.tx', \%args );
 }
 

+ 9 - 2
lib/Trog/Component/Header.pm

@@ -18,7 +18,7 @@ and icon metadata, and the stylesheet and script links.
 
 Render header.tx.
 
-Recognized arguments, all optional:
+It takes these arguments, all optional, and dies on any other:
 
     title / route / lang / author         the page itself
     meta_desc / meta_tags / default_tags  social metadata, built by the route
@@ -33,12 +33,19 @@ The list arguments default to empty rather than to the stock sheets, so that a
 caller which has none -- rss_style(), notably -- gets a header with none, as it
 did when the route rendered this itself.
 
-theme_dir is worked out here rather than passed, as it is the same for every
+theme_dir is worked out here when it is not passed, as it is the same for every
 page and the template only wants it to build asset paths.
 
 =cut
 
+my @arguments = qw{title route lang author meta_desc meta_tags default_tags stylesheets print_styles scripts embed no_doctype title_select theme_dir};
+my %known     = map { $_ => 1 } @arguments;
+
 sub render (%args) {
+    my @unknown = grep { !$known{$_} } sort keys(%args);
+    die "The Header component takes no argument named @unknown\n" if @unknown;
+    $args{$_} //= undef for @arguments;
+
     $args{lang}         //= 'en-US';
     $args{title}        //= 'tCMS';
     $args{stylesheets}  //= [];

+ 14 - 4
lib/Trog/Renderer/Base.pm

@@ -4,7 +4,8 @@ use 5.040;
 use re '/aasx';
 use warnings FATAL => 'all';
 
-use Encode qw{encode_utf8};
+use Encode     qw{encode_utf8};
+use Hash::Util ();
 use IO::Compress::Gzip;
 
 use Text::Xslate;
@@ -42,8 +43,11 @@ Recognized options:
     child_processor / child_renderer => override how post bodies are rendered by
                    the render_it() template function.  Built for you if omitted.
 
-Dies unless Xslate can resolve and render the template.  Returns the body
-string for components, and a PSGI arrayref of [ code, headers, body ] otherwise.
+Dies unless Xslate can resolve and render the template.  It also dies when the
+template reads a variable that data does not have, so a variable that a
+template may read must be in data, as undef if it has no value.  Returns the
+body string for components, and a PSGI arrayref of [ code, headers, body ]
+otherwise.
 
 =cut
 
@@ -98,9 +102,15 @@ sub render (%options) {
     # renders on with an empty string in its place, so the render below succeeds
     # with a hole in it.  Localised here so that a nested render (a component
     # rendering its own template) doesn't answer for its parent's failure.
+    # Sized first, because perl checks the keys of a locked hash only once the
+    # hash has buckets, and an empty one has none.
+    my %vars;
+    keys(%vars) = 1;
+    %vars = %{ $options{data} // {} };
+    Hash::Util::lock_keys(%vars);
     my $body = eval {
         local $Trog::Component::error;
-        my $out = encode_utf8( $renderers{$renderer_key}->render( $options{template}, $options{data} ) );
+        my $out = encode_utf8( $renderers{$renderer_key}->render( $options{template}, \%vars ) );
         die $Trog::Component::error if $Trog::Component::error;
         $out;
     };

+ 9 - 8
lib/Trog/Routes/HTML.pm

@@ -579,6 +579,9 @@ sub index ( $query, $content = '', $i_styles = [] ) {
         $tmpl,
         {
             %$query,
+
+            # What index.tx and embed.tx read that a route may leave unset.
+            ( map { $_ => $query->{$_} } qw{author is_admin failure message to user user_class nochrome route} ),
             search_lang  => $search_lang,
             search_help  => $search_help,
             theme_dir    => Trog::Themes::td(),
@@ -618,9 +621,9 @@ sub setup ($query) {
     return Trog::Renderer->render(
         template => 'notconfigured.tx',
         data     => {
+            %$query,
             title       => 'tCMS Requires Setup to Continue...',
             stylesheets => build_themed_styles( ['notconfigured.css'] ),
-            %$query,
         },
         contenttype => 'text/html',
         code        => 200,
@@ -800,15 +803,14 @@ sub login ($query) {
     return Trog::Renderer->render(
         template => 'login.tx',
         data     => {
+            %$query,
             title       => 'tCMS 2 ~ Login',
             to          => $query->{to},
             failure     => int( $query->{failed} ),
             message     => int( $query->{failed} ) < 1 ? "Login Successful, Redirecting..." : "Login Failed.",
             btnmsg      => $btnmsg,
             stylesheets => build_themed_styles( [qw{structure.css screen.css login.css}] ),
-            theme_dir   => Trog::Themes::td(),
             has_users   => $hasusers,
-            %$query,
         },
         headers     => $headers,
         contenttype => 'text/html',
@@ -1852,6 +1854,7 @@ sub posts ( $query, $direct = 0 ) {
         template => 'posts.tx',
         data     => {
             acls              => \@acls,
+            app               => undef,
             can_edit          => $is_admin,
             forms             => $forms,
             post              => { tags => $tags, extra_tags => \@et, form => $edittype, visibility => $user_visibility, addpost => 1 },
@@ -2267,6 +2270,7 @@ sub sitemap ($query) {
     $query->{to_map}     = \@to_map;
     $query->{is_index}   = $is_index;
     $query->{route_type} = $route_type;
+    $query->{warning}    = $warning;
     $query->{etag}       = $etag;
 
     return Trog::Routes::HTML::index( $query, undef, $styles );
@@ -3461,14 +3465,11 @@ without a doctype.
 =cut
 
 sub rss_style ($query) {
-    $query->{port}         = ":$query->{port}" if $query->{port};
-    $query->{title_select} = 'rss/channel/title';
-    $query->{no_doctype}   = 1;
-
+    $query->{port} = ":$query->{port}" if $query->{port};
     return Trog::Renderer->render(
         template    => 'rss-style.tx',
         contenttype => 'text/xsl',
-        data        => $query,
+        data        => { %$query, title_select => 'rss/channel/title', lang => $query->{lang} },
         code        => 200,
     );
 }

+ 7 - 0
t/Trog-Component-page.t

@@ -75,6 +75,12 @@ subtest 'Header' => sub {
     # It also has no stylesheets or scripts to give, and must not invent any.
     unlike( $xsl, qr/rel="stylesheet"[ ]type="text\/css"[ ]href="\/styles\/screen/, "no stylesheets are conjured up" );
 
+    like(
+        exception { Trog::Component::component( 'Header', { bogus => 1 } ) },
+        qr/takes[ ]no[ ]argument[ ]named[ ]bogus/,
+        "an argument it does not take is fatal"
+    );
+
     # header.tx marks meta_tags raw, so social markup has to survive as markup.
     my $social = Trog::Component::component( 'Header', { meta_tags => '<meta name="twitter:card" />' } );
     like( $social, qr{<meta[ ]name="twitter:card"[ ]/>}, "meta_tags stay markup" );
@@ -142,6 +148,7 @@ subtest 'the whole page still composes' => sub {
                 stylesheets  => ['/styles/screen.css'],
                 print_styles => [],
                 scripts      => [],
+                ( map { $_ => undef } qw{author default_tags embed failure is_admin lang message meta_desc meta_tags search_help search_lang show_madeby to user user_class} ),
             },
         );
     };

+ 1 - 0
t/Trog-Component.t

@@ -202,6 +202,7 @@ subtest 'through the real renderer' => sub {
                 failure  => -1,
                 posts    => [],
                 forms    => [],
+                ( map { $_ => undef } qw{acls app category embed footers headers in_series like limit message months newer nochrome older page pages pages_total paginate_offset post post_visibilities primary_post route rss sizes style subhead tiled to years} ),
             },
         );
     };

+ 1 - 1
t/Trog-DataSource.t

@@ -453,7 +453,7 @@ subtest 'the paginator renders page links for a datasource' => sub {
             template    => 'paginator.tx',
             contenttype => 'text/html',
             component   => 1,
-            data        => { limit => 25, sizes => [ 25, 50, 100 ], years => [2024], months => [ 0 .. 11 ], older => 0, newer => 0, %vars },
+            data        => { limit => 25, sizes => [ 25, 50, 100 ], years => [2024], months => [ 0 .. 11 ], older => 0, newer => 0, embed => undef, like => undef, pages => undef, paginate_offset => undef, page => undef, pages_total => undef, %vars },
         );
     };
 

+ 31 - 0
t/Trog-Renderer-Base.t

@@ -0,0 +1,31 @@
+#!/usr/bin/env perl
+use 5.040;
+use re '/aasx';
+use warnings FATAL => 'all';
+
+use Test::More;
+use Test::Fatal qw{exception};
+
+use FindBin::libs;
+
+require_ok('Trog::Renderer::Base') or BAIL_OUT("Can't find SUT");
+
+subtest 'render' => sub {
+    my %options = ( template => 'manual.tx', contenttype => 'text/html', component => 1 );
+
+    like(
+        exception { Trog::Renderer::Base::render( %options, data => {} ) },
+        qr/disallowed[ ]key[ ]'content'/,
+        'a variable that the template reads and data lacks is fatal, and named'
+    );
+    my $out;
+    is( exception { $out = Trog::Renderer::Base::render( %options, data => { content => undef } ) }, undef, 'but one that data has as undef is not' );
+    like( $out, qr{<div[ ]id="backoffice">\s*</div>}, 'and renders as nothing' );
+
+    my %data = ( content => 'x' );
+    Trog::Renderer::Base::render( %options, data => \%data );
+    $data{later} = 1;
+    is( $data{later}, 1, "and the caller's data is not left locked" );
+};
+
+done_testing();

+ 5 - 0
t/Trog-Routes-HTML-series-lifecycle.t

@@ -1695,6 +1695,11 @@ subtest 'the topbar did not silently overflow' => sub {
     cmp_ok( scalar @titles, '<=', 10, 'and we are not at the _get_series limit' );
 };
 
+subtest 'the sitemap page' => sub {
+    my ( $code, undef, $err ) = render( anon( route => '/sitemap' ), \&Trog::Routes::HTML::sitemap );
+    is( $code, 200, 'renders' ) or diag($err);
+};
+
 subtest 'the tpsgi side effects stayed inert' => sub {
     cmp_ok( $TPSGI->{restarts},  '>', 0, 'post_save did try to signal a restart' );
     cmp_ok( $TPSGI->{callbacks}, '>', 0, 'and did queue post-close callbacks' );

+ 6 - 2
www/templates/html/login.tx

@@ -1,4 +1,8 @@
-: include "components/header.tx";
+<: component('Header', {
+    title       => $title,
+    route       => $route,
+    stylesheets => $stylesheets,
+}) :>
 : include "jsalert.tx";
 <style>
 #jsalert {
@@ -56,4 +60,4 @@
 : }
 
 </div>
-: include "components/footer.tx";
+<: component('Footer') :>

+ 6 - 2
www/templates/html/notconfigured.tx

@@ -1,4 +1,8 @@
-: include 'components/header.tx';
+<: component('Header', {
+    title       => $title,
+    route       => $route,
+    stylesheets => $stylesheets,
+}) :>
 <nav>
     <strong>tCMS Initial Setup</strong>
 </nav>
@@ -20,4 +24,4 @@
         Please <a href="/login?to=/config" alt="Login">Log In</a> and Configure tCMS.
     </p>
 </section>
-: include 'components/footer.tx';
+<: component('Footer') :>