html-template-users — Discussion of HTML::Template and related modules

[htmltmpl] Template from DB, not file

[Joakim S. <jo...@st...>]

Hi there,

Is it possible to use HTML::TEMPLATE with the template as a string from a 
database rather than a file? I've looked in the examples and some of the 
documentation but everywhere the template seems to be stored in a file.


Best regards
Joakim Syversen 

[htmltmpl] Parsing bug under Perl 5.8 and HTML::Template 2.8

[Beard, T. <Ter...@Gl...>]

I've encounter a bug that has apparently happened before.

When using scalar reference template, HTML::Template dies producing the
error:
"HTML::Template->new() : Unknown or unmatched TMPL construct at
/fake/path/for/non/file/template "

I updated the error line, and it produces the output:
"HTML::Template->new() : Unknown or unmatched TMPL [T0B5AR] construct at
/fake/path/for/non/file/template : line 6.
Chunk=3D<tmpl_var name=3D"case_id"> . The reference number is located in =
the
subject
 heading of this email.  at
/export/opt/clarify/perl/lib/HTML/Template.pm line 2297, <F> line 56."

I'm only using the tmpl_var construct. The template itself contains
several of them, but it doesn't always die on the same one. Sometimes
it's the first one, sometimes it the third. There doesn't appear to be a
pattern to it. The same template loaded from a file works perfectly (or
has thus far).

I should note that the text for the template is being supplied from a
.Net based Web Service, and the request and response encoding is set to
UTF-8 (default).=20

When I replace the line: "$which =3D uc($1);" with "$which =3D $1; =
$which =3D~
tr/a-z/A-Z/", it works flawlessly.

The really weird thing is that I added some more debugging, and calling
the uc function as part of an output string, such as:
'print "1=3D[$1][".uc($1)."]" 2=3D[$2] 3=3D[$3] 4=3D[$4]\n";' it works =
like a
charm.

Even more weird is that this works perfectly on our development server,
which is a near duplicate of our QA server where this is occurring.
There is some difference in the installed modules, but nothing that
should impact this (I think).

Please let me know if you need further information to help me diagnose
this.

Thanks!

[htmltmpl] NAME attribute value format

[Jesse M. <htm...@je...>]

Hello,

I'm looking for clarification about what values are permitted for NAME 
attributes, e.g. in <TMPL_VAR NAME="WHATEVER">.

The documentation ( 
http://html-template.sourceforge.net/html_template.html ) says that these 
characters are allowed: Numbers, letters, '.', '/', '+', '-' and '_'.

I've been using colons (":") in my NAME values for some time now (with 
HTML::Template v2.7) without realizing that it's not "allowed", and it's 
worked as expected.

I just took a look at the source code, and it appears that the software will 
parse NAME attribute values including almost any character.  I don't know if 
any additional checking of those values is performed later.  I personally 
think that it's best for software to actually restrict the format of input 
to what's documented, as much as possible.

In any case, I need to know for sure what values are permitted.  I'm writing 
an application, that I'll be releasing as free software, that interacts with 
HTML::Template and it's very important that I write the software and 
documentation with the proper format of NAME values taken into account.

For the values used in my application that correspond to <TMPL_VAR> NAME 
values, I'm using the same format as HTML 4.01 ID / NAME tokens:
[a-zA-Z][a-zA-Z0-9_:.-]*

(See http://www.w3.org/TR/html4/types.html#type-id )

So, the only issue for me is the ":" character, but I don't plan to change 
the format definition I'm using and I need to know whether I can officially 
use that character in HTML::Template NAME values or if I have to warn my 
users about the issue.

Thanks for the great piece of software!  Obviously the best thing for me 
would be if I could legitimately use the ":" character, but the most 
important thing is to find out definitely one way or the other.

Thanks,
Jesse McCarthy

http://www.nextlevelarts.com/


P.S. The documentation I linked to contains information about the mailing 
list that doesn't match information found elsewhere on the site and in the 
v2.8 documentation on CPAN -- not sure if that information is still viable? 

Re: [htmltmpl] Patch for new option "force_untaint" - comments?

[Sven N. <sve...@sv...>]

Sven Neuhaus wrote:
> The "force_untaint" option. This option makes sure that no tainted values
> are set in the template.
> [...]
> Please let me know what you think. I believe this would be very helpful in
> preventing cross-site-scripting (CSS) bugs.

No feedback? :-(
I believe honoring perl's taint flag in HTML::Template is a more perlish and
natural solution to the XSS problem than the proposal by Shlomi Fish
("Suggestion on how to eliminate Cross-site-scripting (XSS) bugs for
good."). Combine this with DBIs TaintIn-flag and it gets pretty hard to
accidentally leave XSS bugs in.

I've been using the patched version of HTML::Template for two weeks now
without problems. I have modified the 2nd patch slightly so it tells you
which parameter is tainted in some easy cases (like the first patch did).

-Sven

Re: [htmltmpl] LOOPING error [code included]

[Robert H. <si...@gm...>]

I found where to put the "die_on_bad_params". Seems a shame to have to 
do that.

Robert

Re: [htmltmpl] LOOPING error [code included]

[Benjamin W. <war...@sb...>]

On Dec 5, 2006, at 9:44 AM, Michael Peters wrote:
> Setting die_on_bad_params to false will let you get around this  
> problem, but that causes
> problems if you wanted strict templates in the first place.

You can also get around the problem without disabling variable-name  
checking for the rest of the template, albeit in a remarkably hackish  
way: just add the appropriate version of

<TMPL_IF NAME="my_missing_var"></TMPL_IF>

on the inside of the loop that doesn't actually make use of the  
variable, and it will show up as a valid variable for both loops.

	--Ben Warfield

Re: [htmltmpl] LOOPING error [code included]

[Robert H. <si...@gm...>]

Michael Peters wrote:
> 
> Roger Burton West wrote:
>> On Tue, Dec 05, 2006 at 09:20:40AM -0500, Robert Hicks wrote:
>>
>>> I have 3 separate loops using each of those. I am converting something
>> >from TT->HT and this works fine in TT. Is there something about how HT
>>> loops that is biting me? I am sure "I" am forgetting something.
>> In my experience, multiple loops of the same name will throw up this
>> problem - only the first loop gets checked. The simplest solution is to
>> set die_on_bad_params to zero.
> 
> He's right. H::T isn't very thorough when checking for the existence of
> variables in loops if the same loop is used more than once. Setting
> die_on_bad_params to false will let you get around this problem, but that causes
> problems if you wanted strict templates in the first place.
> 
> Fixing this has been one of my TODOs for a while... Stupid $work and $life :)
> 
Forgive me if this is ignorant, I am not far along the path:

I have this:

my $rs = $sth->fetchall_arrayref(
          {
              BILL_CODE => 1,
              LABOR_CATEGORY => 1,
              LABOR_CATEGORY_DESCRIPTION => 1,
          }
      );


And I have this:

my $employee_info = $self->_get_employee_info();

Can I not reference those in separate calls:

$template->param(
     one => $employee_info[0],  # not sure what the construct would be
     two => $employee_info[1],
     three => $employee_info[2],
),

If not how do I set that param to 0 in a CA fashion?

Robert

Re: [htmltmpl] LOOPING error [code included]

[Michael P. <mp...@pl...>]

Roger Burton West wrote:
> On Tue, Dec 05, 2006 at 09:20:40AM -0500, Robert Hicks wrote:
> 
>> I have 3 separate loops using each of those. I am converting something
>>from TT->HT and this works fine in TT. Is there something about how HT
>> loops that is biting me? I am sure "I" am forgetting something.
> 
> In my experience, multiple loops of the same name will throw up this
> problem - only the first loop gets checked. The simplest solution is to
> set die_on_bad_params to zero.

He's right. H::T isn't very thorough when checking for the existence of
variables in loops if the same loop is used more than once. Setting
die_on_bad_params to false will let you get around this problem, but that causes
problems if you wanted strict templates in the first place.

Fixing this has been one of my TODOs for a while... Stupid $work and $life :)

-- 
Michael Peters
Developer
Plus Three, LP

Re: [htmltmpl] LOOPING error [code included]

[Roger B. W. <ro...@fi...>]

On Tue, Dec 05, 2006 at 09:20:40AM -0500, Robert Hicks wrote:

>I have 3 separate loops using each of those. I am converting something
>from TT->HT and this works fine in TT. Is there something about how HT
>loops that is biting me? I am sure "I" am forgetting something.

In my experience, multiple loops of the same name will throw up this
problem - only the first loop gets checked. The simplest solution is to
set die_on_bad_params to zero.

Roger

[htmltmpl] LOOPING error [code included]

[Robert H. <si...@gm...>]

Error executing run mode 'display_employee_page':
HTML::Template->output() : fatal error in loop output : HTML::Template :
Attempt to set nonexistent parameter 'labor_category' - this parameter
name doesn't match any declarations in the template file :
(die_on_bad_params => 1) at C:/Perl/site/lib/HTML/Template.pm line 2997
  at lib//Trakker.pm line 142
  at D:/www/DEV/test/index.cgi line 14



I have 1 SQL call bringing back 3 values (BILL_CODE, LABOR_CATEGORY,
LABOR_CATEGORY_DESCRIPTION).

I have 3 separate loops using each of those. I am converting something
from TT->HT and this works fine in TT. Is there something about how HT
loops that is biting me? I am sure "I" am forgetting something.

Here is the SQL sub:

sub _get_employee_info {
     my $self = shift;

     my $sth = $self->dbh->prepare( select_employee_info() );
     $sth->execute;

     my $rs = $sth->fetchall_arrayref(
         {
             BILL_CODE => 1,
             LABOR_CATEGORY => 1,
             LABOR_CATEGORY_DESCRIPTION => 1,
         }
     );

     return $rs;
}

Here is the CA+HT runmode sub:
sub display_employee_page : Runmode {
     my $self = shift;
     my $errs = shift;

     my $q = $self->query;

     my $template = $self->load_tmpl( 'addemployee.tmpl' );

     my $employee_info = $self->_get_employee_info();

     $template->param(
         MODDATE => '04-Dec-2006',
         VERSION => $VERSION,
         EMPLOYEE_INFO => $employee_info,
     );

     $template->param($errs) if $errs;

     return $template->output();
}


Here is the TEMPLATE form:

<form name="tasks" action="" method="post">
<table id="formcontent" summary="main form for task input">

<tr>
<td class="labelcell">Last Name:</td>
<td>
<input type="text" name="last_name" size="25" /><TMPL_VAR err_last_name>
</td>
</tr>

<tr>
<td class="labelcell">First Name:</td>
<td>
<input type="text" name="first_name" size="25" /><TMPL_VAR err_first_name>
</td>
</tr>

<tr>
<td class="labelcell">Billing Code:</td>
<td>
<select name="bill_code">
<option value=""></option>
<TMPL_LOOP NAME=EMPLOYEE_INFO>
<option value="<TMPL_VAR NAME=BILL_CODE>"><TMPL_VAR NAME=BILL_CODE></option>
</TMPL_LOOP>
</select><TMPL_VAR err_bill_code>
</td>
</tr>

<tr>
<td>&nbsp;</td>
<td>- or -</td>
</tr>

<tr>
<td class="labelcell">Add Code:</td>
<td>
<input type="text" name="add_bill_code" size="10" /><TMPL_VAR
err_bill_code_or_add_bill_code>
</td>
</tr>

<tr>
<td class="labelcell">Labor Category:</td>
<td>
<select name="labor_category">
<option value=""></option>
<TMPL_LOOP NAME=EMPLOYEE_INFO>
<option value="<TMPL_VAR NAME=LABOR_CATEGORY>"><TMPL_VAR
NAME=LABOR_CATEGORY></option>
</TMPL_LOOP>
</select><TMPL_VAR err_labor_category>
</td>
</tr>

<tr>
<td>&nbsp;</td>
<td>- or -</td>
</tr>

<tr>
<td class="labelcell">Add Category:</td>
<td>
<input type="text" name="add_labor_category" size="9" /><TMPL_VAR
err_add_labor_category>
</td>
</tr>

<tr>
<td class="labelcell">Description:</td>
<td>
<select name="labor_category_description">
<option value=""></option>
<TMPL_LOOP NAME=EMPLOYEE_INFO>
<option value="<TMPL_VAR NAME=LABOR_CATEGORY_DESCRIPTION"><TMPL_VAR
NAME=LABOR_CATEGORY_DESCRIPTION></option>
</TMPL_LOOP>
</select><TMPL_VAR err_labor_category_description>
</td>
</tr>

<tr>
<td>&nbsp;</td>
<td>- or -</td>
</tr>

<tr>
<td class="labelcell">Add Description:</td>
<td>
<input type="text" name="add_labor_category_description" size="30"
/><TMPL_VAR err_add_labor_category_description>
</td>
</tr>

<tr>
<td>&nbsp;</td>
<td>
<input type="submit" value="Save Employee" class="formbutton" />
</td>
</tr>

</table>

</form>

[htmltmpl] Template variables with dots in HTML::Template::Expr

[Roel P. \(B. & Cnossen\) <ro...@bn...>]

As a WebGUI user/developer I (and all other WebGUI people :-) have the
need to be able to use variables with dots, and this is not possible at
the moment. However there's a simple solution:

in the HTML::Template::Expr module the definition of 'var' in the
$GRAMMAR section needs the inclusion of the dot character:

var : /[A-Za-z_][A-Za-z0-9_.]*/ { \\\$item[1 <file://$item[1/> ] }

=20

This way dots are allowed inside the variable names! Would it be
possible to include this solution in the next release?

Cheers, Roel Plieger.

=20

Re: [htmltmpl] Next H::T release.

[Mathew R. <mat...@ne...>]

>> One could argue that a template variable of "user_some_funky_permission"
>> should be created by the application.   But then why should the
>> application programmer create the cross-product of every template
>> variable; it is up to the template designer to decide when the
>> some_funky_menu gets displayed.
>>     
>
> I agree with what Mathew says (especially about commonly used filters) except
> for this quoted bit.  I seriously disagree with the idea that designers should
> be doing bitwise operations.  I think it is one step too far down the road of
> integrating code and presentation.  That being said - I would never use it so
> it's inclusion/exclusion does not concern me.  I do believe it goes against the
> spirit of H::T though - maybe it should be in a plug-in.
>   
I agree - in fact it already is a plugin => this functionality would
only be available in H::T::E.

Mathew

Re: [htmltmpl] Next H::T release.

[Chris B. <htm...@pa...>]

They say that Mathew Robertson, on or about 03.Dec.2006 19:27, declaimed:=

> One could argue that a template variable of "user_some_funky_permission=
"
> should be created by the application.   But then why should the
> application programmer create the cross-product of every template
> variable; it is up to the template designer to decide when the
> some_funky_menu gets displayed.

I agree with what Mathew says (especially about commonly used filters) ex=
cept
for this quoted bit.  I seriously disagree with the idea that designers s=
hould
be doing bitwise operations.  I think it is one step too far down the roa=
d of
integrating code and presentation.  That being said - I would never use i=
t so
it's inclusion/exclusion does not concern me.  I do believe it goes again=
st the
spirit of H::T though - maybe it should be in a plug-in.

Cheers,
Chris
--=20
Christopher Beck  -  http://pacanukeha.wordpress.com
Implicit in the term =C2=91national defense=C2=92 is the notion of defend=
ing
those values and ideas which set this Nation apart. . . . It would
indeed be ironic if, in the name of national defense, we would
sanction the subversion of . . . those liberties . . . which makes the
defense of the Nation worthwhile. -- Justice Warren in U.S. v. Robel

Re: [htmltmpl] Next H::T release.

[Mathew R. <mat...@ne...>]

>> Bugs:
>> ----
>>
>> - Use "UNIVERSIAL::isa(...)" and "SUPER" in the appropriate places
>> so that sub-classes work correctly.
>>     
>
> Why should H::T use SUPER?
>   
If H::T::E calls H::T->do_something, then when a subclass overrides that
do_something method, the subclass method isn't called.  ie: this is
needed when subclassing H::T::E.  It also important in the
"_new_from_loop" case where you want your subclass hooks to be executed.

>> Optimisations
>> -------------
>>
>> - Allow output()ing use a scalar reference.  This allows the ref to
>> by used by further instances of H::T (or other code), thus saving a
>> copy of the text buffer.
>>     
>
> ++
>
>   
>> - Use a more powerful regular expression to chunk up the template.
>> Since Perl's regex parsing is quite fast, the parsing of a template
>> is quicker since the chunks now always begin with <TMPL_ (rather
>> than simply splitting on '<'
>>     
>
> I'm not sure I follow. What regex would you use instead?
>   
m!(?=<(?:\!--\s*)?/?[Tt][Mm][Pp][Ll]_)!

instead of

m/(?=<)/


The regex itself may be slower - I haven't tested its parse performance,
but since the parse_stack is now much smaller the output()ing is faster
by a small margin.

>> - Provide a profiling option which dumps some profiling statistics.
>>  This can be used to enhance H::T performance.  [ This is how I was
>> able to determine where to look for the performance patch. ]
>>     
>
> ++
>
>   
>> - Make Vanguard compatibility available as a filter rather than part
>> of Template.pm
>>     
>
> ++, or deprecate it and then drop it completely. Sam/Jesse, is anyone still
> using that who also upgrades H::T?
>
>   
>> - Provide a method where a hashref can be passed in as the
>> value-type for a param, so that it self-expands into the
>> corresponding TMPL_VAR's / TMPL_LOOP's.  Often data comes out of a
>> database API as a hash-per-row; depending on how the programmer is
>> using H::T, this can minimise data copying.
>>     
>
> I'm not sure I follow here. hashref's are already used for loops. Are you
> talking about having something like this:
>   $tmpl->param(
>     person => {
>       first_name => 'Michael',
>       last_name  => 'Peters',
>     }
>   );
>
> And then in the template something like this:
>   <tmpl_var person.first_name> <tmpl_var person.last_name> ?
>
> If so, then HTML::Template::Plugin::Dot may be more what you're looking for.
>   
I think there are multiple separate pieces of functionality provided by
HT::P::Dot  which are useful in their own right, but yes.  In fact, the
implementation provided in ::Dot would probably run slightly faster than
my implementation under some cases (although I think it wouldn't handle
new TMPL_XXX tags - not sure...)

Except that HT::Pluggable doesn't subclass from H::T::E  (... in some
very specific hack'ish scenarios using H::T::E is a blessing... I'd
really like to keep using it).


Note also, HT::Pluggable suffers a little from the same lack of
function-call hooks, ie: it re-implements the param() method.  In
reality, the function-call dispatch mechanism may need to implement
something like the implementation in DBI / DBD, rather than subclassing
H::T / H::T::E.

>> Enhacements
>> -----------
>>
>> - Modify the ESCAPE module handling so that it dynamically loads the
>> requested module.  This would allow people to create their own
>> escape code to suite their specific needs. eg: H::T is often used to
>> output text which is not HTML - in those cases the rules for
>> escaping can be different; or, someone may want to build an escape
>> module that chains existing modules.
>>     
>
> This might be useful, but I'm sort of ambivalent.\
>
>   
>> - Provide hooks to allow sub-classes to implement their own TMPL_xxx
>> syntax.  This is particularly useful when a particular H::T::E
>> expression frequently uses a function call -> it becomes possible to
>> promote that function call to become first class TMPL_XXX syntax.
>>  This makes it easier on the template programmer and allows the
>> expression to run faster too.
>>     
>
> Again, possibly useful. Maybe a compelling use case would help spur the discussion.
>   
I provided an example.

Another...  I provide the TMPL_CATGETS call to template developers; it
takes constant-strings and/or template variables.

The TMPL_CATGETS expression takes the arguments and applies them to the
language engine.  The language engine returns parse_stack objects.

Later when the page is rendered, the language-engine objects get
executed returning the appropriate language translation.  Since language
translations are updated daily, the rendering will get the latest
translations.


To provide the ability to create new TMPL_XXX tags, some function calls
need to be added where currently H::T simply dies.

>> - Allow '.' as a template variable.  This makes it a bit easier to
>> group similar variables
>>
>> - Allow the '&' and '|' operators in expressions.
>>     
>
> You want bitwise and/or? use case?
>   
yes

<TMPL_IF EXPR="user_permissions & some_funky_menu_permission">

where "user_permissions" is the users' permission as a bit-set, and
"some_funky_menu_permissions" is a bit-mask.

One could argue that a template variable of "user_some_funky_permission"
should be created by the application.   But then why should the
application programmer create the cross-product of every template
variable; it is up to the template designer to decide when the
some_funky_menu gets displayed.

>> - Package some predefined / common filters.
>>     
>
> like...?
>   
like every filter that has been posted to the users-list.  Here are a few:

#
# allow trailing slash in <TMPL_xxx /> tags
#
sub allow_trailing_slash {
  my $filter = sub {
    my $text_ref = shift;
    my $match = qr/(<[Tt][Mm][Pp][Ll]_[^>]+)\/>/;
    $$text_ref =~ s/$match/$1>/g;
  };
  return $filter;
}

#
# Translate the SSI "include virtual" into a template include:
#
sub ssi_include_virtual {
  my $filter = sub {
    my $text_ref = shift;
    my $match = qr/<!--\s*#include virtual="[\/]?(.+?)"\s*-->/i;
    $$text_ref =~ s/$match/<TMPL_INCLUDE NAME="$1">/g;
  };
  return $filter;
}

#
# Decompress gzip-comressed templates
#
sub gzipped_templates {
  eval { require Compress::Zlib; };
  croak "To use gzip-compressed templates, you need into install
Compress::Zlib" if ($@);
  my $filter = sub {
    my $text_ref = shift;
    require Compress::Zlib;
    $text_ref = Compress::Zlib::uncompress($text_ref);
  };
  return $filter;
}

#
# Allow template variables to use %var% syntax
#
sub percent_variables {
  my $filter = sub {
    my $text_ref = shift;
    my $match = qr/%([-\w\/\.+]+)%/;
    $$text_ref =~ s/$match/<TMPL_VAR NAME="$1">/g;
  };
  return $filter;
}

#
# Strip newline's following TMPL_XXX statements
#
sub strip_tmpl_newline {
  my $filter = sub {
    my $text_ref = shift;
    $$text_ref =~ s/(<TMPL_[~>]>)[\r\n]+/$1/g;
  };
}


>> - Package a preload mechanism for preloading templates into the
>> memory/file caches.
>>     
>
> ++
>
> Krang already does this on it's own to load all it's H::T template. Why not put
> the code into H::T itself.
>
>   
>> - Now that H::T::E supports a unary context, provide some unary
>> operators too. eg: the 'not' operator
>>     
>
> ++
>
>   
>> - Provide TMPL_ELSIF ... This simply makes it easier on the template
>> programmer.  And incurs no runtime overhead to boot.
>>     
>
> ++++ - this would prevent a lot of ugly looking templates that I've been guilty
> of making.
>
>   
>> - Provide '__even__' when using loop-context-vars.  Yes it is
>> redundant but some people's brain hurts when using inverted contexts
>> (ie: TMPL_UNLESS + TMPL_ELSE).  In particular, it works well when
>> used with TMPL_ELSIF.
>>     
>
> Again, I'm ambivalent.
>
>   
>> - Allow recursive H::T invocations for those cases where people
>> _realy_ want to do <TMPL_INCLUDE <TMPL_VAR>>... [ and thus deal with
>> the consequences of such an act... :-) ]
>>     
>
> Would this just apply to tmpl_include or to all vars? If everything, then it
> would need a 2 pass parser right? That would be pretty painful performance wise.
>   
yes, this needs the parser to run multiple times, and it runs quite a
bit slower to boot - what a deal....  However, H::T is not always used
in an online web environment - you may be able to tolerate some
performance loss at the expense of making your templates more dynamic.

Mathew

Re: [htmltmpl] Next H::T release.

[Michael P. <mp...@pl...>]

Mathew Robertson wrote:
> Hi Sam
> 
> Here are a few ideas which would result in minimal changes to H::T
> which would allow others to use it in different ways.  Note that in
> most cases I have working code which implements these ideas.

Just to chime in and mention what I would find useful or not.
> 
> Bugs:
> ----
> 
> - Use "UNIVERSIAL::isa(...)" and "SUPER" in the appropriate places
> so that sub-classes work correctly.

Why should H::T use SUPER?

> Optimisations
> -------------
> 
> - Allow output()ing use a scalar reference.  This allows the ref to
> by used by further instances of H::T (or other code), thus saving a
> copy of the text buffer.

++

> - Use a more powerful regular expression to chunk up the template.
> Since Perl's regex parsing is quite fast, the parsing of a template
> is quicker since the chunks now always begin with <TMPL_ (rather
> than simply splitting on '<'

I'm not sure I follow. What regex would you use instead?

> - Provide a profiling option which dumps some profiling statistics.
>  This can be used to enhance H::T performance.  [ This is how I was
> able to determine where to look for the performance patch. ]

++

> - Make Vanguard compatibility available as a filter rather than part
> of Template.pm

++, or deprecate it and then drop it completely. Sam/Jesse, is anyone still
using that who also upgrades H::T?

> - Provide a method where a hashref can be passed in as the
> value-type for a param, so that it self-expands into the
> corresponding TMPL_VAR's / TMPL_LOOP's.  Often data comes out of a
> database API as a hash-per-row; depending on how the programmer is
> using H::T, this can minimise data copying.

I'm not sure I follow here. hashref's are already used for loops. Are you
talking about having something like this:
  $tmpl->param(
    person => {
      first_name => 'Michael',
      last_name  => 'Peters',
    }
  );

And then in the template something like this:
  <tmpl_var person.first_name> <tmpl_var person.last_name> ?

If so, then HTML::Template::Plugin::Dot may be more what you're looking for.

> Enhacements
> -----------
> 
> - Modify the ESCAPE module handling so that it dynamically loads the
> requested module.  This would allow people to create their own
> escape code to suite their specific needs. eg: H::T is often used to
> output text which is not HTML - in those cases the rules for
> escaping can be different; or, someone may want to build an escape
> module that chains existing modules.

This might be useful, but I'm sort of ambivalent.

> - Provide hooks to allow sub-classes to implement their own TMPL_xxx
> syntax.  This is particularly useful when a particular H::T::E
> expression frequently uses a function call -> it becomes possible to
> promote that function call to become first class TMPL_XXX syntax.
>  This makes it easier on the template programmer and allows the
> expression to run faster too.

Again, possibly useful. Maybe a compelling use case would help spur the discussion.

> 
> - Allow '.' as a template variable.  This makes it a bit easier to
> group similar variables
> 
> - Allow the '&' and '|' operators in expressions.

You want bitwise and/or? use case?
> 
> - Package some predefined / common filters.

like...?

> - Package a preload mechanism for preloading templates into the
> memory/file caches.

++

Krang already does this on it's own to load all it's H::T template. Why not put
the code into H::T itself.

> - Now that H::T::E supports a unary context, provide some unary
> operators too. eg: the 'not' operator

++

> - Provide TMPL_ELSIF ... This simply makes it easier on the template
> programmer.  And incurs no runtime overhead to boot.

++++ - this would prevent a lot of ugly looking templates that I've been guilty
of making.

> - Provide '__even__' when using loop-context-vars.  Yes it is
> redundant but some people's brain hurts when using inverted contexts
> (ie: TMPL_UNLESS + TMPL_ELSE).  In particular, it works well when
> used with TMPL_ELSIF.

Again, I'm ambivalent.

> - Allow recursive H::T invocations for those cases where people
> _realy_ want to do <TMPL_INCLUDE <TMPL_VAR>>... [ and thus deal with
> the consequences of such an act... :-) ]

Would this just apply to tmpl_include or to all vars? If everything, then it
would need a 2 pass parser right? That would be pretty painful performance wise.

-- 
Michael Peters
Developer
Plus Three, LP

[htmltmpl] Patch for new option "force_untaint"

[Sven N. <sve...@sv...>]

Hi,

I have opened a new bug (#23592) on rt.cpan.org for a new feature request:
The "force_untaint" option. This option makes sure that no tainted values
are set in the template.
If set to 1, only TMPL_VARs with no ESCAPE-attribute must be untainted,
if set to 2, every TMPL_VAR must be untainted.

I have attached a patch to the bug that implements this feature.

Please let me know what you think. I believe this would be very helpful in
preventing cross-site-scripting (CSS) bugs.

Regards,
-Sven Neuhaus

Re: [htmltmpl] Next H::T release.

[Mathew R. <mat...@ne...>]

Hi Sam

Here are a few ideas which would result in minimal changes to H::T
which would allow others to use it in different ways.  Note that in
most cases I have working code which implements these ideas.

Bugs:
----

- Use "UNIVERSIAL::isa(...)" and "SUPER" in the appropriate places
so that sub-classes work correctly.

Optimisations
-------------

- Allow output()ing use a scalar reference.  This allows the ref to
by used by further instances of H::T (or other code), thus saving a
copy of the text buffer.

- Optimise H::T::LOOP's output() method.  It current contains some
'if' checks that dont need to exist.

- Use a more powerful regular expression to chunk up the template.
Since Perl's regex parsing is quite fast, the parsing of a template
is quicker since the chunks now always begin with <TMPL_ (rather
than simply splitting on '<'

- Provide a profiling option which dumps some profiling statistics.
 This can be used to enhance H::T performance.  [ This is how I was
able to determine where to look for the performance patch. ]

- Make Vanguard compatibility available as a filter rather than part
of Template.pm

- Provide a method where a hashref can be passed in as the
value-type for a param, so that it self-expands into the
corresponding TMPL_VAR's / TMPL_LOOP's.  Often data comes out of a
database API as a hash-per-row; depending on how the programmer is
using H::T, this can minimise data copying.

Enhacements
-----------

- Modify the ESCAPE module handling so that it dynamically loads the
requested module.  This would allow people to create their own
escape code to suite their specific needs. eg: H::T is often used to
output text which is not HTML - in those cases the rules for
escaping can be different; or, someone may want to build an escape
module that chains existing modules.

- Provide hooks to allow sub-classes to implement their own TMPL_xxx
syntax.  This is particularly useful when a particular H::T::E
expression frequently uses a function call -> it becomes possible to
promote that function call to become first class TMPL_XXX syntax.
 This makes it easier on the template programmer and allows the
expression to run faster too.

- Allow '.' as a template variable.  This makes it a bit easier to
group similar variables

- Allow the '&' and '|' operators in expressions.

- Package some predefined / common filters.

- Package a preload mechanism for preloading templates into the
memory/file caches.

- Now that H::T::E supports a unary context, provide some unary
operators too. eg: the 'not' operator

- Provide TMPL_ELSIF ... This simply makes it easier on the template
programmer.  And incurs no runtime overhead to boot.

- Provide '__even__' when using loop-context-vars.  Yes it is
redundant but some people's brain hurts when using inverted contexts
(ie: TMPL_UNLESS + TMPL_ELSE).  In particular, it works well when
used with TMPL_ELSIF.

- Allow recursive H::T invocations for those cases where people
_realy_ want to do <TMPL_INCLUDE <TMPL_VAR>>... [ and thus deal with
the consequences of such an act... :-) ]

cheers,
Mathew


Sam Tregar wrote:
> 
> On Fri, 17 Nov 2006, Mathew Robertson wrote:
> 
>> sure thing - see attached.
>>
>> I have a few other ideas about enhancements which you may or may not
>> be interested in - should I furnish a list of ideas?
> 
> Sure, I don't mind taking a look.  I'm more likely to be interested in
> bug fixes than new features at this point, but I'll try to keep an
> open mind.
> 
> -sam

Re: [htmltmpl] The sf.net Subversion Repository and the Phalanx One

[Sam T. <sa...@tr...>]

On Thu, 16 Nov 2006, James Keenan wrote:

> Sam, we'd still welcome feedback.

Thanks Jim.  I'll see what I can do this weekend.

-sam

Re: [htmltmpl] Next H::T release.

[Sam T. <sa...@tr...>]

On Fri, 17 Nov 2006, Mathew Robertson wrote:

> sure thing - see attached.
>
> I have a few other ideas about enhancements which you may or may not
> be interested in - should I furnish a list of ideas?

Sure, I don't mind taking a look.  I'm more likely to be interested in
bug fixes than new features at this point, but I'll try to keep an
open mind.

-sam

Re: [htmltmpl] Next H::T release.

[Mathew R. <mat...@ne...>]

sure thing - see attached.

I have a few other ideas about enhancements which you may or may not
be interested in - should I furnish a list of ideas?

Mathew

Sam Tregar wrote:
> On Fri, 17 Nov 2006, Mathew Robertson wrote:
> 
>> Since you mentioned that you are hoping to do a release sooner or
>> later, could you consider the following two changes:
> 
> Good stuff.  Any chance I could get it in unified diff format (diff
> -u)?
> 
> -sam

Re: [htmltmpl] Next H::T release.

[Sam T. <sa...@tr...>]

On Fri, 17 Nov 2006, Mathew Robertson wrote:

> Since you mentioned that you are hoping to do a release sooner or
> later, could you consider the following two changes:

Good stuff.  Any chance I could get it in unified diff format (diff
-u)?

-sam

[htmltmpl] Next H::T release.

[Mathew R. <mat...@ne...>]

Hi Sam,

Since you mentioned that you are hoping to do a release sooner or
later, could you consider the following two changes:

- bug file for included line numbers

Change this code (starting on line: 2318):

    # count newlines in chunk and advance line count
    $fcounter += scalar(@{[$chunk =~ m/(\n)/g]});
    # if we just crossed the end of an included file
    # pop off the record and re-alias to the enclosing file's info
    pop(@fstack), (*fname, *fcounter, *fmax) = \ (
@{$fstack[$#fstack]} )
      if ($fcounter > $fmax);

With this code:

    # count newlines in chunk and advance line count
    $fcounter += scalar(@{[$chunk =~ m/(\n)/g]});
    # if we just crossed the end of an included file
    # pop off the record and re-alias to the enclosing file's info
    while ($fcounter > $fmax) {
      my $counter_offset = $fcounter - $fmax;
      pop(@fstack), (*fname, *fcounter, *fmax) = \ (
@{$fstack[$#fstack]} );
      $fcounter += $counter_offset;
    }


This correctly rolls back the number of fstack entries so that line
number errors are correct for the given template.




- performance enhancement during output() phase

Change this code (starting line: 2622):

  # support the associate magic, searching for undefined params and
  # attempting to fill them from the associated objects.
  if (scalar(@{$options->{associate}})) {
    # prepare case-mapping hashes to do case-insensitive matching
    # against associated objects.  This allows CGI.pm to be
    # case-sensitive and still work with asssociate.
    my (%case_map, $lparam);
    foreach my $associated_object (@{$options->{associate}}) {
      # what a hack!  This should really be optimized out for
case_sensitive.
      if ($options->{case_sensitive}) {
        map {
          $case_map{$associated_object}{$_} = $_
        } $associated_object->param();
      } else {
        map {
          $case_map{$associated_object}{lc($_)} = $_
        } $associated_object->param();
      }
    }

    foreach my $param (keys %{$self->{param_map}}) {
      unless (defined($self->param($param))) {
      OBJ: foreach my $associated_object (reverse
@{$options->{associate}}) {
          $self->param($param, scalar
$associated_object->param($case_map{$associated_object}{$param})),
last OBJ
            if (exists($case_map{$associated_object}{$param}));
        }
      }
    }
  }

with this code:

  # support the associate magic, searching for undefined params and
  # attempting to fill them from the associated objects.
  if (scalar(@{$options->{associate}})) {
    my @undef_params;
    foreach my $param (keys %{$self->{param_map}}) {
      next if (defined $self->param($param));
      push @undef_params, $param;
    }
    if (scalar(@undef_params)) {
      my $value;
      # if case sensitive mode or no CGI objects, we can use the
fast path
      if ($options->{case_sensitive} or (grep { !/^1/ } map {
UNIVERSAL::isa($_,'HTML::Template') } @{$options->{associate}}) == 0) {
        foreach my $param (@undef_params) {
          foreach my $associated_object (reverse
@{$options->{associate}}) {
            $value = $associated_object->param($param);
            next unless (defined $value);
            $self->param($param, scalar $value);
            last;
          }
        }
      } else {
        my %case_map;
        foreach my $associated_object (@{$options->{associate}}) {
          map { $case_map{$associated_object}{lc($_)} = $_ }
$associated_object->param();
        }
        my $associated_param;
        foreach my $param (@undef_params) {
          foreach my $associated_object (reverse
@{$options->{associate}}) {
            $associated_param = $case_map{$associated_object}{$param};
            next unless (defined $associated_param);
            $value = $associated_object->param($associated_param);
            next unless (defined $value);
            $self->param($param, scalar $value);
            last;
          }
        }
      }
    }
  }

The can result form a small 10% speed up, to more than 20x speed up,
depending on the template.


regards,
Mathew

Re: [htmltmpl] The sf.net Subversion Repository and the Phalanx One

[James K. <jk...@ve...>]

On Nov 16, 2006, at 3:08 PM, html-template-users- 
re...@li... wrote:

>
> Date: Wed, 15 Nov 2006 15:59:44 -0500 (EST)
> From: Sam Tregar <sa...@tr...>
> Subject: Re: [htmltmpl] The sf.net Subversion Repository and the
> 	Phalanx One
> To: Shlomi Fish <sh...@ig...>
> Cc: htm...@li...
> Message-ID: <Pin...@hi...>
> Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
>
> On Wed, 15 Nov 2006, Shlomi Fish wrote:
>
>> A question if I may. Why weren't the tests and other changes that
>> were done to the Phalanx work on HTML-Template:
>>
>> * http://svn.perl.org/phalanx/HTML-Template/
>> * http://hew.ca/yapc/phalanx/slides/TABLE_OF_CONTENTS.html
>>
>> Integrated into the mainline HTML-Template at:
>>
>> https://svn.sourceforge.net/svnroot/html-template/
>
> Lack of motivation, I suppose.  As far as I know they never found or
> fixed a bug in HTML::Template.  At this point their repo is out of
> date and it would be a painful process to do the merge.
>



Here is the summary of the Perl Seminar NY Phalanx project work on  
HTML::Template which we submitted to Sam Tregar on June 11, 2005.  It  
was written in POD, so you might want to copy-and-paste it into a  
file and run perldoc on it.

For good measure, I've placed a tarball of the final version of our  
work on HTML::Template here:  http://thenceforward.net/perl/misc/HTML- 
Template-2.701.tar.gz

(You can get a tarball of our work on Text::Template in the same  
directory.)

Sam, we'd still welcome feedback.

Jim Keenan


=head1 SUMMARY

This document summarizes work done on Perl module  HTML::Template by
members of Perl Seminar New York as part of the Phalanx project.  We
discuss what we were able to accomplish as of June 11, 2005, as well as
what we were not able to do, what might remain to be done,
or what we think should be done.

=head1 Definitions

''We'' equals the Perl Seminar NY Phalanx hoplites who have contributed
to revisions as of the above date:  David H Adler (dha), Jim Keenan
(jkeenan), Marc Prewitt (mprewitt) and Alex Gill (alexgill).

''You'' equals Sam Tregar.

=head1 Diffs

In order to generate meaningful diff files, we increased C<$VERSION> to
2.701.  All diffs were generated against version 2.7 as taken from CPAN.
(We made no reference to the Sourceforge version of the distribution.)

We present diffs in three different formats, each of which has its
merits:

=over 4

=item svn diff

Diff files generated by Subversion's C<svn diff> command run
individually on each of the two files which have undergone revision so
far:

     MANIFEST
     Template.pm

Each diff file so generated will end with suffix C<.diff>.

=item difftidy

We run each of the revised files listed above through F<perltidy> and
then run the results through the system's F<diff> program.  This may --
or may not -- help to reduce spurious differences caused by, I<e.g.,>
elimination of trailing whitespace.  Each such diff file will end with
suffix C<.ovn>.

=item diffcode and diffpod

We wrote a Perl script based on the work of Perl Seminar NY member
Glenn Maciag
which produces one diff file for differences in code (F<diffcode>) and a
separate diff file for differences in documentation written in POD
format (F<diffpod>).  We found this useful for modules where code and
POD are interleaved because the content is more focused yet the line
numbers are preserved.

=back

Pick and choose!

=head1 Code Coverage

The Perlsemny Phalanx project used Paul Johnson's Devel::Cover module to
measure the extent to which HTML::Template's test suite actually tested
its code.  The following table shows HTML::Template's code coverage as
measured at two different points in time:  just before we began our
work, and where we stood on the above date after two joint hacking
sessions.  ''Version 175'' refers to entry 175 in the Phalanx Subversion
repository and is, for all practical purposes, equivalent to the current
version of HTML::Template found on CPAN, i.e., version 2.7.  Coverage
results are shown for statement, branch, condition and subroutine
coverage.

   Version 175
     File                     stmt    branch    cond     sub
     Template.pm              85.2      63.5    50.0    84.9

   Version 344
     File                     stmt    branch    cond     sub
     Template.pm              90.7      72.7    65.8    94.4

As the data suggest, HTML::Template already had a very high percentage
-- 85.2% -- of its statements covered before Phalanx began work on it.
However, the coverage of branches and conditions was lower, and some
subroutines were as yet completely uncovered.

As a result of Phalanx's work through early June 2005, statement
coverage was increased to over 90%, branch coverage improved to
over 72%, condition coverage increased by a quarter to over 65% and
subroutine coverage is now at more than 94%.

The major obstacle to even more improved coverage is technical:  Certain
features of HTML::Template are only available on OSes where one can
install IPC::ShareLite, Gtop, etc.  To date, we have not had access to
such a system, so features such as C<shared_cache>, C<double_cache> and
C<memory_debug> are so far untested.  However, we feel that some of what
we learned so far will be readily applicable to those features once we
have access to them.  So we can confidently expect even more
improvements in code coverage as our work continues.

=head2 A Note on POD Coverage

In the table above we present only the first four columns in a typical
Devel::Cover coverage report.  While Devel::Cover can provide POD
coverage as well, it does so only by relying on Perl extension
Pod::Coverage.  But Pod::Coverage, at least in its native format, only
reports accurately on documentation provided that the module author has
written the POD using subroutines as the titles of subsections.  In
certain cases you have done this but, like many other module authors,
you have not done this for all subroutines -- nor do we think you
necessarily should do this.  We saw no reason to rewrite the
documentation with new subsection headings simply to get a better
looking statistic out of Pod::Coverage and, in turn, Devel::Cover.
So we have not included the coverage column in the table above.

=head1 Revisions of Code

=head2 Refactoring of Code Repeated within C<new()> and  
C<_new_from_loop()>

The following code appeared in both C<new()> (beginning at 961) and
C<_new_from_loop()> (beginning at 1118):

     for (my $x = 0; $x <= $#_; $x += 2) {
       defined($_[($x + 1)]) or croak("HTML::Template->new() called  
with odd number of option parameters - should be of the form option  
=> value");
       $options->{lc($_[$x])} = $_[($x + 1)];
     }

It was refactored into a new subroutine, C<_load_supplied_options()>.
Example:

     $options = _load_supplied_options( [@_], $options);

Here is the new subroutine:

     sub _load_supplied_options {
         my $argsref = shift;
         my $options = shift;
         for (my $x = 0; $x <= $#_; $x += 2) {
           defined($_[($x + 1)]) or croak("HTML::Template->new()  
called with odd number of option parameters - should be of the form  
option => value");
           $options->{lc($_[$x])} = $_[($x + 1)];
         }
         return $options;
     }

=head2 Additional Error-Checking in C<new()>

The documentation makes clear that the caching features can only be used
when the source of a template is a file -- not when it is a string, an
array or a filehandle.  What would happen in the current version of
HTML::Template if someone tried to activate caching when the template
source was not a file?

Experimentation showed that the program would die a horrible death.
Multiple error messages would be emitted from subroutines several levels
below C<new()>, but these error messages would not clearly point to the
problem.  For example, calling this script:

     #!/usr/local/bin/perl
     use strict;
     use warnings;
     use HTML::Template;

     my ($stemplate, $template_string);

     open my $fh, 'templates/simple.tmpl'
         or die "Couldn't open simple.tmpl for reading: $!";
     {
         local $/;
         $template_string = <$fh>;
         seek $fh, 0, 0;
     }

     $stemplate = HTML::Template->new(
         type   => 'scalarref',
         source => \$template_string,
         cache  => 1,
     );

... generated these messages:

     HTML::Template->new() : Cannot find file ''. at
     /usr/local/lib/perl5/site_perl/5.8.4/HTML/Template.pm line 1293
             HTML::Template::_cache_key('HTML::Template=HASH(0x82b3cc)')
     called at /usr/local/lib/perl5/site_perl/5.8.4/HTML/Template.pm  
line 1269
             HTML::Template::_commit_to_cache('HTML::Template=HASH 
(0x82b3cc)')
     called at /usr/local/lib/perl5/site_perl/5.8.4/HTML/Template.pm  
line 1197
             HTML::Template::_init('HTML::Template=HASH(0x82b3cc)')  
called at
     /usr/local/lib/perl5/site_perl/5.8.4/HTML/Template.pm line 1083
             HTML::Template::new('HTML::Template', 'type', 'scalarref',
     'source', 'SCALAR(0x806f00)', 'cache', 1) called at templstr.pl  
line 16

We solved this problem by adding some additional error-checking code to
the constructor which causes the program to C<croak> if the user
attempts to call for caching with a non-file source.

     # check that cache options are not used with non-cacheable  
templates
     croak "Cannot have caching when template source is not file"
       if grep { exists($options->{$_}) } qw( filehandle arrayref  
scalarref)
         and
          grep {$options->{$_}} qw( cache blind_cache file_cache  
shared_cache
                                    double_cache double_file_cache );

All tests continued to pass.  Adding this code to the constructor meant
that some error-checking code in more internal subroutines (I<e.g.,>
C<_cache_key>) could safely be eliminated, thereby increasing the
total percentage of code covered.  (From this point forward, we'll refer
to this new code as the ''double-grep test.'')

=head2 Elimination of Unnecessary Tests for Definedness of Variables

=head3 Within C<new()>

In two cases you call for the constructor to C<croak> unless a
two-part condition is met.

=over 4

=item *

Beginning at 1032:

     if (exists($options->{filename})) {
         croak("HTML::Template->new called with empty filename  
parameter!")
           unless defined $options->{filename} and length $options-> 
{filename};
     }

=item *

Beginning at 1048:

     if ($options->{file_cache}) {
       # make sure we have a file_cache_dir option
       croak("You must specify the file_cache_dir option if you want  
to use file_cache.")
         unless defined $options->{file_cache_dir} and
           length $options->{file_cache_dir};
     ...

=back

When we performed coverage analysis on these two blocks, we noted that
the case where the I<first> part of each C<unless> clause was not  
defined
was uncovered.

     line  err      %     !l  l&&!r   l&&r   expr
     ----- --- ------ ------ ------ ------   ----
     1033  ***     66      0      1     58   defined $$options 
{'filename'} and length $$options{'filename'}
     1050  ***     33      0      0      5   defined $$options 
{'file_cache_dir'} and length $$options{'file_cache_dir'}

We hypothesized that in each case the first part of the
condition was I<always> defined at this point in the program.  If so, it
would not need to be tested and could safely be eliminated.  When we
tried to pass C<undef> as a value in these two cases, the code now
represented by C<_load_supplied_options> detected an 'unbalanced
key-value pair', threw an error and caused the program to C<croak> --
I<before> either of these two blocks of code was reached.

Hence, the two blocks could be simplified to read:

=over 4

=item *

     if (exists($options->{filename})) {
         croak("HTML::Template->new called with empty filename  
parameter!")
           unless length $options->{filename};
     }

=item *

     if ($options->{file_cache}) {
       # make sure we have a file_cache_dir option
       croak("You must specify the file_cache_dir option if you want  
to use file_cache.")
         unless length $options->{file_cache_dir};
     ...

=back

All tests continued to pass after the elimination of the superfluous
tests for definedness.  Since these lines no longer include an C<and>
condition, these lines are no longer reported by Devel::Cover as
conditions -- which means the total coverage ratio for conditions
increases.

=head3 Within C<_init()>

In a way very similar to the cases just described, there are a number of
situations where you call for the C<_init()> to C<croak> unless a
two-part condition is met.

=over 4

=item *

Beginning at 1162:

     } elsif ($options->{double_file_cache}) {
       # try the normal cache, return if we have it.
       $self->_fetch_from_cache();
       return if (defined $self->{param_map} and defined $self-> 
{parse_stack});

=item *

Beginning at 1170:

       # put it in the local cache if we got it.
       $self->_commit_to_cache()
         if (defined $self->{param_map} and defined $self-> 
{parse_stack});

=item *

Beginning at 1184:

     # if we got a cache hit, return
     return if (defined $self->{param_map} and defined $self-> 
{parse_stack});

=back

In each case, the coverage reported indicated that the case where the
first part of the condition was true while the second part was false was
never being tested.
     line  err      %     !l  l&&!r   l&&r   expr
     ----- --- ------ ------ ------ ------   ----
     1165  ***      0      0      0      0   defined $$self 
{'param_map'} and defined $$self{'parse_stack'}
     1171  ***      0      0      0      0   defined $$self 
{'param_map'} and defined $$self{'parse_stack'}
     1185  ***     66     57      0      7   defined $$self 
{'param_map'} and defined $$self{'parse_stack'}

We hypothesized that if C<$self-E<gt>{param_map}> is defined,
C<$self-E<gt>{parse_stack}> must always be defined as well.  In that  
case,
we would only need the first part of the condition and could dispense
with the second.

While we don't yet understand C<param_map> and C<parse_stack> well
enough to prove this point by logic, it was empirically verified by
simplifying and testing the code.  These three blocks were rewritten as
follows:

=over 4

=item *

     } elsif ($options->{double_file_cache}) {
       # try the normal cache, return if we have it.
       $self->_fetch_from_cache();
       return if (defined $self->{param_map});

=item *

       # put it in the local cache if we got it.
       $self->_commit_to_cache()
         if (defined $self->{param_map});

=item *

     # if we got a cache hit, return
     return if (defined $self->{param_map});

=back

All tests continued to pass once these changes were made.
As was the case discussed above, since these lines no longer include
an C<and> condition, these lines are no longer reported by  
Devel::Cover as
conditions -- which means the total coverage ratio for conditions
increases.

=head2 Elimination of Unnecessary Tests for Existence of Variables

=head3 Within C<_fetch_from_cache()>

C<_fetch_from_cache()> begins as follows:

     sub _fetch_from_cache {
       my $self = shift;
       my $options = $self->{options};

       # return if there's no file here
       return unless exists($options->{filename});  # line 1214

The coverage report on branches suggested that the 'true' branch of line
1214 -- where C<$options-E<gt>{filename}> did I<not> exist -- was  
never tested.
     line  err      %   true  false   branch
     ----- --- ------ ------ ------   ------
     1214  ***     50      0      6   unless exists $$options 
{'filename'}

C<_fetch_from_cache()>, however, is always called from within C<_init()>
-- which, in turn, is always called from within C<new()> at a point
I<after> we have guaranteed that C<$options-E<gt>{filename}> not only  
exists
but is defined and of non-zero length.  Line 1214 is therefore
superfluous and may be eliminated, thereby increasing the branch
coverage ratio.

=head3 Within C<_cache_key()>

C<_cache_key()> begins:

     sub _cache_key {
         my $self = shift;
         my $options = $self->{options};

         # determine path to file unless already known
         my $filepath = $options->{filepath};
         if (not defined $filepath) {
             $filepath = $self->_find_file($options->{filename});
             confess("HTML::Template->new() : Cannot find file  
'$options->{filename}'.")
               unless defined($filepath);
             $options->{filepath} = $filepath;
         }

         # assemble pieces of the key
         my @key = ($filepath);
         push(@key, @{$options->{path}}) if $options->{path};
     ...

The branch part of the coverage report indicated that there was quite a
bit of uncovered code here.

     line  err      %   true  false   branch
     ----- --- ------ ------ ------   ------
     1291  ***     50      0     15   if (not defined $filepath)
     1293  ***      0      0      0   unless defined $filepath
     1300  ***     50     15      0   if $$options{'path'}

The L<double-grep test|additional_errorchecking_in_new__> discussed
above takes care of this coverage
deficiency.  It guarantees that, by this point in a program,
C<$options-E<gt>{filepath}> is always defined.  So quite a few lines of
code can be eliminated outright.

     sub _cache_key {
         my $self = shift;
         my $options = $self->{options};

         # assemble pieces of the key
         my @key = ($options->{filepath});
         push(@key, @{$options->{path}});
     ...

All tests again pass.  Re-running coverage analysis indicates that this
block of code is now completely covered.

=head3 Within C<_fetch_from_file_cache()>

     sub _fetch_from_file_cache {
       my $self = shift;
       my $options = $self->{options};
       return unless exists($options->{filename});
     ...

Coverage analysis indicated that the 'true' branch here -- the case
where C<$options-E<gt>{filename}> did I<not> exist -- was untested.

     line  err      %   true  false   branch
     ----- --- ------ ------ ------   ------
     1334  ***     50      0      5   unless exists $$options 
{'filename'}

Once again, the addition of the L<double-grep test| 
additional_errorchecking_in_new__> inside C<new()>
guaranteed that this C<filename> must exist by this point.  Hence,
C<_fetch_from_file_cache()> could be rewritten to begin:

     sub _fetch_from_file_cache {
       my $self = shift;
       my $options = $self->{options};
     ...

All tests continued to pass, and because an entire statement was
eliminated, both statement and branch coverage ratios increased.

=head3 Within C<_find_file()>

In C<_find_file()>, there exist these two lines of code:

     if (exists($ENV{HTML_TEMPLATE_ROOT}) and defined($ENV 
{HTML_TEMPLATE_ROOT})) {  # Line 1541
     ...
     if (exists($ENV{HTML_TEMPLATE_ROOT})) {  # Line 1556

In the first line, the test for definedness is logically sufficient.  In
the second line, a test for definedness would be better than the test
for mere existence.  So these lines can each be rewritten as:

     if (defined($ENV{HTML_TEMPLATE_ROOT})) {

All tests pass and the condition coverage ratio improves because the
first line no longer includes a condition.

=head2 Other Simplification of Code

=head3 C<param()>

We have not thoroughly plumbed the depths of C<param()>, so there is
probably quite a bit of work left there for us to do.  However, we did
manage to simplify one block of code beginning at line 2479:

       if (!scalar(@_)) {
         croak("HTML::Template->param() : Single reference arg to  
param() must be a hash-ref!  You gave me a $type.")
           unless $type eq 'HASH' or
             (ref($first) and UNIVERSAL::isa($first, 'HASH'));
         push(@_, %$first);

The C<unless> condition is visibly complex, and we have learned through
experience that complex conditions tend to contain code untested by a
module's test suite.  Sure enough, the coverage report showed for:

=over 4

=item Statements

     line  err   stmt branch   cond    sub    pod   time   code
     2479         125    100                        6445     if (! 
scalar(@_)) {
     2480  ***     48     50      0                  272       croak 
("HTML::Template->param() : Single reference arg to param() must be a  
hash-ref!  You gave me a $type.")
           ***                   33
     2481                                                         
unless $type eq 'HASH' or
     2482                                                           
(ref($first) and UNIVERSAL::isa($first, 'HASH'));

=item Branches

     line  err      %   true  false   branch
     ----- --- ------ ------ ------   ------
     2479         100     48     77   if (not scalar @_) { }
     2480  ***     50      0     48   unless $type eq 'HASH' or ref  
$first and UNIVERSAL::isa($first, 'HASH')

=item Conditions

     and 3 conditions

     line  err      %     !l  l&&!r   l&&r   expr
     ----- --- ------ ------ ------ ------   ----
     2480  ***      0      0      0      0   ref $first and  
UNIVERSAL::isa($first, 'HASH')

=back

We tackled this code at our second HTML::Template joint hacking
session.  More precisely, we tackled it on our third round of beer at
our second joint hacking session.  We simplified the code to the
following:

       if (!scalar(@_)) {
         croak("HTML::Template->param() : Single reference arg to  
param() must be a hash-ref!  You gave me a $type.")
             unless $type eq 'HASH' or UNIVERSAL::isa($first, 'HASH');
         push(@_, %$first);

All the code now became coverable by tests.  All tests passed.

=head1 Revisions of Tests

=head2 F<t/99-old-test-pl.t>

This file, the sole test file in version 2.7, has been left unchanged.

=head2 New F<t/*.t> Tests

=over 4

=item F<t/01-bad-args.t>

Test whether constructor fails with appropriate messages if passed bad
or missing arguments.

=item F<t/02-parse.t>

Test previously untested code inside C<HTML::Template::_parse()>.  Much
remains to be done, as we were just beginning our Phalanx efforts when
we were working in this area.

=item F<t/03-associate.t>

Test previously untested method C<HTML::Template::associateCGI()>.

=item F<t/04-type-source.t>

Test the 'type-source' style of constructor C<HTML::Template::new()>.

     $stemplate = HTML::Template->new(
         type   => 'scalarref',
         source => \$template_string,
     );

And similarly for cases where the source is an array or a filehandle.

=item F<t/05-blind-cache.t>

Test the previously untested C<blind_cache> option to constructor
C<HTML::Template::new()>.

     $template = HTML::Template->new(
         path => ['templates/'],
         filename => 'simple.tmpl',
         blind_cache => 1,
     );

I<Note:>  According to the documentation, when the C<blind_cache> option
is set to 1, ''the module behaves exactly as with normal
caching but does not check to see if the file has changed on each
request.''  Unfortunately, there is no documentation beyond this.
Specifically, (a) there is no test for it in F<t/99-old-test-pl.t>;
(b) there is no example of it in the documentation; and (c) we could
find no references to it in the archives of the HTML::Template mailing
list on Sourceforge.

So we were left to guess at how to code up a test.  We basically  
cloned the
test in F<t/99-old-test-pl.t> beginning at line 370.  We hypothesized  
that
if we
created an object with the C<blind_cache> option, slept for 1 second,  
then
changed the template file and got the same output string notwithstanding
the change in the template file, we could deem the test a pass.  And it
did indeed pass.  But since our understanding of the C<blind_cache>
option is limited, we don't yet know if the test was valid.

=item F<t/06-file-cache-dir.t>

Test edge cases in use of C<file_cache> and C<file_cache_dir> options
to constructor C<HTML::Template::new()>.  Example:  test case where
C<file_cache> option is set to C<undef> but a C<file_cache_dir> value is
provided; examine error message.

=item F<t/07-double-file-cache.t>

Test the previous untested C<double_file_cache> option to
C<HTML::Template::new()>.

     $template = HTML::Template->new(
         path => ['templates/'],
         filename => 'simple.tmpl',
         double_file_cache => 1,
         file_cache_dir => './blib/temp_cache_dir',
     );

=item F<t/08-cache-debug.t>

Automate testing of information printed to STDERR when the  
C<cache_debug>
option to
C<HTML::Template::new()> is turned on.  Tests for this functionality in
F<t/99-old-test-pl.t> were ''non automated,'' I<i.e.>, in order to  
assess
their results, the user would have had to turn the option on and  
visually
inspect STDERR as the test were displayed on the terminal.  Now the
output is captured and analyzed automatically.

=item F<t/09-caching-precluded.t>

Test that C<HTML::Template::new()> now precludes the
possibility of any of the six cache options having a
true value if the template source is a filehandle, string or array.  The
constructor now does additional error-checking and, if a violation is
found, the program dies and an appropriate error message is emitted via
C<croak> and analyzed.

=item F<t/10-param.t>

Test edge cases in use of C<HTML::Template::param()>.  More tests will
probably be added as we understand this function better.

=item F<t/11-non-file-templates.t>

Test whether simple output is correctly created when the template source
is a string, array or a filehandle.

=back

=head2 Supportive Testing Modules in F<t/testlib>

We are using core module F<Test::More> throughout.  In order to  
extend the
test coverage as far as possible, however, we have in certain
circumstances found it expedient to use functionality provided by other,
non-core CPAN modules.  Where appropriate, we have included this
functionality underneath the F<t/testlib> directory.

For example, to test HTML::Template's C<cache_debug> option, a user
currently has to uncomment that option and observe STDERR as the tests
are run.  We supplement by providing tests in F<t/08-cache-debug.t>
which automatically capture debugging messages printed to STDERR and
comparing those messages to predicted values with F<Test::More>  
functions.
We capture those debugging messages with parts of non-core CPAN
distribution F<IO::Capture>, included under F<t/testlib>.

=head2 The 700 Club

Recognizing your feelings about HTML::Template, we are trying to make
specific efforts to allay your fears that our Phalanx work will at some
point cause HTML::Template to 'break'.  One way that revisions to the
module I<could> make it break would be changes to the constructor (or to
functions called from within the constructor) which produced a different
data structure within the HTML::Template object.

We therefore have gone to some lengths to demonstrate that the object
created by C<new()> in 2.701 is the same as that created by C<new()> in
2.7.  We extracted a large portion of the tests found in
F<t/99-old-test-pl.t> which included a new constructor call and placed
them in six files with visibly distinct names: F<t/700-cons.t>,
F<t/701-cons.t>, ... F<t/706-cons.t>:  the 700 Club.  In these test  
files
we create an HTML::Template object with specified parameters with
version 2.701 and then make exactly the same call with version 2.7.  We
then compare the two objects with C<Test::More::is_deeply()>.  We do
this by including F<HTML/Template.pm> under F<t/testlib>.

The results were unambiguous:  the object created
by C<HTML::Template::new()> under 2.701 is the same as that created by
the constructor under 2.7.

=head1 Revisions of Documentation

Changes proposed for the documentation in F<Template.pm> are most easily
seen by examining the file F<diffpod> attached.
When you look at F<diffpod> you should see that most changes proposed
are cosmetic.  For a more consistent look in the HTML representation of
the docs, many variable names which previously appeared in the body  
font or
inside quotation marks have now been placed in monospace font.
Examples:

     121c121
     < If you called param() with a value like sam"my you'll get in  
trouble
     ---
     > If you called C<param()> with a value like sam"my you'll get  
in trouble

     406c406
     < to access the template text.  You can use "filename =>  
'file.tmpl'" to
     ---
     > to access the template text.  You can use C<filename =>  
'file.tmpl'> to

A few spelling errors were found and corrected.  Example:

     2838c2841
     < Just like C<param()>, C<query()> with no arguements returns  
all the
     ---
     > Just like C<param()>, C<query()> with no arguments returns all  
the

In other cases the only change is elimination of trailing whitespace at
the end of lines.  Other than these changes no changes have been
suggested so far for the documentation.  However, since we still have
more work to do on the code and tests, there may arise situations
where we will suggest changes in the documentation as well.

=head1 Recommendations

We recommend that the tarball accompanying this message -- less the six
tests in the ''700 Club'' -- be uploaded to CPAN as the next version of
HTML::Template.  In other words, we recommend that what we have called
version 2.701 to renamed (in F<Template.pm>) version 2.71.

There I<are> parts of HTML::Template for which we either (a) cannot yet
write tests (because we haven't tried it on OSes which support features
such as C<shared_cache>, C<double_cache> or C<memory_debug>) or have not
yet written tests (parts of C<param()> and C<_parse>).  Nonetheless, the
substantial increases in statement, branch, condition and subroutine
coverage we have achieved make HTML::Template stronger by providing
greater assurance that future revisions will not cause it to ''break.''
In particular, we recommend that any patches you are currently
considering for HTML::Template (such as those suggested on the
HTML::Template mailing list) be applied against 2.71 rather than 2.70.

Since we didn't change the functionality or interface of HTML::Template
in any respect, no current users of HTML::Template would be required to
upgrade to 2.71, nor would they get any surprises if they did so.  But
people installing HTML::Template for the first time would install a more
thoroughly tested version of it.

Thank you very much.

=cut

Re: [htmltmpl] Suggestion on how to eliminate Cross-site-scripting (XSS) bugs for good.

[Tom H. <tom...@pu...>]

Sam Tregar wrote:
> On Tue, 17 Oct 2006, Tom Heady wrote:
> 
>> Actually, I found that turning off escaping (ESCAPE="0") does not work
>> if you specify a default escape.
>>
>> See http://rt.cpan.org/Public/Bug/Display.html?id=18274 for more details
>> and a fix.
> 
> I'll make sure this gets into the next release.  I'm planning to put
> one out soon.
> 

Thanks

Tom

Re: [htmltmpl] The sf.net Subversion Repository and the Phalanx One

[Sam T. <sa...@tr...>]

On Thu, 16 Nov 2006, Shlomi Fish wrote:

>> Took me exactly 10 minutes to forward port the patch from the Phalanx
>> repository. So it wasn't such a painful process. :-D
>
> Hmmm... I spoke too soon. The first incarnation of the patch did not pass the
> tests. I now have a better patch at the same URL:
>
> http://www.shlomifish.org/Files/files/code/perl/HTML-Template/h-t-phalanx.diff.bz2
>
> Please apply it instead.

Thanks Shlomi, this is helpful.  I'll sort through it and apply what I
can live with.  Some of these tests look counter-productive to me
(like the ones that ensure the internal structure of the object stays
the same as v2.7), but most look like improvements.

-sam