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

Escaping JS for html <script> tags #79

Closed
lukasschlueter opened this issue Nov 28, 2018 · 3 comments
Closed

Escaping JS for html <script> tags #79

lukasschlueter opened this issue Nov 28, 2018 · 3 comments
Assignees
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@lukasschlueter
Copy link
Contributor

Partials in html <script> tags don't get escaped, possibly breaking JS code.

<!-- sample.html -->
<h1>Some HTML with JS</h1>

<script>
    console.log("<%= partial("someText.txt") %>")
</script>
// _someText.txt
Text with characters "breaking" the JS 'code'

_someText.txt would not get JS escaped as the content-type is text/html.

I don't know a simple way to fix this and I'm not sure if we should escape it.
From my understanding, this would be a lot of work including teaching plush how to parse html.

Originally posted by @lukasschlueter in #77 (comment)

@lukasschlueter
Copy link
Contributor Author

As @sio4 stated, it's possible to use this in a safe way:

<!-- fixed.html -->
<h1>Some HTML with JS</h1>

<script>
    <% let message = partial("someText.txt") %>
    console.log("<%= jsEscape(inspect(message)) %>")
</script>

Printing Text with characters &#34;breaking&#34; the JS &#39;code&#39; to console

@sio4
Copy link
Member

sio4 commented Nov 29, 2018

I think this issue is not so important and improving this is much expensive than we can get.
we can just document this possibility (or caution) which is coming from the nature of HTML.

@sio4
Copy link
Member

sio4 commented Sep 5, 2022

I think this issue is not so important and improving this is much expensive than we can get. we can just document this possibility (or caution) which is coming from the nature of HTML.

Long and old story :-)
Oh, my explanation in English was not as good as always :-) Sorry for my poor English. Maybe what I wanted to tell is not about "the importancy" but about the trade-off between possibility+efficiency and the effect of the result.

As the example shows, the issue is not directly caused by the partial helper but is caused by the usage of the parsed content. When the partial function does its job, it focuses on the given information (via Context) and that is the only information. the function cannot see how it will be used after it returns the value, so it cannot conditionally handle it when it escapes or when just as is.

The usage is the user's own thing, so users (developers) need to be careful about it if they use the partial helper for end-user generated content. We may be able to consider a force escape option via context but I feel it is too much.

I am going to close this long (including the history of the original PR #77) and old issue today, but please feel free to reopen it if you feel this issue is critical. Better examples and descriptions will help to triage the issue more. PR is also welcome.

Additionally,
The behavior of the partial function regarding this issue could be found in:

func Test_PartialHelper_JavaScript(t *testing.T) {
r := require.New(t)
name := "index.js"
data := map[string]interface{}{}
help := HelperContext{Context: NewContext()}
help.Set("contentType", "application/javascript")
help.Set("partialFeeder", func(string) (string, error) {
return `alert('\'Hello\'');`, nil
})
html, err := partialHelper(name, data, help)
r.NoError(err)
r.Equal(`alert('\'Hello\'');`, string(html))
}
func Test_PartialHelper_JavaScript_Without_Extension(t *testing.T) {
r := require.New(t)
name := "index"
data := map[string]interface{}{}
help := HelperContext{Context: NewContext()}
help.Set("contentType", "application/javascript")
help.Set("partialFeeder", func(string) (string, error) {
return `alert('\'Hello\'');`, nil
})
html, err := partialHelper(name, data, help)
r.NoError(err)
r.Equal(`alert('\'Hello\'');`, string(html))
}
func Test_PartialHelper_Javascript_With_HTML(t *testing.T) {
r := require.New(t)
name := "index.html"
data := map[string]interface{}{}
help := HelperContext{Context: NewContext()}
help.Set("contentType", "application/javascript")
help.Set("partialFeeder", func(string) (string, error) {
return `alert('\'Hello\'');`, nil
})
html, err := partialHelper(name, data, help)
r.NoError(err)
r.Equal(`alert(\'\\\'Hello\\\'\');`, string(html))
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants