feat: Check script tags for inline JS #48
Conversation
d3xter666
force-pushed
the
check-script-tags
branch
from
eef1c39 to
7414d89
Compare
| // https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script/type#attribute_is_not_set_default_an_empty_string_or_a_javascript_mime_type | ||
| return attr.name.value !== "type" || | ||
| (attr.name.value === "type" && | ||
| (attr.value.value === "" || attr.value.value === "text/javascript")); |
There was a problem hiding this comment.
In the scope of SAPUI5, I find ~180 occurrences of the legacy MIME type application/javascript. Maybe we should include that one as well? The other legacy types, I didn't find.
According to https://mimesniff.spec.whatwg.org/#javascript-mime-type-essence-match, the MIME types are case insensitive. I don't expect the SAX parser to normalize attribute values, so the check maybe should be case insensitive.
Last but not least: we most likely won't process scripts of type module any time soon, but the check for inline scripts could complain about them already (if they're inline)
There was a problem hiding this comment.
Thanks, @codeworrior !
I have added the case insensitive checks and also the application/javascript type.
Regarding the module type, currently it's being ignored as we check only for missing type property or against a list of hardcoded, case insensitive (already, thanks!) types.
I have added type="module" to the test cases
There was a problem hiding this comment.
@codeworrior's comment wasn't fully addressed, right? I also agree that the check should complain about type="module", as it is also relevant for CSP unsafe-inline.
In addition, a <script> tag with a "src" attribute and a comment (or even code) as content is falsely reported as finding. Browsers (tested with Chrome only) ignore the script tag content in case a "src" attribute is provided (even without a value).
<script src="foo.js"> // should not be reported as it is not a CSP violation
</script><script src> // should also not be reported as it is not a CSP violation
console.log("this code won't run");
</script>There was a problem hiding this comment.
I missunderstood it. However, I have addressed the changes in the following PR: #70
d3xter666
force-pushed
the
check-script-tags
branch
from
a7c520b to
93495c6
Compare
…r checks (#70) This change addresses the following comments: #48 (comment)
JIRA: CPOUI5FOUNDATION-826