1 Topic by arkonan

  • arkonan
  • Member
  • Offline

Topic: Problems after updating from 1.2.4 to 1.2.12

Hi,

I recently updated from 1.2.4 to 1.2.12 and had to modify htmLawed slightly to get our test suite working.  Here are the issues I encountered:


(1)  Depending on the “safe” configuration either ", app, javascript; *: data, javascript, file, http, https" or "; *: file, http, https” is appended to the end of the user-provided “schemes” configuration.  The documentation doesn’t mention that anything will be appended to the config string so I assume this is a bug.  It looks like an operator precedence/parentheses issue, potentially complicated by the ternary operator precedence changing between PHP 7 and 8:

$x = (isset($C['schemes'][2]) && strpos($C['schemes'], ':')
        ? strtolower($C['schemes'])
        : 'href: aim, feed, file, ftp, gopher, http, https, irc, mailto, news, nntp, sftp, ssh, tel, telnet')
       . (empty($C['safe'])
          ? ', app, javascript; *: data, javascript, '
          : '; *:')
       . 'file, http, https';

(2) Only <summary> is allowed as a child of <details> and all other tags are removed despite <details> supporting flow content.  I believe this is happening because in hl_balance() the <details> element is listed in $validMomKidAr.  It is also listed in $otherValidMomKidAr which seems like the correct place for it.


(3) A <br/> tag is stripped out but a <br /> with a space before the / is not stripped out.  I believe this is happening due to a change in the regex used in hl_tag() which detects the end of the tag by looking for whitespace or >.  Having it also stop on / appears to fix the problem.


(4) The <ruby> tag is not allowed to have text directly inside it.  As a result an example from MDN:

<ruby>
明日 <rp>(</rp><rt>Ashita</rt><rp>)</rp>
</ruby>

gets turned into 

<ruby>
     <rp>(</rp><rt>Ashita</rt><rp>)</rp>
</ruby>

2 Reply by patnaik (edited by patnaik 2023-04-30 01:11:50)

  • patnaik
  • Administrator
  • Offline

Re: Problems after updating from 1.2.4 to 1.2.12

Thank you for the valuable feedback. I am releasing version 1.2.13 to fix these issues:

1. Nesting/content logic for 'details' and 'ruby', and handling of '<br/>' are corrected to fix isssues #2-4.

2. Re: issue #1, there is no operator precedence difference between PHP versions 8 and older; note the parentheses. The documentation does mention that for attributes not specified in $config['schemes'], these schemes are permitted: file, http, https, plus app and javascript when $config['safe'] is 0.

3 Reply by arkonan

  • arkonan
  • Member
  • Offline

Re: Problems after updating from 1.2.4 to 1.2.12

Indenting the code in 1.2.12 so that the parens are more obvious:

        $x = (
                isset($C['schemes'][2]) && strpos($C['schemes'], ':')
                ? strtolower($C['schemes'])
                : 'href: aim, feed, file, ftp, gopher, http, https, irc, mailto, news, nntp, sftp, ssh, tel, telnet'
            )
            . (
                empty($C['safe']) 
                ? ', app, javascript; *: data, javascript, ' 
                : '; *:'
            )
            . 'file, http, https';

I am guessing the intent is not to append to $C['schemes'] if it is set?  My provided string is being overridden by the appended string.

In order to prevent the appending of the extra code to my config "schemes" I moved the parens so they are more like:

    $x = (isset($C['schemes'][2]) && strpos($C['schemes'], ':'))
         ? strtolower($C['schemes']) 
         : (
            'href: aim, feed, file, ftp, gopher, http, https, irc, mailto, news, nntp, sftp, ssh, tel, telnet'
            . (
                empty($C['safe']) 
                ? ', app, javascript; *: data, javascript, '
                : '; *:'
            )
            . 'file, http, https'
         );

I believe that the parentheses are the issue with the suffix getting added to my "schemes" configuration.  I simply wanted to mention the PHP 8+ operator precedence because I am using 8.1 and in Operator Precedence (https://www.php.net/manual/en/language.operators.precedence.php) they note:

non-associative    ? :    ternary (left-associative prior to PHP 8.0.0)

4 Reply by patnaik

  • patnaik
  • Administrator
  • Offline

Re: Problems after updating from 1.2.4 to 1.2.12

Thank you arkonan. You are right. I have now fixed this issue, affecting v1.2.8-1.2.12, in v1.2.13.

5 Reply by arkonan

  • arkonan
  • Member
  • Offline

Re: Problems after updating from 1.2.4 to 1.2.12

No problem, thanks for looking into it so quickly!