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.

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: should escape JSON without using \x #33671

Closed
earthboundkid opened this issue Aug 15, 2019 · 15 comments
Closed

html/template: should escape JSON without using \x #33671

earthboundkid opened this issue Aug 15, 2019 · 15 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@earthboundkid
Copy link
Contributor

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

go version go1.12.7 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

Ran this snippet

package main

import (
	"html/template"
	"log"
	"os"
)

func main() {
	const tpl = `<!DOCTYPE html>
<html>
	<head>
		<meta charset="UTF-8">
		<title>{{ .Title }}</title>
	</head>
	<body>
		<script type="application/ld+json">
		{
			"@context": "http://schema.org",
			"@type": "WebPage",
			"name":"{{ .Title }}",
			"description": "{{ .Description }}"
		}
		</script>
	</body>
</html>`

	check := func(err error) {
		if err != nil {
			log.Fatal(err)
		}
	}
	t, err := template.New("webpage").Parse(tpl)
	check(err)

	data := struct {
		Title, Description string
	}{
		Title:       "<My \"Cool\" Page>",
		Description: "A \"Cool\" Page by 'Me'",
	}

	err = t.Execute(os.Stdout, data)
	check(err)
}

What did you expect to see?

An HTML document containing valid JSON.

What did you see instead?

Invalid JSON, as reported by https://search.google.com/structured-data/testing-tool/

Go escapes what is between the <script> tags as though it were JavaScript because of #26053. That's mostly correct, except that there are subtle difference between valid JavaScript and valid JSON. Specifically, https://search.google.com/structured-data/testing-tool/ reports that \x3c style escaping is not correct for LD JSON. It needs to be \u003c instead.
@earthboundkid
Copy link
Contributor Author

FWIW, http://json.org agrees that \u escapes are valid JSON and \x escapes are not.

ISTM, the simple solution is to tell the JavaScript escaper to always use \u. JS and JSON have other differences around whitespace handing, but it's easy enough to make them compatible.

@earthboundkid earthboundkid changed the title html/template should recognize <script type="application/ld+json"> html/template should escape JSON without using \x Aug 15, 2019
@agnivade
Copy link
Contributor

@mikesamuel @empijei

@bcmills bcmills changed the title html/template should escape JSON without using \x html/template: should escape JSON without using \x Aug 19, 2019
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 19, 2019
@bcmills bcmills added this to the Go1.14 milestone Aug 19, 2019
@earthboundkid earthboundkid mentioned this issue Aug 29, 2019
Closed
@gcushen gcushen mentioned this issue Sep 20, 2019
Closed
8 tasks
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@empijei
Copy link
Contributor

empijei commented Oct 16, 2019

I investigated this a bit and I confirm it is an issue.

( Some context for future reference: https://json-ld.org/ )

@carlmjohnson would you like to work on a CL for this?
@bcmills this doesn't need further investigation

@Harumaro
Copy link

We are having this exact same problem as of today, is there a deadline or a workaround for this? It's really impacting our SEO.

@earthboundkid
Copy link
Contributor Author

I started working on a fix, but honestly, I'm in over my head because I don't know what is valid or invalid in JS and I don't understand what all the test cases are testing for.

In terms of a workaround, if you're using Hugo, you can do {{ $x | jsonify | safeJS }}. Other Go templating systems should have similar ways of converting to JSON and marking as safe.

@Harumaro
Copy link

Thanks, if anybody needs a workaround for gin gonic, it's in Chinese, but Google translate is helpful.
https://codus.me/blog/golang-template-json-ld-error-encoding

@empijei
Copy link
Contributor

empijei commented Mar 23, 2020

@Harumaro there is no deadline that I am aware of but if you are willing to work on a fix I can do the review and approval to expedite this.

@earthboundkid earthboundkid mentioned this issue Mar 23, 2020
Closed
@earthboundkid
Copy link
Contributor Author

@empijei Sent a broken CL with just the \x → \u change. It needs a lot of test fixes.

@gopherbot
Copy link

Change https://golang.org/cl/224898 mentions this issue: html/template: escape JS/JSON with \u

@empijei
Copy link
Contributor

empijei commented Mar 23, 2020 via email

@liuggio
Copy link

liuggio commented Mar 23, 2020 •
edited

I love the PR you made we really need it, without the fixed code we are issuing a lot of errors on js console (SEO problem and logging problems our sentry is out of quota for this)

In terms of a workaround, if you're using Hugo, you can do {{ $x | jsonify | safeJS }}

@carlmjohnson the workaround is not working
https://play.golang.org/p/Elcp4dF7NhS

waiting for this, now we are patching go :)

(edited)
the real missing feature is the "raw" content to avoid unnecessary escaped strings.
or custom fn escaper

@earthboundkid
Copy link
Contributor Author

earthboundkid commented Mar 23, 2020 •
edited

This fixes your snippet without waiting on a language change (months in the best case): https://play.golang.org/p/pNtKGJgTQs0

@empijei
Copy link
Contributor

empijei commented Mar 27, 2020

@empijei Sent a broken CL with just the \x → \u change. It needs a lot of test fixes.

I can take a look and see if I can fix it.

@gopherbot
Copy link

Change https://golang.org/cl/226097 mentions this issue: html/template: switch to unicode escape instea of hex

@earthboundkid
Copy link
Contributor Author

🎉

@moorereason moorereason mentioned this issue May 13, 2020
Closed
@golang golang locked and limited conversation to collaborators Apr 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
8 participants
@rsc @earthboundkid @liuggio @empijei @Harumaro @agnivade @bcmills @gopherbot