wp.sanitize.stripTags could rely on the browser for HTML parsing

[dmsnell]

This JavaScript function recreates emblematic problems in HTML parsing by relying on over-simplified regular expressions. It runs, however, inside a browser where a reliable HTML parser exists, and so could or should lean on that parser in order to operate.

See ​discussion. Follow-up to [60907] for #48054.

[hbhalodia]

Hi @westonruter @dmsnell, If I am collecting it correctly, we should remove the usage of regex and use something like DOMParser or a simple HTML tag via createElement, add it as an innerHTML and extract the innerText from it and return that?

Something like below,

const parser = new DOMParser();
const doc = parser.parseFromString( text, 'text/html' );

return doc.body.innerText || '';

or

const element = document.createElement( 'div' );
element.innerHTML = text;

return element.innerText;

Let me know if this is something we need to update?

Thanks,

[dmsnell]

@hbhalodia yes exactly! though I believe that the script is trying to remove comments, tags, and SCRIPT or STYLE contents. I believe that .innerText returns this, but I always get confused between that and .textContent.

[hbhalodia]

Yeah, it's bit confusing. I guess innerText returns the content that is usually seen in frontend, so for example if div has display: none; it would not return innerText from that div, but if we use textContent, it would return and also preserves the spaces and indentation.

So I am not sure as well what to use here, but Ideally if we do not need to preserve indentation, spaces we should go with textContent, instead of innerText but I am open to suggestions.

Cc: @westonruter @dmsnell

[dmsnell]

sounds good @hbhalodia; looking forward to your PR

looks like this came in through [31534], but that itself might have been influenced by Prototype.js which was added in 2006. nice longstanding opportunity 😉

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

• Used HTML parsing instead of regex matching.

[hbhalodia]

Hi @dmsnell, I do have raised the PR, Can you please review and suggest improvements if any?

Thank You,

@dmsnell Here's the result of the file being minified as wp-includes/js/wp-sanitize.min.js via npm run build:dev:

/*! This file is auto-generated */
window.wp=window.wp||{},wp.sanitize={stripTags:function(t){t=(new DOMParser).parseFromString(t,"text/html");return t.body.innerText=t.body.innerText,t.body.innerHTML},stripTagsAndEncodeText:function(t){let e=wp.sanitize.stripTags(t),n=document.createElement("textarea");try{n.textContent=e,e=wp.sanitize.stripTags(n.value)}catch(t){}return e}};

With prettier formatting:

/*! This file is auto-generated */
((window.wp = window.wp || {}),
  (wp.sanitize = {
    stripTags: function (t) {
      t = new DOMParser().parseFromString(t, "text/html");
      return ((t.body.innerText = t.body.innerText), t.body.innerHTML);
    },
    stripTagsAndEncodeText: function (t) {
      let e = wp.sanitize.stripTags(t),
        n = document.createElement("textarea");
      try {
        ((n.textContent = e), (e = wp.sanitize.stripTags(n.value)));
      } catch (t) {}
      return e;
    },
  }));

When combined with:

const unsafeText = `Hello: <style style="display:block"><script>document.write('evil');</script></style>[[Image(https://s.w.org/style/images/about/WordPress-logotype-wmark.png)]]`;

console.log(wp.sanitize.stripTags(unsafeText));

The output is as expected:

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

If you don’t beat me to it, I’ll try and merge this today.

@dmsnell Looks like there is a ​commit freeze until after 6.9 is released.

I've drafted the following commit message:

General: Leverage `DOMParser` to implement `wp.sanitize.stripTags()`.

Developed in https://github.com/WordPress/wordpress-develop/pull/10536.

Follow-up to [60907].

Props hbhalodia, dmsnell, westonruter.
See #48054.
Fixes #64274.

[westonruter]

In 61347:

General: Leverage DOMParser to implement wp.sanitize.stripTags().

Developed in ​https://github.com/WordPress/wordpress-develop/pull/10536

Follow-up to [60907].

Props hbhalodia, dmsnell, westonruter.
See #48054.
Fixes #64274.

[westonruter]

Regression identified: #64574

[westonruter]

In 61578:

General: Preserve back-compat for wp.sanitize.stripTags() to return empty string when passed null/undefined.

This also bumps the esversion to 11 in tests/qunit/.jshintrc to match the root .jshintrc, following [61544] for #64562.

Follow-up to [61347], [60907].

Props westonruter, jonsurrell, mukesh27, hugod.
See #64274.
Fixes #64574.

[westonruter]

In 61585:

General: Further preserve back-compat for wp.sanitize.stripTags() to return empty string when falsy value supplied.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/10856

Follow-up to [61578], [61347], [60907].

Props jonsurrell, hugod, westonruter.
See #64274.
Fixes #64574.

[westonruter]

In 61783:

General: Update wp.sanitize.stripTags() to return empty string when not passed a string.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/10994

Follow-up to r61585, r61578.

Props westonruter, jonsurrell, dmsnell, hugod.
See #64574, #64274.