#63851 closed task (blessed) (fixed)
Audit wp_json_encode usage with script tags
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | General | Keywords: | good-first-bug has-patch |
Focuses: | javascript | Cc: |
Description (last modified by )
wp_json_encode()
is used to print data in script tags in many places without the appropriate JSON flags.
#62797 is an example of the general problem where wp_json_encode( $data )
is insufficient to safely print data in a script tag.
A detailed analysis of the problem can be found here where I recommend wp_json_encode( $data, JSON_HEX_TAG | JSON_UNESCAPED_SLASHES )
to encode data safely for use in script tags.
Note that safe in this context means that the content will not alter the expected HTML document structure. The SCRIPT
element will not close prematurely, nor will the element be prevented from closing at its apparent </script>
close tag. Safe in this context has nothing to do with the behavior of the JavaScript when evaluated by the browser. The JavaScript behavior in unaffected by these changes.
A good example of the ongoing issue is wp_localize_script()
. The following snippet breaks the containing script tag and prevents the script element from closing:
<?php wp_enqueue_script( '_example', '/ex.js', array(), '0.0', false ); wp_localize_script( '_example', '_', array( '<!--' => '<script>' ) );
This produces HTML like
<script id="_example-js-extra"> var _ = {"<!--":"<script>"}; </script> <script src="http://localhost:8888/ex.js?ver=0.0" id="_example-js"></script> …the rest of the HTML page
Resulting in a tree like
└─SCRIPT id="_example-js-extra" └─#text var _ = {"<!--":"<script>"}; </script> <script src="http://localhost:8888/ex.js?ver=0.0" id="_example-js"></script> …the rest of the HTML page
The SCRIPT element does not close as expected and the rest of the HTML page is part of the script contents.
[60648] fixed the issue in #62797 and is a good example of the necessary changes.
Change History (10)
#6
in reply to:
↑ 5
@
2 months ago
Replying to siliconforks:
Would it be better for
wp_json_encode()
to do this automatically?
Good question! How JSON (or anything) should be encoded for an HTML page depends on the context. This focuses on JSON in script tags. Elsewhere, JSON should be encoded in other ways with different flags. For example, JSON inside of a <pre>
tag would use a completely encoding strategy.
I wouldn't change the default behavior of wp_json_encode()
(largely analogous to PHP's json_encode()
).
#51159 suggests adding different flavors of wp_json_encode()
for different contexts. It's a good idea in theory, but it's difficult to know what contexts should be supported. Even then, some sites may want to use JSON_UNESCAPED_UNICODE
, but that isn't appropriate for all sites.
I'd like to focus on addressing these potential problems in Core on this ticket and leave the addition and implementation of new or different functionality aside.
This ticket was mentioned in PR #9557 on WordPress/wordpress-develop by @devasheeshkaul.
2 months ago
#7
- Keywords has-patch added
This patch audits the use of wp_json_encode()
where encoded data is used inside script tags/elements and updates those calls to use safer encoding flags (JSON_HEX_TAG
and JSON_UNESCAPED_SLASHES
)
Trac ticket: https://core.trac.wordpress.org/ticket/63851
@devasheeshkaul commented on PR #9557:
2 months ago
#8
How comprehensive is this audit? I believe there were other usages that included some JSON flags. Did you consider those cases and whether they use appropriate flags, or is this only the cases with no flags?
Thank you for the review, @sirreal! Yes, I went through every instance of wp_json_encode()
that is (or could be) used inside a script tag. From what I saw, the cases that already had JSON flags were also using the correct ones to prevent this issue.
I'll do another pass to double check all usages and update the patch if I spot anything missed.
I'm adding "good-first-bug" to this ticket. The necessary changes should be clear and not too invlied. [60648] is a good example for folks to follow.