html/template: srcset incorrectly escaped

[okdave]

tl;dr: https://play.golang.org/p/8A4TQ1-Kvt

The html/template defaults to the contentTypeURL content type for any attribute that contains the substring "src".

However the srcset attribute is a set of URLs which are separated by whitespace and optional extra size/density indicators (see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#attr-srcset).

For example "image1.jpg w200, image2.jpg w400".

When this is escaped as the contentTypeURL the spaces are encoded, the whole thing looks like a single URL, and the resource fails to load.

tmpl := template.Must(template.New("foo").Parse(`<img srcset="{{.}}">`))
tmpl.Execute(os.Stdout, "1.jpg w200, 2.jpg w200")

Got <img srcset="1.jpg%20w200,%202.jpg%20w200">
Want <img srcset="1.jpg w200, 2.jpg w200">

[okdave]

Also, I can't figure out how to override the escaping using the types in the package. The two that I thought would work (HTMLAttr and URL) both have no effect: https://play.golang.org/p/ZSSV7AneLL

[bradfitz]

/cc @robpike @adg

[cespare]

@okdave I don't know the answer to your question, but you can work around the issue by using HTMLAttr with the whole attribute: https://play.golang.org/p/uZUPL7OgA2

[okdave]

Thanks @cespare that's exactly what I had to end up doing, though it felt wrong.

[rsc]

This looks like it will need to introduce new states into the HTML analyzer.

/cc @mikesamuel for advice.

[zombiezen]

I would be interested in picking this up. Any advice on where to start?

[rsc]

The html/template doc comment explains a lot about what is going on in this package.

[mikesamuel]

@zombiezen

https://golang.org/src/html/template/content.go probably needs a new content type for space delimited URLs.

The table of attribute types in https://golang.org/src/html/template/attr.go probably needs a mapping for srcset.

url.go would then probably need a URL escaper variant that allows [\t\n\r ] through unescaped.

I forgot quite where content types are mapped to escapers, but some grepping around should find it easily enough.

[zombiezen]

Cool. I'll take a go at this.

[gopherbot]

CL https://golang.org/cl/38324 mentions this issue.

[odeke-em]

I've ran the trybots on that CL so that to reignite the conversation on @zombiezen's CL as I see it is marked for Go1.9, what do you think @bradfitz?

[zombiezen]

I'm happy to carry this forward, but I need review from someone who understands the security implications.

[odeke-em]

Punting this to Go1.10 as per @rsc's comment on the CL.

[namusyaka]

@rsc @odeke-em @zombiezen I've tried to upload patch set for adding @rsc's comments in tests.
Could you take a look at the CL? Sorry if there is a problem with the way.

[mikesamuel]

Per the discussion about URL filtering in srcset at 38324/4, here's a patch: url.go.diff.txt.

I tried to upload a patch set, but need to sort out some Gerrit credential issue.