Executing a template involving variadic functions featuring
a []interface{} slice (such as printf) could result in a
panic in reflect.Value.Call, due to incorrect type checking.
The following expressions failed (with a panic):
{{true|printf}}
{{1|printf}}
{{1.1|printf}}
{{'x'|printf}}
{{1+2i|printf}}
Implemented proper type checks for the fixed parameters of the
variadic functions.
Fixes#10946
Change-Id: Ia75333f651f73b3d2e024cb0c47cc30d90cb6852
Reviewed-on: https://go-review.googlesource.com/10403
Reviewed-by: Rob Pike <r@golang.org>
The current escape code panics when an action involves chain nodes.
Such nodes can be seen in the following situation:
{{ . | AAA.B }} - AAA being a registered function
The above expression is actually valid, because AAA could return a
map containing a B key. The tests in text/template explicitly
demonstrate this case.
Fix allIdents to cover also chain nodes.
While I was investigating this issue, I realized that the tests
introduced in similar CL 9621 were incorrect. Parse errors were
caught as expected, but for the wrong reason. Fixed them as well.
No changes in text/template code itself.
Fixes#10801
Change-Id: Ic9fe43b63669298ca52c3f499e2725dd2bb818a8
Reviewed-on: https://go-review.googlesource.com/10340
Reviewed-by: Rob Pike <r@golang.org>
t.init() should be called at the time of template creation
i.e, template.New() and t.New() instead of later in the process.
- Removed calls of t.init() from t.Parse(), t.Execute(), t.Funcs()
- Also got rid of t.common != nil checks as it should never be nil
Fixes#10879
Change-Id: I1b7ac812f02c841ae80037babce7e2b0a2df13e8
Reviewed-on: https://go-review.googlesource.com/10240
Reviewed-by: Rob Pike <r@golang.org>
The Template objects are supposed to be goroutine-safe once they
have been parsed. This includes the text and html ones.
For html/template, the escape mechanism is triggered at execution
time. It may alter the internal structures of the template, so
a mutex protects them against concurrent accesses.
The text/template package is free of any synchronization primitive.
A race condition may occur when nested templates are escaped:
the escape algorithm alters the function maps of the associated
text templates, while a concurrent template execution may access
the function maps in read mode.
The less invasive fix I have found is to introduce a RWMutex in
text/template to protect the function maps. This is unfortunate
but it should be effective.
Fixes#9945
Change-Id: I1edb73c0ed0f1fcddd2f1516230b548b92ab1269
Reviewed-on: https://go-review.googlesource.com/10101
Reviewed-by: Rob Pike <r@golang.org>
Missed a case; just need to call validateType.
Fixes#10800.
Change-Id: I81997ca7a9feb1be31c8b47e631b32712d7ffb86
Reviewed-on: https://go-review.googlesource.com/10031
Reviewed-by: Andrew Gerrand <adg@golang.org>
This was added during testing but is unnecessary.
Thanks to gravis on GitHub for catching it.
See #10574.
Change-Id: I4a8f76d237e67f5a0ea189a0f3cadddbf426778a
Reviewed-on: https://go-review.googlesource.com/9841
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Catch some malformed pipelines at parsing time.
The current code accepts pipelines such as:
{{12|.}}
{{"hello"|print|false}}
{{.|"blah blah"}}
Such pipelines generate panic in html/template at execution time.
Add an extra check to verify all the commands of the pipeline are executable
(except for the first one).
Fixes#10610
Change-Id: Id72236ba8f76a59fa284fe3d4c2cb073e50b51f1
Reviewed-on: https://go-review.googlesource.com/9626
Reviewed-by: Rob Pike <r@golang.org>
When a parse error occurred, the lexing goroutine would lay idle.
It's not likely a problem but if the program is for some reason
accepting badly formed data repeatedly, it's wasteful.
The solution is easy: Just drain the input on error. We know this
will succeed because the input is always a string and is therefore
guaranteed finite.
With debugging prints in the package tests I've shown this is effective,
shutting down 79 goroutines that would otherwise linger, out of 123 total.
Fixes#10574.
Change-Id: I8aa536e327b219189a7e7f604a116fa562ae1c39
Reviewed-on: https://go-review.googlesource.com/9658
Reviewed-by: Russ Cox <rsc@golang.org>
At the end of lexInsideAction(), we return lexInsideAction: this is the default
behaviour when we are still parsing an action. But some switch branches return
lexInsideAction too.
So let's ensure code consistency by always reaching the end of the
lexInsideAction function when needed.
Change-Id: I7e9d8d6e51f29ecd6db6bdd63b36017845d95368
Reviewed-on: https://go-review.googlesource.com/9441
Reviewed-by: Rob Pike <r@golang.org>
Ideal constants in the template package are a little different from Go.
This is a case that slipped through the cracks: A huge integer number
was accepted as a floating-point number, but this loses precision
and is confusing. Also, the code in the template package (as opposed
to the parse package) wasn't expecting it.
Root this out at the source: If an integer doesn't fit an int64 or uint64,
complain right away.
Change-Id: I375621e6f5333c4d53f053a3c84a9af051711b7a
Reviewed-on: https://go-review.googlesource.com/9651
Reviewed-by: Russ Cox <rsc@golang.org>
The current parser ignores obvious errors such as:
{{0.1.E}}
{{true.any}}
{{"hello".wrong}}
{{nil.E}}
The common problem is that a chain is built from
a literal value. It then panics at execution time.
Furthermore, a double dot triggers the same behavior:
{{..E}}
Addresses a TODO left in Tree.operand to catch these
errors at parsing time.
Note that identifiers can include a '.', and pipelines
could return an object which a field can be derived
from (like a variable), so they are excluded from the check.
Fixes#10615
Change-Id: I903706d1c17861b5a8354632c291e73c9c0bc4e1
Reviewed-on: https://go-review.googlesource.com/9621
Reviewed-by: Rob Pike <r@golang.org>
An unmatched {{else}} should trigger a parsing error.
The top level parser is able to issue an error in case
of unmatched {{end}}. It does it a posteriori (i.e. after having
parsed the action).
Extend this behavior to also check for unmatched {{else}}
Fixes#10611
Change-Id: I1d4f433cc64e11bea5f4d61419ccc707ac01bb1d
Reviewed-on: https://go-review.googlesource.com/9620
Reviewed-by: Rob Pike <r@golang.org>
This was disallowed for error-checking reasons but people ask for
it, it's easy, and it's clear what it all means.
Fixes#7323.
Change-Id: I26542f5ac6519e45b335ad789713a4d9e356279b
Reviewed-on: https://go-review.googlesource.com/9537
Reviewed-by: Russ Cox <rsc@golang.org>
Add one option, which is the motivating example, a way to control
what happens when a map is indexed with a key that is not in the map.
Rather than do something specific for that case, we provide a simple
general option mechanism to avoid adding API if something else
comes up. This general approach also makes it easy for html/template
to track (and adapt, should that become important).
New method: Option(option string...). The option strings are key=value
pairs or just simple strings (no =).
New option:
missingkey: Control the behavior during execution if a map is
indexed with a key that is not present in the map.
"missingkey=default" or "missingkey=invalid"
The default behavior: Do nothing and continue execution.
If printed, the result of the index operation is the string
"<no value>".
"missingkey=zero"
The operation returns the zero value for the map type's element.
"missingkey=error"
Execution stops immediately with an error.
Fixes#6288.
Change-Id: Id811e2b99dc05aff324d517faac113ef3c25293a
Reviewed-on: https://go-review.googlesource.com/8462
Reviewed-by: Robert Griesemer <gri@golang.org>
Currently, scanner uses -1 to represent 2 different states:
1. I haven't yet scanned anything, call it "Beginning of File"
2. I've reached the end of the input, ie EOF
The result of this behavior is that calling Peek() when next()
has detected the end of the input and set s.ch to scanner.EOF,
is that Peek() things "oh, s.ch is < 0, which to me means that
I haven't scanned any next yet, let me try and clear the BOM
marker."
When this behavior is run on a typical IO, next() will issue
a Read and get (0, io.EOF) back for the second time without
blocking and Peek() will return scanner.EOF.
The bug comes into play when, inside a terminal, hitting Control-D.
This causes the terminal to return a EOF condition to the reader
but it does not actually close the fd.
So, combining these 2 situations, we arrive at the bug:
What is expected: hitting Control-D in a terminal will make Peek()
return scanner.EOF instantly.
What actually happens:
0. Code waiting in Next()
1. User hits Control-D
2. fd returns EOF condition
3. EOF bubbles it's way out to line 249 in scanner.go
4. next() returns scanner.EOF
5. Next() saves the scanner.EOF to s.ch and returns the previous value
6. Peek() runs, sees s.ch < 0, mistakenly thinks it hasn't run yet and
tries to read the BOM marker.
7. next() sees the buffer is empty and tries to fill it again, blocking
on line 249.
The fix is simple: use a different code to indicate that no data
has been scanned.
Change-Id: Iee8f4da5881682c4d4c36b93b9bf397ac5798179
Reviewed-on: https://go-review.googlesource.com/7913
Reviewed-by: Robert Griesemer <gri@golang.org>
text/template turned this into an error but html/template crashed.
Refactor text/template.Execute to export a new function,
text/template.DefinedTemplates, so html/template can get the same
helpful error message in this case, and invoke it when there is no
definition for a template being escaped.
Fixes#10204.
Change-Id: I1d04e9e7ebca829bc08509caeb65e75da969711f
Reviewed-on: https://go-review.googlesource.com/7855
Reviewed-by: Russ Cox <rsc@golang.org>
An explicit nil in an expression like nil.Foo caused a panic
because the evaluator attempted to reflect on the nil.
A typeless nil like this cannot be used to do anything, so
just error out.
Fixes#9426
Change-Id: Icd2c9c7533dda742748bf161eced163991a12f54
Reviewed-on: https://go-review.googlesource.com/7643
Reviewed-by: David Symonds <dsymonds@golang.org>
Simple bug in argument processing: The final arg may
be the pipeline value, in which case it gets bound to the
fixed argument section. The code got that wrong. Easy
to fix.
Fixes#8950.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/161750043
Was just a missing case (literally) in the type checker.
Fixes#8473.
LGTM=adg
R=golang-codereviews, adg
CC=golang-codereviews
https://golang.org/cl/142460043
Previously, signed and unsigned integers could not be compared, but
this has problems with things like comparing 'x' with a byte in a string.
Since signed and unsigned integers have a well-defined ordering,
even though their types are different, and since we already allow
comparison regardless of the size of the integers, why not allow it
regardless of the sign?
Integers only, a fine place to draw the line.
Fixes#7489.
LGTM=adg
R=golang-codereviews, adg
CC=golang-codereviews
https://golang.org/cl/149780043
