-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
html/template: confusing disallowed escapers detection #22086
Comments
cc @bradfitz |
CC @stjj89 |
The relevant CL is actually golang.org/cl/40936, which relaxes the policy in golang.org/cl/37880, and allows predefined escapers ("html" and "urlquery") to occur in template actions only if either occurs as the last action in a pipeline. This is actually explained in the documentation for the error type you received (see ErrPredefinedEscaper). We allow a predefined escaper to occur at the end of the pipeline since it will not interfere with the behavior of the contextual auto-escaper when placed in that position. This was meant to break less users in Go1.9.
The error message you got was technically correct---the predefined escaping action not at the end of the pipeline is the problem, and should be removed. The error message is deliberately opaque to discourage users from using predefined escapers in actions at all, since that defeats the purpose of a contextual auto-escaping template system. While I do agree that the error message is confusing in this particular case, it is an unfortunate consequence of us being lenient in some cases to soften the impact of this breaking change. Another hypothetical develop who writes both I think that a reasonable way to address this confusion is to point the user to the error documentation, which explains the change and the accommodation that we made for migration. I'm thinking something like:
|
@stjj89 something like that new error message SGTM. especially since said documentation (which I had indeed not consulted/seen) specifically states to avoid any of these two escapers anyway. |
Change https://golang.org/cl/69032 mentions this issue: |
Error messages cannot convey every nuance of html/template or really of any package. Users are expected to read the docs, or at least to look at the docs if they have questions. In this case if you go to https://golang.org/pkg/html/template/ or run "godoc html/template" and Ctrl-F for "predefined escaper", you find what you're looking for. Please let's not start putting URLs into error strings. |
@rsc and yet I somehow missed that particularly piece of doc until it was pointed out to me. I just hope that won't happen to others. |
What version of Go are you using (
go version
)?go version go1.9 linux/amd64
Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?Linux, amd64.
What did you do?
For context, we got that bug report:
perkeep/perkeep#960
which correctly identified that we were now wrongly using some escapers , wrt to https://go-review.googlesource.com/c/go/+/37880
As expected, the problem turned out to be something like:
in a template.
And changing the template to:
fixed the template error message.
However, out of curiosity, I played a bit more with the pipeline, and here's what I find to be confusing:
if I do:
I get "html/template:subject:28:50: predefined escaper "html" disallowed in template", which seems to suggest that "html" is the problematic one.
but if i then do:
I then get "html/template:subject:28:50: predefined escaper "urlquery" disallowed in template".
And if then remove either of them, so either
or
I get no error.
So, while the first error message might suggest that only "html" is problematic (and might lead one to just remove the "html" escaper), the rest of experiment suggests that it's the succession of the both of them that is a problem, or that even maybe they should all be removed.
It seems to me that the error message is not quite clear enough, as it might not lead to the correct fix for this kind of situation, especially for users not aware of https://go-review.googlesource.com/c/go/+/37880 (so most of them?)
WDYT?
The text was updated successfully, but these errors were encountered: