automatically mark static text as raw?

[Robert Messer]

We are using Text::Xslate to generate bits of html.  It works well for this, but we are finding that we have to litter our .tx files with calls to mark_raw.  That's because we sometimes need to include optional sections or extra bits of html.  So we might have this in a .tx file:

[% if ($needs_something) { mark_raw('<div>something</div>') } %]

That doesn't seem too bad in this small example, but when we have a lot of static strings, we have to have "mark_raw" all over the place.

It would be nice if there were some way to declare "all static strings are raw."  That is, if I am supplying a static string, I know it is not user input or dangerous.  There doesn't seem to be a great reason to force the use mark_raw for static strings.

So is there any way to automatically mark static strings as raw?  Or perhaps some other way to accomplish the same thing?  Thanks.

[Daisuke Maki]

just curious: Why can't you do

[% IF (foo) %]<div>something</div>[% END %]

?

--
--
You received this message because you are subscribed to the Google
Groups "Xslate" group.
To post to this group, send email to xsl...@googlegroups.com
To unsubscribe from this group, send email to
xslate+un...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/xslate

---
You received this message because you are subscribed to the Google Groups "Xslate" group.
To unsubscribe from this group and stop receiving emails from it, send an email to xslate+un...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Robert Messer]

Sometimes we could do that, but in many cases it is awkward.  My original example was like this:

[% if ($foo) { mark_raw('<div>') ~ $something ~ mark_raw('</div>') } %]

So we could change it to this:

%% if ($foo) {

<div>[% $something %]</div>

%% }

or:

[% if ($foo) %]<div>[% $something %]</div>[% } %]

But if we didn't have to do "mark_raw", then this reads a little better, and a bit more concise:

[% if ($foo) { '<div>' ~ $something ~ '</div>' } %]

And sometimes we define constants, and it is mainly just a nuisance to have to mark constant strings as raw.  I suppose it isn't a critical feature, but if there were some way to make Text::Xslate automatically consider static strings or constants inside an expression as raw, then that would be better I think.  If you don't think it is a good default, then maybe it could be an option?  Thanks,

Rob

You received this message because you are subscribed to a topic in the Google Groups "Xslate" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/xslate/HSkrE7xIaXE/unsubscribe.
To unsubscribe from this group and all its topics, send an email to xslate+un...@googlegroups.com.

[Daisuke Maki]

The string could be static, but it could still contain mal-formated content, so I don't think mixing the two ideas is a good thing to do.

You could easily have:

   use constant FOO => ">foo<";

   $tx->render(..., {foo => FOO});

It's okay if all variables are perfectly controlled by you and your developers, but you probably have many different inputs, not all of which are html-safe. I'm also willing to bet there's going to be that one person in  your team who's going to insert a possible XSS in the future when you're least prepared.

If you're sure your variables are static and html-safe, maybe what you need to do is to mark them raw when you're declaring them, or when you're passing them to Xslate

use Text::Xslate qw(mark_raw);
use constant FOO => mark_raw("foo");

my $tx = ...

$tx->render("template.tx", {

    foo => FOO,

    bar => mark_raw("bar"),

});



If you were to implement it anyway, I see a big problem in the way to  determine if a string is "static string" is in Perl. More precisely, with all the existing workarounds for "constant" strings in Perl how far do we go to detect them?

For example.. "use constant" could be easy. There's that READONLY flag:

$ perl -MDevel::Peek -E 'use constant FOO => "foo"; Dump(FOO)'

SV = PV(0x7ffc73859f40) at 0x7ffc73878630

  REFCNT = 2

  FLAGS = (PADMY,POK,READONLY,pPOK)

  PV = 0x7ffc7341b130 "foo"\0

  CUR = 3

  LEN = 16

Literal strings passed... hmm, still not bad.

$ perl -MDevel::Peek -E 'Dump("foo")'

SV = PV(0x7ff54205c390) at 0x7ff5420033a8

  REFCNT = 1

  FLAGS = (POK,READONLY,pPOK)

  PV = 0x7ff541c0d660 "foo"\0

  CUR = 3

  LEN = 16

But the moment you assign that string into a scalar, it's no longer readonly!

$ perl -MDevel::Peek -E 'my $foo = "foo"; Dump($foo)'

SV = PV(0x7fa36a804070) at 0x7fa36a8033a8

  REFCNT = 1

  FLAGS = (PADMY,POK,pPOK)

  PV = 0x7fa36a405980 "foo"\0

  CUR = 3

  LEN = 16

What if we use other approaches like Readonly.pm ? :

$ perl -MReadonly -MDevel::Peek -E 'Readonly::Scalar my $foo => "foo"; Dump($foo)'

SV = PVMG(0x7ff8b1059200) at 0x7ff8b10033a8

  REFCNT = 1

  FLAGS = (PADMY,GMG,SMG,RMG)

  IV = 0

  NV = 0

  PV = 0

  MAGIC = 0x7ff8b0d05820

    MG_VIRTUAL = &PL_vtbl_packelem

    MG_TYPE = PERL_MAGIC_tiedscalar(q)

    MG_FLAGS = 0x02

      REFCOUNTED

    MG_OBJ = 0x7ff8b10034c8

    SV = IV(0x7ff8b10034b8) at 0x7ff8b10034c8

      REFCNT = 1

      FLAGS = (ROK)

      RV = 0x7ff8b10958a8

      SV = PVMG(0x7ff8b10591a0) at 0x7ff8b10958a8

        REFCNT = 1

        FLAGS = (PADMY,OBJECT,POK,pPOK)

        IV = 0

        NV = 0

        PV = 0x7ff8b0d03ce0 "foo"\0

        CUR = 3

        LEN = 16

        STASH = 0x7ff8b104baf0 "Readonly::Scalar"

Eek.

If you only support "use constant", then the behavior changes for a very particular subset of inputs. the moment you have enough variables, I can see all sorts of havoc breaking loose. And supporting all those different ways to mark a string constant doesn't sound realistic.

Now, taking a step back. If the user is ABSOLUTELY sure that none of the above worst cases are going to happen, then maybe the correct way to handle this is to allow changing the default filtering to an arbitrary Perl subroutine:

my $tx = Text::Xslate->new( default_filter => sub { "unsafe filtering that tries to check for READONLY flags" } );

This to me sounds... okay. Because then you're clearly stating you want to shoot your own foot :)

However, since this operation would be applied to pretty much all variable output, it would certainly slow down Text::Xslate, and again, I'm not sure if it's worth it, which brings back to my earlier proposition to explicitly marking them strings raw before hand

--d

[Robert Messer]

I see what you are saying, but if somebody creating a string is going to add mal-formatted content, they could just as easily do that outside the evaluation blocks.  Either way it would be a bug and the page won't render correctly.  So you could say that the '<div>' here:

[% if ($foo) { '<div>' ~ $something ~ '</div>' } %]

is no more or less secure or likely to be malformed than if you break up the block and do it the other way:

[% if ($foo) { %]<div>[% $something %]</div>[% } %]

Plus we sometimes have quite complicated templates, and if we have mark_raw all over the place, it is more of a concern that we won't catch somebody doing this: [% mark_raw($user_input) %].  In fact it would be more secure if mark_raw was not available at all from within template code.  It would also work for us if mark_raw only worked on static strings, so that it was not possible to do mark_raw($user_input), just mark_raw('my static string').  But of course normally a function doesn't know where the data it gets came from, nor should it.

We could try to add all static strings as data and mark them raw in the incoming hashref, but sometimes that creates too much separation and makes it confusing.  For example, we have a case something like this:

<div class="whatever" [% if ($width) { raw('style="width:') ~ $width ~ raw('"') } %] />

If we set up a bunch of little strings like this as data, that would make it harder when the .tx file changes.  We are better off saying raw('"').  It wouldn't be worth slowing things down or using a perl sub as a default filter to get rid of it.

And the raw() calls aren't that bad anyway, just a bit of noise and a small thing in the grand scheme of things.  Text::Xslate is working well for us, it is an excellent templating engine.  If there is no straightforward and fast solution to get rid of the calls to mark_raw, then we'll just leave them in where they are needed, and also put static strings outside of evaluated blocks in some places where it makes more sense that way. 

Thanks for your responses on this.

Rob