Opened 8 years ago
Last modified 8 years ago
#40737 new defect (bug)
Script tags inside unclosed HTML comment in value passed to WP_Script->localize() breaks page
Reported by: | rmarscher | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Script Loader | Keywords: | has-patch |
Focuses: | javascript | Cc: |
Description
Hi. Thanks for your time. I encountered an edge case with the WP_Script->localize() method that can cause malformed html and cause javascript on the page to break.
Our site has a custom field for adding pixel tracking code html snippets. On the post edit admin page, the values from the custom fields are run through the localize method by the Wordpress-SEO plugin which adds their values to the wpseoReplaceVarsL10n var.
One of our users pasted code into this field like the following:
<!-- Code comment --!> <script> console.log('test'); </script>
This gets serialized to JSON by WP_Script->localize and ends up being output like this. I simplified the structure a bit for the example.
<script type='text/javascript'> /* <![CDATA[ */ var wpseoReplaceVarsL10n = {"custom_header_html":"<!-- Code comment --!>\r\n<script>\r\nconsole.log('test');\r\n<\/script>"}; /* ]]> */ </script>
This catches some interesting edge behavior in the browser. Notice the code comment is not actually closed correctly because it has --!>
instead of -->
. When testing that in html, I noticed that browsers seem to treat it as closing the comment and it doesn't break the page. It must be a common enough mistake.
If the field value is changed it have the proper -->
, there are no issues. It also requires the <script>
tag to be inside the unclosed comment for it to be an issue.
It can be worked around by replacing <script>
tags in the output with a string concatenated <scri" + "pt>
version. Other potential solutions would be replacing the <
and >
tag markers with unicode points like the JSON_HEX_TAG
option to json_encode()
in PHP 5.3+ does. But that will cause a lot more replacements and also increase the page size more.
I think str_replace( '<script', '<scr" + "ipt', wp_json_encode( $l10n ) )
is probably the least intrusive solution. I attached a patch with that update for your consideration.
Thanks for your time reviewing this.