Skip to content

Commit

Permalink
MDL-39378 Improve s() performance.
Browse files Browse the repository at this point in the history
These changes give about a 10% speed-up in this function. The significant changes are:
1. Simplify the if logic to remove unnecssary cases.
2. Dont pass default argument values to htmlspecialchars, just using the
   defaults is faster.
3. I can confirm that /i regex is faster than the equivalent regex without the i.

I also added more unit tests to test the edge cases.
  • Loading branch information
timhunt committed Apr 29, 2013
1 parent dc67a0e commit e47a7b9
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 8 deletions.
22 changes: 20 additions & 2 deletions lib/tests/weblib_test.php
Expand Up @@ -70,9 +70,27 @@ function test_format_string() {
} }


function test_s() { function test_s() {
$this->assertEquals(s("This Breaks \" Strict"), "This Breaks " Strict"); // Special cases.
$this->assertEquals(s("This Breaks <a>\" Strict</a>"), "This Breaks &lt;a&gt;&quot; Strict&lt;/a&gt;"); $this->assertSame('0', s(0));
$this->assertSame('0', s('0'));
$this->assertSame('0', s(false));
$this->assertSame('', s(null));

// Normal cases.
$this->assertEquals('This Breaks &quot; Strict', s('This Breaks " Strict'));
$this->assertEquals('This Breaks &lt;a&gt;&quot; Strict&lt;/a&gt;', s('This Breaks <a>" Strict</a>'));

// Unicode characters.
$this->assertEquals('Café', s('Café'));
$this->assertEquals('一, 二, 三', s('一, 二, 三'));

// Don't escape already-escaped numeric entities. (Note, this behaviour
// may not be desirable. Perhaps we should remove these tests and that
// functionality, but we can only do that if we understand why it was added.)
$this->assertEquals('An entity: &#x09ff;.', s('An entity: &#x09ff;.')); $this->assertEquals('An entity: &#x09ff;.', s('An entity: &#x09ff;.'));
$this->assertEquals('An entity: &#1073;.', s('An entity: &#1073;.'));
$this->assertEquals('An entity: &amp;amp;.', s('An entity: &amp;.'));
$this->assertEquals('Not an entity: &amp;amp;#x09ff;.', s('Not an entity: &amp;#x09ff;.'));
} }


function test_format_text_email() { function test_format_text_email() {
Expand Down
12 changes: 6 additions & 6 deletions lib/weblib.php
Expand Up @@ -85,19 +85,19 @@
* Returns $var with HTML characters (like "<", ">", etc.) properly quoted. * Returns $var with HTML characters (like "<", ">", etc.) properly quoted.
* This function is very similar to {@link p()} * This function is very similar to {@link p()}
* *
* @todo Remove obsolete param $obsolete if not used anywhere
*
* @param string $var the string potentially containing HTML characters * @param string $var the string potentially containing HTML characters
* @param boolean $obsolete no longer used.
* @return string * @return string
*/ */
function s($var, $obsolete = false) { function s($var) {


if ($var === '0' or $var === false or $var === 0) { if ($var === false) {
return '0'; return '0';
} }


return preg_replace("/&amp;#(\d+|x[0-9a-f]+);/i", "&#$1;", htmlspecialchars($var, ENT_QUOTES, 'UTF-8', true)); // When we move to PHP 5.4 as a minimum version, change ENT_QUOTES on the
// next line to ENT_QUOTES | ENT_HTML5 | ENT_SUBSTITUTE, and remove the
// 'UTF-8' argument. Both bring a speed-increase.
return preg_replace('/&amp;#(\d+|x[0-9a-f]+);/i', '&#$1;', htmlspecialchars($var, ENT_QUOTES, 'UTF-8'));
} }


/** /**
Expand Down

0 comments on commit e47a7b9

Please sign in to comment.