html/template: stateURL context percent encodes "(", ")" yet they are valid path characters

[gmccue]

The html/template context stateURL appears to be percent encoding valid characters. I believe this is because shouldEscape() in net/url encodes these characters by default: https://github.com/ecosia/go/blob/master/src/net/url/url.go#L101

RFC3986 allows for the use of ( and ) characters in the query component: https://tools.ietf.org/html/rfc3986#section-3.4

1. What version of Go are you using (go version)?
   1.6.2
2. What operating system and processor architecture are you using (go env)?
   linux/amd64
3. What did you do?
   Create a template with a variable to be used for the href attribute. Pass a URL including ( or ) to the template and compile. ( and ) are percent encoded during compilation.
   An example on play.golang.org: https://play.golang.org/p/QFtR42GyfI
4. What did you expect to see?
   <a href="http://my.domain/path-to-lang(en)">Click this link.</a>
5. What did you see instead?
   <a href="http://my.domain/path-to-lang%28en%29">Click this link.</a>

[gmccue]

Related to: #12036, and this PR

[ianlancetaylor]

Does this extra encoding cause any harm?

[gmccue]

In my particular use-case is does, as the URL that is output is to a third-party re-direction URL that does not perform proper URL un-encoding.

I will submit a PR shortly.

[odeke-em]

I don't see any connection to net/http so am going to change the title of this bug to relate it to net/url instead. Please correct me if am wrong.

[bradfitz]

Or html/template?

[odeke-em]

Good plan, thanks. Updated.

[rsc]

The problem here is not net/url but the fact that the html/template escaper uses the same code for escaping <a href="https://site.com/$PATH"> and CSS like url(https://site.com/$PATH). In the latter case, it is important to escape the parens.

Any RFC-compliant server should treat /path(en) and /path%28en%29 the same. This code is very subtle, and I am not convinced that the benefit to broken servers is worth the risk of introducing a security problem in this code.

I think we should leave this alone.

/cc @mikesamuel