Repository: ampproject/amp-toolbox-php (php)

Issue:
Uncaught `DomException` in `Document::moveInvalidHeadNodesToBody`
When trying to parse `https://chuseok.org/places-to-visit-during-chuseok-mid-autumn-summer/` with `Document::fromHtml()`, I get an uncaught `DomException`:

```
PHP Fatal error:  Uncaught DOMException: Not Found Error in /vendor/ampproject/amp-toolbox/src/Dom/Document.php:553
```

This is the offending code:

https://github.com/ampproject/amp-toolbox-php/blob/89cd1f6bad3512fb8a08aae5fb6aedc64e988392/src/Dom/Document.php#L655-L669

PHPStorm warns about accessing `$node` potentially causing a null pointer exception too.

This fixes the bug for me:

```dif
-while ($node) {
+while ($node !== null) {
```

Pull request:
Fix error when source is missing closing head tag
Fixes #522 

This PR fixes the uncaught `DOMException: Not Found Error` when the Document class tries to load a source that has no closing head and body tags and contains invalid node in the head tag. For example,
```html
<html><head><meta name="foo" content="bar"><foo></foo><bar></bar>
```
With this fix, we will get a proper structured html like this,
```html
<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8" />
    <meta name="foo" content="bar" />
  </head>
  <body>
    <foo></foo><bar></bar>
  </body>
</html>
```

Patch:
diff --git a/src/Dom/Document.php b/src/Dom/Document.php
index aa21b3d57..26f94a3fc 100644
--- a/src/Dom/Document.php
+++ b/src/Dom/Document.php
@@ -566,7 +566,13 @@ private function normalizeDocumentStructure($content)
         } elseif (! preg_match(self::HTML_STRUCTURE_BODY_END_TAG, $content, $matches)) {
             // Only <body> missing.
             // @todo This is an expensive regex operation, look into further optimization.
-            $content = preg_replace(self::HTML_STRUCTURE_HEAD_TAG, '$0<body>', $content, 1);
+            $content = preg_replace(self::HTML_STRUCTURE_HEAD_TAG, '$0<body>', $content, 1, $count);
+
+            // Closing </head> tag is missing.
+            if (! $count) {
+                $content = $content . '</head><body>';
+            }
+
             $content .= '</body>';
         }
 


Test patch:
diff --git a/tests/Dom/DocumentTest.php b/tests/Dom/DocumentTest.php
index 316d02196..dc02277fa 100644
--- a/tests/Dom/DocumentTest.php
+++ b/tests/Dom/DocumentTest.php
@@ -11,6 +11,7 @@
 use AmpProject\Html\Tag;
 use AmpProject\Tests\MarkupComparison;
 use AmpProject\Tests\TestCase;
+use AmpProject\Tests\TestMarkup;
 use AmpProject\Validator\Spec\CssRuleset\AmpNoTransformed;
 use AmpProject\Validator\Spec\SpecRule;
 use DOMNode;
@@ -752,6 +753,17 @@ public function testInvalidHeadNodes()
         $this->assertEquals('meta', $dom->head->firstChild->tagName);
         $this->assertNull($dom->head->firstChild->nextSibling);
         $this->assertEquals(6, $dom->body->childNodes->length);
+
+        // Test with missing closing head tag.
+        $expected = TestMarkup::DOCTYPE . '<html>'
+            . '<head>' . TestMarkup::META_CHARSET . '<meta name="foo" content="bar"></head>'
+            . '<body><foo></foo><bar></bar></body></html>';
+
+        $html     = '<html><head><meta name="foo" content="bar"><foo></foo><bar></bar>';
+        $document = Document::fromHtml($html);
+        $output   = $document->saveHTML();
+
+        $this->assertEqualMarkup($expected, $output);
     }
 
     /**