Skip to content
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.

Sign up for GitHub

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

Jump to bottom

html/template: script type attribute inside condition causes an error #59112

Open
willfaught opened this issue Mar 18, 2023 · 8 comments
Open

html/template: script type attribute inside condition causes an error #59112

willfaught opened this issue Mar 18, 2023 · 8 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@willfaught
Copy link
Contributor

willfaught commented Mar 18, 2023 •
edited

What did you do?

Template:

<script {{ if true }} type="" {{ end }}></script>

What did you expect to see?

<script type=""></script>

What did you see instead?

{{if}} branches end in different contexts:
{stateTag delimNone urlPartNone jsCtxRegexp attrNone elementNone <nil>},
{stateTag delimNone urlPartNone jsCtxRegexp attrNone elementScript <nil>}

If you remove the condition around the type attribute, or rename type to something else, then there's no error.

Remarks

The <script> type attribute value should be able to be dynamic to substitute in the various valid values for type:

  • text/javascript
  • module
  • importmap
  • etc.

Note that the original template was:

<script {{ with $type }} type="{{ . }}" {{ end }}></script>

See here for the original context.

This is a valid construct for every other HTML attribute I've ever tried it on.

This issue was already reported in #57136, however it's clear that the maintainer who closed the issue didn't understand it, as the author explained after it was closed. There was no reply.

What version of Go are you using (go version)?

$ go version
go version go1.20.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/Will/Library/Caches/go-build"
GOENV="/Users/Will/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/Will/Library/Application Support/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/Will/Library/Application Support/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.20.2/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.20.2/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.20.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/bx/qk0phsxd265fqj512dnnpg080000gn/T/go-build282336036=/tmp/go-build -gno-record-gcc-switches -fno-common"
@seankhliao
Copy link
Member

cc @golang/security

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 19, 2023
@seankhliao seankhliao added this to the Unplanned milestone Mar 19, 2023
@ianlancetaylor
Copy link
Contributor

This is related to #6701 and #12149. The type expression is meaningful within a <script> tag, to say what kind of script it is. The error message is confusing, but I'm not sure there is anything to fix here other than perhaps generating a better error message.

CC @robpike

@willfaught
Copy link
Contributor Author

@ianlancetaylor I don't understand how this is related to weird semicolons in markup input or invalid quotations in markup output. What is the correct error message in this case?

@seankhliao
Copy link
Member

While type="" (or unset type) might equal type="text/javascript",
javascript isn't the only allowed type of content in <script> tags.
("Any other value" in the MDN link you provided).

Different content types require different types of escaping, and with a dynamic value for type=, it becomes impossible for html/template to know which sort of escaping (if any) should be applied to the following content.

@willfaught
Copy link
Contributor Author

Different content types require different types of escaping,

When type=module, there's no inner text, so it's not an issue in that case, but I can see how encoding is an issue for language MIME types.

it becomes impossible for html/template to know which sort of escaping (if any) should be applied to the following content

Then shouldn't constructing a dynamic type attribute value with the printf function be disallowed as well? That is currently allowed, at least in Hugo in combination with Hugo's safeHTMLAttr function.

@ianlancetaylor
Copy link
Contributor

I don't understand how this is related to weird semicolons in markup input or invalid quotations in markup output. What is the correct error message in this case?

I don't know for sure, but perhaps a better error message would be something like "attempt to change script type in conditional context". I don't see how html/template can be expected to correctly handle such a case.

Then shouldn't constructing a dynamic type attribute value with the printf function be disallowed as well? That is currently allowed, at least in Hugo in combination with Hugo's safeHTMLAttr function.

Perhaps I'm missing something, but that seems like an issue with Hugo, not html/template as such.

@willfaught
Copy link
Contributor Author

I don't see how html/template can be expected to correctly handle such a case.

Just spit balling: Would it be possible to evaluate the tag first, then the inner text once the escaping is known? Then type wouldn't have to be special-cased.

Regardless, the behavior is surprising, and it should be explained in the package doc, along with any other special cases, in my opinion.

that seems like an issue with Hugo, not html/template as such.

Hugo uses html/template under the hood. I assume safeHTMLAttr is a normal custom function that html/template allows, but perhaps that's wrong. @jmooring or @bep might know more.

@gopherbot
Copy link

Change https://go.dev/cl/496145 mentions this issue: html/template: example for disallowed script type change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@willfaught @ianlancetaylor @gopherbot @seankhliao