Address an issue where <script> tags aren't stripped.

[kienstra]

Request For Code Review

Hi @ThierryA,
Could you please review this pull request? The target branch is now 0.6, but I don't know if that's right.

This addresses the issue that @westonruter raised. Before, when you added this to the post content:

Hello <script>document.write('world');</script>

It produced:

<p>Hello <script><![CDATA[document.write('world');]]></script></p>

The expected output is:

<p>Hello </p>

This looks to be related to how process_node() gets the $attr_spec_list. In some cases, all of the specs will have an equal 'score,' as indicated in $spec_ids_sorted.

And sometimes, all of them will be missing a mandatory attribute. In that case, $attr_spec_list will be empty.

If it is, this uses the first spec list in $rule_spec_list_to_validate. This will need regression testing.

[westonruter]

Note that the CDATA section will be eliminated with #891.

[ThierryA]

Great work on debugging this @kienstra. Changes are approved from my perspective.

[kienstra]

Basic Testing

Hi @ThierryA and @westonruter,
Thanks for working on this. I've been doing basic testing of this, visiting pages from the WordPress theme unit test data and the embed test page from the wp-cli script.

I haven't seen any regression yet. But some more sanity-checking would probably help.

[westonruter]

@kienstra I used git-bisect between 0.5.1 and 0.6 to find the commit that introduced the problem. Given this script https://gist.github.com/westonruter/39d251d6f3ff8f9958f8fd1d11167da5 I found that the issue appeared as of a066117. Prior to that commit, the script tag is successfully stripped.

[westonruter]

I tested the page generated via create-embed-test-post.php and it is outputting a lot of <script async=""></script> which are making it AMP-invalid. These aren't getting sanitized from source HTML like <script async src="//cdn.someecards.com/assets/embed/embed-v1.07.min.js" charset="utf-8"></script>.

[kienstra]

Tested, No Regressions Found

Hi @westonruter,
Your commits above look good. I tested them on the AMP version of about 20 posts and pages from the Theme Unit Test Data.

There weren't any regressions from the earlier version, 0.5.1. Of course, we're still working toward support of some embeds that we didn't expect to work yet in this version. But there were no new AMP errors.