From 55eb7ebdcc9ba767f978dff510614bbd2e0c309d Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 7 Apr 2017 12:17:38 +1200 Subject: [PATCH] FIX Do not insert requirements more than once in includeInHTML This change consolidates the string replacements used to insert requirements into the page content to help ensure that they are not compounding and overwriting eachother. The added test case includes where a user may have a Javascript snippet that contains a closing head tag, and the test ensures that it does not get injected with requirements as well as the actual head tag in the DOM. --- tests/forms/RequirementsTest.php | 30 ++++++++++++++++++++++++ view/Requirements.php | 40 +++++++++++++++++++++----------- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/tests/forms/RequirementsTest.php b/tests/forms/RequirementsTest.php index aa459766f..2f1f2341c 100644 --- a/tests/forms/RequirementsTest.php +++ b/tests/forms/RequirementsTest.php @@ -452,6 +452,36 @@ class RequirementsTest extends SapphireTest { $this->assertNotRegexp('/RequirementsTest_b\.css\?m=[\d]*&foo=bar&bla=blubb"/', $html); } + /** + * Ensure that if a JS snippet somewhere in the page (via requirements) contains that it doesn't + * get injected with requirements + */ + public function testHeadTagIsNotInjectedTwice() { + $template = '
My header

Body

'; + $basePath = $this->getCurrentRelativePath(); + + $backend = new Requirements_Backend; + // The offending snippet that contains a closing head tag + $backend->customScript('var myvar="";'); + // Insert a variety of random requirements + $backend->css($basePath .'/RequirementsTest_a.css'); + $backend->javascript($basePath .'/RequirementsTest_a.js'); + $backend->insertHeadTags(''); + + // Case A: JS forced to bottom + $backend->set_force_js_to_bottom(true); + $this->assertContains('var myvar="";', $backend->includeInHTML(false, $template)); + + // Case B: JS written to body + $backend->set_force_js_to_bottom(false); + $backend->set_write_js_to_body(true); + $this->assertContains('var myvar="";', $backend->includeInHTML(false, $template)); + + // Case C: Neither of the above + $backend->set_write_js_to_body(false); + $this->assertContains('var myvar="";', $backend->includeInHTML(false, $template)); + } + public function assertFileIncluded($backend, $type, $files) { $type = strtolower($type); switch (strtolower($type)) { diff --git a/view/Requirements.php b/view/Requirements.php index b2e1eaa80..4b1bd7b33 100644 --- a/view/Requirements.php +++ b/view/Requirements.php @@ -863,19 +863,18 @@ class Requirements_Backend { $requirements .= "$customHeadTag\n"; } + $replacements = array(); if ($this->force_js_to_bottom) { - // Remove all newlines from code to preserve layout - $jsRequirements = preg_replace('/>\n*/', '>', $jsRequirements); + $jsRequirements = $this->removeNewlinesFromCode($jsRequirements); // Forcefully put the scripts at the bottom of the body instead of before the first // script tag. - $content = preg_replace("/(<\/body[^>]*>)/i", $jsRequirements . "\\1", $content); + $replacements["/(<\/body[^>]*>)/i"] = $jsRequirements . "\\1"; // Put CSS at the bottom of the head - $content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content); - } elseif($this->write_js_to_body) { - // Remove all newlines from code to preserve layout - $jsRequirements = preg_replace('/>\n*/', '>', $jsRequirements); + $replacements["/(<\/head>)/i"] = $requirements . "\\1"; + } elseif ($this->write_js_to_body) { + $jsRequirements = $this->removeNewlinesFromCode($jsRequirements); // If your template already has script tags in the body, then we try to put our script // tags just before those. Otherwise, we put it at the bottom. @@ -892,23 +891,38 @@ class Requirements_Backend { $commentTags[1] == '-->' ); - if($canWriteToBody) { - $content = substr($content,0,$p1) . $jsRequirements . substr($content,$p1); + if ($canWriteToBody) { + $content = substr($content, 0, $p1) . $jsRequirements . substr($content, $p1); } else { - $content = preg_replace("/(<\/body[^>]*>)/i", $jsRequirements . "\\1", $content); + $replacements["/(<\/body[^>]*>)/i"] = $jsRequirements . "\\1"; } // Put CSS at the bottom of the head - $content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content); + $replacements["/(<\/head>)/i"] = $requirements . "\\1"; } else { - $content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content); - $content = preg_replace("/(<\/head>)/i", $jsRequirements . "\\1", $content); + // Put CSS and Javascript together before the closing head tag + $replacements["/(<\/head>)/i"] = $requirements . $jsRequirements. "\\1"; + } + + if (!empty($replacements)) { + // Replace everything at once (only once) + $content = preg_replace(array_keys($replacements), array_values($replacements), $content, 1); } } return $content; } + /** + * Remove all newlines from code to preserve layout + * + * @param string $code + * @return string + */ + protected function removeNewlinesFromCode($code) { + return preg_replace('/>\n*/', '>', $code); + } + /** * Attach requirements inclusion to X-Include-JS and X-Include-CSS headers on the given * HTTP Response