diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 9f6dcaa..c277787 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -17,7 +17,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Setup PHP uses: shivammathur/setup-php@v2 diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 382ff9a..64fbdf3 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -18,7 +18,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: fetch-depth: 0 @@ -34,7 +34,7 @@ jobs: - name: Cache Composer packages id: composer-cache - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: vendor key: ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }} @@ -66,7 +66,7 @@ jobs: echo "✅ Coverage is acceptable ($COVERAGE%)" - name: Comment PR with results - uses: actions/github-script@v7 + uses: actions/github-script@v8 if: always() with: script: | diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bb74326..f07d357 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -31,7 +31,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Setup PHP uses: shivammathur/setup-php@v2 diff --git a/composer.json b/composer.json index efcb811..abbb1db 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ }, "require-dev": { "phpunit/phpunit": "^10.5 || ^11.0", - "orchestra/testbench": "^8.0 || ^9.0 || ^10.0", + "orchestra/testbench": "^10.6.0", "squizlabs/php_codesniffer": "^3.6", "mockery/mockery": "^1.6" }, diff --git a/src/Middleware/InsertDNSPrefetch.php b/src/Middleware/InsertDNSPrefetch.php index abc9a70..8ab920f 100644 --- a/src/Middleware/InsertDNSPrefetch.php +++ b/src/Middleware/InsertDNSPrefetch.php @@ -6,16 +6,76 @@ class InsertDNSPrefetch extends PageSpeed { public function apply($buffer) { + // Extract URLs only from HTML attributes, not from script/style content + $urls = []; + + // Step 1: Extract URLs from script src/href attributes preg_match_all( - '#\bhttps?://[^\s()<>]+(?:\([\w\d]+\)|([^[:punct:]\s]|/))#', + '#]+src=["\']([^"\']+)["\']#i', $buffer, - $matches, - PREG_OFFSET_CAPTURE + $scriptMatches ); + if (!empty($scriptMatches[1])) { + $urls = array_merge($urls, $scriptMatches[1]); + } + + // Step 2: Extract URLs from link href attributes + preg_match_all( + '#]+href=["\']([^"\']+)["\']#i', + $buffer, + $linkMatches + ); + if (!empty($linkMatches[1])) { + $urls = array_merge($urls, $linkMatches[1]); + } + + // Step 3: Extract URLs from img src attributes + preg_match_all( + '#]+src=["\']?([^"\'\s>]+)["\']?#i', + $buffer, + $imgMatches + ); + if (!empty($imgMatches[1])) { + $urls = array_merge($urls, $imgMatches[1]); + } + + // Step 4: Extract URLs from anchor href attributes + preg_match_all( + '#]+href=["\']([^"\']+)["\']#i', + $buffer, + $anchorMatches + ); + if (!empty($anchorMatches[1])) { + $urls = array_merge($urls, $anchorMatches[1]); + } + + // Step 5: Extract URLs from iframe src attributes + preg_match_all( + '#]+src=["\']([^"\']+)["\']#i', + $buffer, + $iframeMatches + ); + if (!empty($iframeMatches[1])) { + $urls = array_merge($urls, $iframeMatches[1]); + } + + // Step 6: Extract URLs from video/audio source elements + preg_match_all( + '#<(?:video|audio|source)[^>]+src=["\']([^"\']+)["\']#i', + $buffer, + $mediaMatches + ); + if (!empty($mediaMatches[1])) { + $urls = array_merge($urls, $mediaMatches[1]); + } - $dnsPrefetch = collect($matches[0])->map(function ($item) { + // Filter to keep only external URLs (http:// or https://) + $externalUrls = array_filter($urls, function ($url) { + return preg_match('#^https?://#i', $url); + }); - $domain = (new TrimUrls)->apply($item[0]); + $dnsPrefetch = collect($externalUrls)->map(function ($url) { + $domain = (new TrimUrls)->apply($url); $domain = explode( '/', str_replace('//', '', $domain) diff --git a/src/Middleware/PageSpeed.php b/src/Middleware/PageSpeed.php index 98738da..af81364 100644 --- a/src/Middleware/PageSpeed.php +++ b/src/Middleware/PageSpeed.php @@ -49,7 +49,16 @@ public function handle($request, Closure $next) */ protected function replace(array $replace, $buffer) { - return preg_replace(array_keys($replace), array_values($replace), $buffer); + $result = preg_replace(array_keys($replace), array_values($replace), $buffer); + + // Check for PCRE errors (e.g., backtrack limit, recursion limit exceeded) + if ($result === null && preg_last_error() !== PREG_NO_ERROR) { + // Log the error or handle it appropriately + // For now, return the original buffer to prevent blank pages + return $buffer; + } + + return $result; } /** diff --git a/src/Middleware/RemoveComments.php b/src/Middleware/RemoveComments.php index 0a24d93..7b0aa40 100644 --- a/src/Middleware/RemoveComments.php +++ b/src/Middleware/RemoveComments.php @@ -11,7 +11,7 @@ public function apply($buffer) { // First, remove multi-line comments (/* ... */) $buffer = $this->replaceInsideHtmlTags(['script', 'style'], self::REGEX_MATCH_MULTILINE_COMMENTS, '', $buffer); - + // Then, remove single-line comments (//) more carefully $buffer = $this->removeSingleLineComments($buffer); @@ -21,7 +21,7 @@ public function apply($buffer) return $this->replace($replaceHtmlRules, $buffer); } - + /** * Remove single-line comments (//) from script tags while preserving them inside strings * @@ -34,10 +34,10 @@ protected function removeSingleLineComments($buffer) $tagAfterReplace = $this->removeCommentsFromTag($tagMatched); $buffer = str_replace($tagMatched, $tagAfterReplace, $buffer); } - + return $buffer; } - + /** * Remove // comments from a script/style tag content * @@ -53,18 +53,18 @@ protected function removeCommentsFromTag($tag) } elseif (strpos($tag, "\r") !== false) { $lineEnding = "\r"; } - + // Split by lines to process each line $lines = preg_split('/\r\n|\r|\n/', $tag); $processedLines = []; - + foreach ($lines as $line) { $processedLines[] = $this->removeSingleLineCommentFromLine($line); } - + return implode($lineEnding, $processedLines); } - + /** * Remove // comment from a single line while preserving // inside strings * @@ -79,38 +79,38 @@ protected function removeSingleLineCommentFromLine($line) $inDoubleQuote = false; $inRegex = false; $escaped = false; - + for ($i = 0; $i < $length; $i++) { $char = $line[$i]; $nextChar = $i + 1 < $length ? $line[$i + 1] : ''; $prevChar = $i > 0 ? $line[$i - 1] : ''; - + // Handle escape sequences if ($escaped) { $result .= $char; $escaped = false; continue; } - + if ($char === '\\' && ($inSingleQuote || $inDoubleQuote || $inRegex)) { $result .= $char; $escaped = true; continue; } - + // Toggle quote states if ($char === '"' && !$inSingleQuote && !$inRegex) { $inDoubleQuote = !$inDoubleQuote; $result .= $char; continue; } - + if ($char === "'" && !$inDoubleQuote && !$inRegex) { $inSingleQuote = !$inSingleQuote; $result .= $char; continue; } - + // Handle regex literals (basic detection) if ($char === '/' && !$inSingleQuote && !$inDoubleQuote) { // Check if this might be a regex literal @@ -123,7 +123,7 @@ protected function removeSingleLineCommentFromLine($line) continue; } } - + // End of regex literal if ($inRegex) { $inRegex = false; @@ -131,7 +131,7 @@ protected function removeSingleLineCommentFromLine($line) continue; } } - + // Check for // comment outside of strings if (!$inSingleQuote && !$inDoubleQuote && !$inRegex && $char === '/' && $nextChar === '/') { // Check if this is not part of a URL (preceded by :) @@ -140,10 +140,10 @@ protected function removeSingleLineCommentFromLine($line) break; } } - + $result .= $char; } - + return $result; } } diff --git a/tests/Middleware/CollapseWhitespaceTest.php b/tests/Middleware/CollapseWhitespaceTest.php index a1a68cd..12cab95 100644 --- a/tests/Middleware/CollapseWhitespaceTest.php +++ b/tests/Middleware/CollapseWhitespaceTest.php @@ -43,13 +43,13 @@ public function test_collapse_whitespace(): void $this->response->getContent() ); } - + public function test_javascript_not_broken_by_comment_removal_and_whitespace_collapse(): void { // This test ensures that when comments are removed and whitespace is collapsed, // the JavaScript code remains functional and nothing is accidentally commented out $content = $this->response->getContent(); - + // Ensure all expected JavaScript statements are present $this->assertStringContainsString("console.log('Laravel');", $content); $this->assertStringContainsString("console.log('Page');", $content); @@ -57,7 +57,7 @@ public function test_javascript_not_broken_by_comment_removal_and_whitespace_col $this->assertStringContainsString('var url = "http://example.com";', $content); $this->assertStringContainsString('var text = "Some text";', $content); $this->assertStringContainsString("console.log('Important code');", $content); - + // Ensure comments are removed $this->assertStringNotContainsString("// This comment should be removed", $content); $this->assertStringNotContainsString("// This comment should also be removed", $content); diff --git a/tests/Middleware/EdgeCaseCommentsTest.php b/tests/Middleware/EdgeCaseCommentsTest.php index 681c586..49eaccb 100644 --- a/tests/Middleware/EdgeCaseCommentsTest.php +++ b/tests/Middleware/EdgeCaseCommentsTest.php @@ -31,28 +31,28 @@ public function test_comment_with_special_characters_before_it(): void $request = Request::create('/test', 'GET'); $response = new Response($html); - + $collapseWhitespace = new CollapseWhitespace(); - $final = $collapseWhitespace->handle($request, function() use ($response) { + $final = $collapseWhitespace->handle($request, function () use ($response) { return $response; }); - + $content = $final->getContent(); - + // All code should be present $this->assertStringContainsString('var a = 1;', $content); $this->assertStringContainsString('var b = 2;', $content); $this->assertStringContainsString('var c = 3;', $content); $this->assertStringContainsString('var d = 4;', $content); $this->assertStringContainsString('doSomething();', $content); - + // Comments should be removed $this->assertStringNotContainsString('// Comment without space', $content); $this->assertStringNotContainsString('// Comment with space', $content); $this->assertStringNotContainsString('// Comment with multiple spaces', $content); $this->assertStringNotContainsString('// Comment with tab', $content); } - + /** * Test comment after closing quote or parenthesis */ @@ -77,14 +77,14 @@ public function test_comment_after_various_characters(): void $request = Request::create('/test', 'GET'); $response = new Response($html); - + $collapseWhitespace = new CollapseWhitespace(); - $final = $collapseWhitespace->handle($request, function() use ($response) { + $final = $collapseWhitespace->handle($request, function () use ($response) { return $response; }); - + $content = $final->getContent(); - + // All code should be executable $this->assertStringContainsString('func();', $content); $this->assertStringContainsString('var x = getValue();', $content); @@ -93,11 +93,11 @@ public function test_comment_after_various_characters(): void $this->assertStringContainsString('var arr = [1, 2];', $content); $this->assertStringContainsString('var obj = {a: 1};', $content); $this->assertStringContainsString('next();', $content); - + // No comments should remain $this->assertStringNotContainsString('// After', $content); } - + /** * Test that // inside strings is not treated as comment */ @@ -120,21 +120,21 @@ public function test_slashes_inside_strings_are_preserved(): void $request = Request::create('/test', 'GET'); $response = new Response($html); - + $collapseWhitespace = new CollapseWhitespace(); - $final = $collapseWhitespace->handle($request, function() use ($response) { + $final = $collapseWhitespace->handle($request, function () use ($response) { return $response; }); - + $content = $final->getContent(); - + // Content inside strings should be preserved $this->assertStringContainsString('http://', $content); $this->assertStringContainsString('https://example.com/path', $content); $this->assertStringContainsString('This is not a // comment', $content); $this->assertStringContainsString('doSomething();', $content); } - + /** * Test regex pattern with slashes */ @@ -156,14 +156,14 @@ public function test_regex_patterns_are_preserved(): void $request = Request::create('/test', 'GET'); $response = new Response($html); - + $collapseWhitespace = new CollapseWhitespace(); - $final = $collapseWhitespace->handle($request, function() use ($response) { + $final = $collapseWhitespace->handle($request, function () use ($response) { return $response; }); - + $content = $final->getContent(); - + // Regex patterns should be preserved $this->assertStringContainsString('/test\/pattern/', $content); $this->assertStringContainsString('/http:\/\//', $content); diff --git a/tests/Middleware/InsertDNSPrefetchTest.php b/tests/Middleware/InsertDNSPrefetchTest.php index 032ce84..f3b9e3b 100644 --- a/tests/Middleware/InsertDNSPrefetchTest.php +++ b/tests/Middleware/InsertDNSPrefetchTest.php @@ -22,4 +22,81 @@ public function test_insert_dns_prefetch(): void $this->assertStringContainsString('', $response->getContent()); $this->assertStringContainsString('', $response->getContent()); } + + public function test_dns_prefetch_ignores_urls_inside_json_ld_script(): void + { + $html = <<<'HTML' + + + + Test + + + + Real Link + + +HTML; + + $middleware = new InsertDNSPrefetch(); + $actual = $middleware->apply($html); + + // Should include DNS prefetch for actual HTML links + $this->assertStringContainsString('', $actual); + + // Should NOT include DNS prefetch for URLs inside JSON-LD script + $this->assertStringNotContainsString('', $actual); + $this->assertStringNotContainsString('', $actual); + } + + public function test_dns_prefetch_ignores_urls_inside_javascript(): void + { + $html = <<<'HTML' + + + + Test + + + + + + +HTML; + + $middleware = new InsertDNSPrefetch(); + $actual = $middleware->apply($html); + + // Should include DNS prefetch for actual HTML attributes + $this->assertStringContainsString('', $actual); + + // Should NOT include DNS prefetch for URLs inside JavaScript + $this->assertStringNotContainsString('', $actual); + $this->assertStringNotContainsString('', $actual); + } } diff --git a/tests/Middleware/JavaScriptCommentsBugTest.php b/tests/Middleware/JavaScriptCommentsBugTest.php index 2f6f7f4..78994bd 100644 --- a/tests/Middleware/JavaScriptCommentsBugTest.php +++ b/tests/Middleware/JavaScriptCommentsBugTest.php @@ -32,36 +32,36 @@ public function test_javascript_comments_dont_break_code_after_collapse(): void $request = Request::create('/test', 'GET'); $response = new Response($html); - + $removeComments = new RemoveComments(); $collapseWhitespace = new CollapseWhitespace(); - + // Apply both middlewares - $afterRemoveComments = $removeComments->handle($request, function() use ($response) { + $afterRemoveComments = $removeComments->handle($request, function () use ($response) { return $response; }); - - $final = $collapseWhitespace->handle($request, function() use ($afterRemoveComments) { + + $final = $collapseWhitespace->handle($request, function () use ($afterRemoveComments) { return $afterRemoveComments; }); - + $content = $final->getContent(); - + // The code should still be present and not commented out $this->assertStringContainsString('var x = 1;', $content); $this->assertStringContainsString('var y = 2;', $content); $this->assertStringContainsString('console.log(x + y);', $content); - + // Comments should be removed $this->assertStringNotContainsString('// First comment', $content); $this->assertStringNotContainsString('// Second comment', $content); - + // Make sure the second and third lines are not commented out // This would happen if the comment wasn't removed and newlines were collapsed $this->assertStringNotContainsString('// First comment var y = 2;', $content); $this->assertStringNotContainsString('// Second comment console.log', $content); } - + /** * Test edge case: comment after string with quote */ @@ -82,29 +82,29 @@ public function test_comment_after_string_is_removed(): void $request = Request::create('/test', 'GET'); $response = new Response($html); - + $collapseWhitespace = new CollapseWhitespace(); - - $final = $collapseWhitespace->handle($request, function() use ($response) { + + $final = $collapseWhitespace->handle($request, function () use ($response) { return $response; }); - + $content = $final->getContent(); - + // All code should be present $this->assertStringContainsString('var url = "http://example.com";', $content); $this->assertStringContainsString("var name = 'John';", $content); $this->assertStringContainsString('doSomething();', $content); - + // Comments should be removed $this->assertStringNotContainsString('// URL comment', $content); $this->assertStringNotContainsString('// Name comment', $content); - + // Ensure nothing is accidentally commented out $this->assertStringNotContainsString('// URL comment var name', $content); $this->assertStringNotContainsString('// Name comment doSomething', $content); } - + /** * Test that URLs with // are not affected */ @@ -125,15 +125,15 @@ public function test_urls_with_slashes_are_preserved(): void $request = Request::create('/test', 'GET'); $response = new Response($html); - + $collapseWhitespace = new CollapseWhitespace(); - - $final = $collapseWhitespace->handle($request, function() use ($response) { + + $final = $collapseWhitespace->handle($request, function () use ($response) { return $response; }); - + $content = $final->getContent(); - + // URLs should be preserved $this->assertStringContainsString('http://example.com', $content); $this->assertStringContainsString('https://test.com', $content); diff --git a/tests/Middleware/LargeHtmlTest.php b/tests/Middleware/LargeHtmlTest.php new file mode 100644 index 0000000..f2e6361 --- /dev/null +++ b/tests/Middleware/LargeHtmlTest.php @@ -0,0 +1,119 @@ +generateLargeHtml(5000); // 5000 lines + + $request = Request::create('/test', 'GET'); + $response = new Response($largeContent); + + $middleware = new RemoveComments(); + $result = $middleware->handle($request, function () use ($response) { + return $response; + }); + + $content = $result->getContent(); + + // Ensure HTML comments are removed + $this->assertStringNotContainsString('', $content); + $this->assertStringNotContainsString('', $content); + + // Ensure JavaScript comments are removed + $this->assertStringNotContainsString('// JavaScript comment', $content); + $this->assertStringNotContainsString('/* Multi-line comment */', $content); + + // Ensure actual content is preserved + $this->assertStringContainsString('console.log("test");', $content); + $this->assertStringContainsString('
', $content); + } + + /** + * Test with extremely large HTML (edge case) + */ + public function test_comments_removed_in_extremely_large_html(): void + { + // Generate an extremely large HTML document (20,000 lines) + $largeContent = $this->generateLargeHtml(20000); + + $request = Request::create('/test', 'GET'); + $response = new Response($largeContent); + + $middleware = new RemoveComments(); + $result = $middleware->handle($request, function () use ($response) { + return $response; + }); + + $content = $result->getContent(); + + // Ensure HTML comments are removed + $this->assertStringNotContainsString('', $content); + + // Ensure JavaScript comments are removed + $this->assertStringNotContainsString('// JavaScript comment', $content); + + // Ensure the page is not blank + $this->assertNotEmpty($content); + $this->assertGreaterThan(10000, strlen($content)); + } + + /** + * Generate large HTML content for testing + */ + private function generateLargeHtml(int $lines): string + { + $html = <<<'HTML' + + + + Large HTML Test + + + + + + +HTML; + + // Add many content blocks + for ($i = 0; $i < $lines; $i++) { + $html .= "\n
\n"; + $html .= " \n"; + $html .= "

Content line $i

\n"; + $html .= " \n"; + $html .= "
"; + } + + $html .= "\n\n"; + + return $html; + } + + protected function getMiddleware() + { + // Not used in this test + } +} diff --git a/tests/Middleware/RemoveCommentsTest.php b/tests/Middleware/RemoveCommentsTest.php index 32dc39f..6a87a9b 100644 --- a/tests/Middleware/RemoveCommentsTest.php +++ b/tests/Middleware/RemoveCommentsTest.php @@ -125,12 +125,12 @@ public function test_remove_js_comments(): void $this->assertStringContainsString("console.log('Laravel');", $this->response->getContent()); $this->assertStringContainsString("console.log('Page');", $this->response->getContent()); $this->assertStringContainsString("console.log('Speed!');", $this->response->getContent()); - + // Test that comments after strings are properly removed $this->assertStringNotContainsString("// This comment should be removed", $this->response->getContent()); $this->assertStringNotContainsString("// This comment should also be removed", $this->response->getContent()); $this->assertStringNotContainsString("// Don't break this", $this->response->getContent()); - + // Ensure the actual code is still there $this->assertStringContainsString('var url = "http://example.com";', $this->response->getContent()); $this->assertStringContainsString('var text = "Some text";', $this->response->getContent());