wp_add_inline_script does not properly escape '<!-- <script>' in contents

[artpi]

I am adding this issue in the "Editor" because this is how it's easiest to reproduce and how I stumbled onto it, but it is possible to replace the issue outside of editor context.

Reproduction path

1. Start writing a post in Gutenberg
2. Enter code editor
3. Paste following code

<p><!-- <script> </p>

   </p>

4. Save
5. Exit code editor into block editor
6. Reload the page
7. Observe the editor crash and burn

Here is video of a reproduction on a fresh site:

​https://screen.studio/share/ELc9d6zJ

What is going on?

block_editor_rest_api_preload is using wp_add_inline_script to preload the currently edited block content into the page
​https://github.com/WordPress/WordPress/blob/0086f4ba4080285c90c08854a5d085c76b6d5109/wp-includes/block-editor.php#L767

Because the <script> tag is hidden in an comment that has not been properly terminated, it is impossible to get out of the script tag now, and nothing gets rendered afterwards.

Tested up to WordPress version 6.7.1

[ankitkumarshah]

Reproduction Report

Description

This report validates whether the issue can be reproduced.

Environment

• WordPress: 6.8-alpha-59593
• PHP: 8.1.29
• Server: nginx/1.16.0
• Database: mysqli (Server: 8.0.16 / Client: mysqlnd 8.1.29)
• Browser: Chrome 131.0.0.0
• OS: macOS
• Theme: Twenty Seventeen 3.8
• MU Plugins: None activated
• Plugins:
  • Gutenslider — The last WordPress slider you will ever need. 6.1.0
  • Test Plugin
  • Test Reports 1.2.0
  • WordPress Beta Tester 3.6.2

Actual Results

✅ Error condition occurs (reproduced).
Hi @artpi,

Thank you for bringing this up. By following the provided steps, I reproduced the issue successfully.

Screencast

​https://ppgtp1rtd3.ufs.sh/f/TnWMEUzoUd85Fb83cfTuejSJhDdioPMH6NAwRtYmfvQBT3W9

[abcd95]

Hey @artpi, Thanks for bringing this up.

I was able to reproduce the issue - ​Screencast

The problem occurs because wp_add_inline_script() injects the block content inside a <script> tag for preloading. When there's an unclosed HTML comment containing <script> in the content, it breaks out of the string context since browsers parse this as the start of a new script tag, causing the editor to crash.

I will work on this to find a fix.

Additional Note: This happens with all variants of the script like <!-- <SCRIPT>

[dilipbheda]

@artpi, thank you for the report. I've addressed it with the attached PR.

@abcd95 @ankitkumarshah, could you please review the attached PR and let me know if you find any other issues?

Thanks!

[sainathpoojary]

Hey @dilipbheda,

I have tested the PR. While trying to reproduce the issue, I encountered this warning. It might be related to a regex issue in this ​line

[jonsurrell]

​The issue has been present in some form since 5.0.

[jonsurrell]

A fix for all cases from the wp_add_inline_script side will be difficult. It may be something the HTML API could handle.

The problem in this case is with the JSON encoding. Use of the correct flags should fix the problem, namely JSON_HEX_TAG.

​A good example to follow is this code used by script modules.

Fixes #62797 by correctly escaping JSON included inside JavaScript in a script tag.

wp_add_inline_script will correctly escape closing script tags </script> which prevents script contents from breaking out of the script, it does not handle cases where some content may enter the HTML ​script data double escaped state and prevent the real script closing tag from closing the script tag as expected, leading to a broken document where the script tag remains open.

This can be tested by applying the patch and following the reproduction steps in #62797.

Trac ticket: https://core.trac.wordpress.org/ticket/62797

[shanemuir]

Testing without patch applied.

[shanemuir]

Testing with patch applied.

[shanemuir]

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: ​https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/8145.diff

Environment

• WordPress: 6.8-alpha-59274-src
• PHP: 8.3.15
• Server: Apache/2.4.62 (Unix) OpenSSL/3.4.0 PHP/8.3.15
• Database: mysqli (Server: 8.0.39 / Client: mysqlnd 8.3.15)
• Browser: Chrome 132.0.0.0
• OS: macOS
• Theme: Twenty Twenty-Five 1.0
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Actual Results

1. ✅ Issue resolved with patch.

Additional Notes

Only testing using @jonsurrell patch, let me know if we need to test the other patch too?

Supplemental Artifacts

Added videos of testing with and without patch applied.

• Fix json encode that may break enclosing script tag
• Update or fix tests
• Try encoding '<' in inline script tags

Trac ticket:

This won't work, SCRIPT tags have special parsing rules and HTML entities are not escaped.

Will you explain the intention of this change and how it addresses the issue?

I plan to add tests to this using assertEqualMarkup when it lands in ​https://github.com/WordPress/wordpress-develop/pull/8882.

I'm concerned this pattern is very common. I wonder if there should be some dedicated functions.

Here are some examples of this exact pattern in the site editor that are susceptible to the same problem:

​https://github.com/WordPress/wordpress-develop/blob/f83e66389dbe3164ccf0fb2a1132064fb161b8e4/src/wp-admin/site-editor.php#L254-L292

I'm concerned this pattern is very common. I wonder if there should be some dedicated functions.

What’s your concern? that it will contain </script> tag closers?

What’s your concern? that it will contain </script> tag closers?

That's the problem here that may produce a broken page. I'm unaware of others, but wp_add_inline_script + wp_json_encode seem common to use together but risky with the default arguments.

I've pushed a test leveraging assertEqualHTML with the fix reverted. That test will fail on CI, then I'll push the fix again.

The failure appears as Error: Paused at incomplete token. because the <script> tag remains unclosed.

​Example failures on CI can be observed here:

1) Tests_Blocks_Editor::test_preload_closes_script_tag
Error: Paused at incomplete token.

(I'm updating the test name after the CI run, but the result is the same).

That's the problem here that may produce a broken page. I'm unaware of others, but wp_add_inline_script + wp_json_encode seem common to use together but risky with the default arguments.

Could we perhaps create a new Issue or document where we can start enumerating the situations in which this can go bad? perhaps a new test suite with no new code, in a PR.

I find that it’s hard for me to grasp unless it’s a particularly good day and I have my full concentration.

There are differences, aren’t there, between printing these three situations?

<script type="application/json">
{ "some": "json" }
</script>

<script>
const data = { "some": "json" };
</script>

<script>
callMeMaybe( { "some": "json" } );
</script>

I thought these were distinct cases with different edges but I can’t remember how.