fix XML encoding for anyURI

related to #35 Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
CycloneDX · Dec 3, 2021 · 62e615e · 62e615e
1 parent e94ce9e
commit 62e615e
Show file tree

Hide file tree

Showing 12 changed files with 293 additions and 51 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
 
 ## unreleased
 
+* Fixed
+  * XML serializer & DOM normalizer no longer generate invalid `XML::anyURI`. (via [#34])
+
+[#34]: https://github.com/CycloneDX/cyclonedx-php-library/pull/34
+
 ## 1.3.0 - 2021-12-01
 
 * Changed

diff --git a/src/Core/Helpers/XmlTrait.php b/src/Core/Helpers/XmlTrait.php
@@ -0,0 +1,81 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of CycloneDX PHP Library.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright (c) Steve Springett. All Rights Reserved.
+ */
+
+namespace CycloneDX\Core\Helpers;
+
+use DOMDocument;
+
+/**
+ * @author jkowalleck
+ *
+ * @internal
+ */
+trait XmlTrait
+{
+    /**
+     * Make a string valid to XML::anyURI spec - best-effort.
+     *
+     * Complete and failsafe implementation is pretty context-dependent.
+     * Best-effort solution: replacement & drop every URI that is not well-formed already.
+     *
+     * @see http://www.w3.org/TR/xmlschema-2/#anyURI
+     * @see http://www.datypic.com/sc/xsd/t-xsd_anyURI.html
+     *
+     * @return string|null string on success; null if encoding failed, or input was null
+     */
+    private function encodeAnyUriBE(?string $uri): ?string
+    {
+        if (null === $uri) {
+            return null;
+        }
+
+        $uri = str_replace(
+            [' ', '[', ']', '<', '>', '{', '}'],
+            ['%20', '%5B', '%5D', '%3C', '%3E', '%7B', '%7D'],
+            $uri
+        );
+
+        return $this->filterAnyUri($uri)
+            ? $uri
+            : null; // @codeCoverageIgnore
+    }
+
+    /**
+     * @SuppressWarnings(PHPMD.ErrorControlOperator) as there is no easy way to control libxml warning output
+     */
+    private function filterAnyUri(string $uri): bool
+    {
+        $doc = new DOMDocument();
+        $doc->appendChild($doc->createElement('t'))
+            ->appendChild($doc->createCDATASection($uri));
+
+        return @$doc->schemaValidateSource(
+            <<<'XML'
+                <?xml version="1.0" encoding="UTF-8"?>
+                <xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema">
+                    <xs:element name="t" type="xs:anyURI" />
+                </xs:schema>
+                XML
+        );
+    }
+}
diff --git a/src/Core/Serialize/DOM/Normalizers/ComponentNormalizer.php b/src/Core/Serialize/DOM/Normalizers/ComponentNormalizer.php
@@ -25,6 +25,7 @@
 
 use CycloneDX\Core\Factories\LicenseFactory;
 use CycloneDX\Core\Helpers\SimpleDomTrait;
+use CycloneDX\Core\Helpers\XmlTrait;
 use CycloneDX\Core\Models\Component;
 use CycloneDX\Core\Models\License\LicenseExpression;
 use CycloneDX\Core\Repositories\DisjunctiveLicenseRepository;
@@ -41,6 +42,7 @@
 class ComponentNormalizer extends AbstractNormalizer
 {
     use SimpleDomTrait;
+    use XmlTrait;
 
     /**
      * @throws DomainException
@@ -161,7 +163,11 @@ private function normalizePurl(?PackageUrl $purl): ?DOMElement
     {
         return null === $purl
             ? null
-            : $this->simpleDomSafeTextElement($this->getNormalizerFactory()->getDocument(), 'purl', (string) $purl);
+            : $this->simpleDomSafeTextElement(
+                $this->getNormalizerFactory()->getDocument(),
+                'purl',
+                $this->encodeAnyUriBE((string) $purl)
+            );
     }
 
     private function normalizeExternalReferences(?ExternalReferenceRepository $externalReferenceRepository): ?DOMElement

diff --git a/src/Core/Serialize/DOM/Normalizers/DisjunctiveLicenseNormalizer.php b/src/Core/Serialize/DOM/Normalizers/DisjunctiveLicenseNormalizer.php
@@ -24,6 +24,7 @@
 namespace CycloneDX\Core\Serialize\DOM\Normalizers;
 
 use CycloneDX\Core\Helpers\SimpleDomTrait;
+use CycloneDX\Core\Helpers\XmlTrait;
 use CycloneDX\Core\Models\License\AbstractDisjunctiveLicense;
 use CycloneDX\Core\Models\License\DisjunctiveLicenseWithId;
 use CycloneDX\Core\Models\License\DisjunctiveLicenseWithName;
@@ -37,6 +38,7 @@
 class DisjunctiveLicenseNormalizer extends AbstractNormalizer
 {
     use SimpleDomTrait;
+    use XmlTrait;
 
     /**
      * @psalm-assert DisjunctiveLicenseWithId|DisjunctiveLicenseWithName $license
@@ -62,7 +64,7 @@ public function normalize(AbstractDisjunctiveLicense $license): DOMElement
             [
                 $this->simpleDomSafeTextElement($document, 'id', $id),
                 $this->simpleDomSafeTextElement($document, 'name', $name),
-                $this->simpleDomSafeTextElement($document, 'url', $license->getUrl()),
+                $this->simpleDomSafeTextElement($document, 'url', $this->encodeAnyUriBE($license->getUrl())),
             ]
         );
     }

diff --git a/src/Core/Serialize/DOM/Normalizers/ExternalReferenceNormalizer.php b/src/Core/Serialize/DOM/Normalizers/ExternalReferenceNormalizer.php
@@ -24,25 +24,38 @@
 namespace CycloneDX\Core\Serialize\DOM\Normalizers;
 
 use CycloneDX\Core\Helpers\SimpleDomTrait;
+use CycloneDX\Core\Helpers\XmlTrait;
 use CycloneDX\Core\Models\ExternalReference;
 use CycloneDX\Core\Repositories\HashRepository;
 use CycloneDX\Core\Serialize\DOM\AbstractNormalizer;
+use DomainException;
 use DOMElement;
+use UnexpectedValueException;
 
 /**
  * @author jkowalleck
  */
 class ExternalReferenceNormalizer extends AbstractNormalizer
 {
     use SimpleDomTrait;
+    use XmlTrait;
 
     /**
-     * @throws \DomainException
+     * @throws DomainException
+     * @throws UnexpectedValueException when url was unable to convert to XML::anyURI
      */
     public function normalize(ExternalReference $externalReference): DOMElement
     {
         // could throw DomainException if the type was not supported
 
+        $refURI = $externalReference->getUrl();
+        $anyURI = $this->encodeAnyUriBE($refURI);
+        if (null === $anyURI) {
+            // @codeCoverageIgnoreStart
+            throw new UnexpectedValueException("unable to make anyURI from: $refURI");
+            // @codeCoverageIgnoreEnd
+        }
+
         $doc = $this->getNormalizerFactory()->getDocument();
 
         return $this->simpleDomAppendChildren(
@@ -53,7 +66,7 @@ public function normalize(ExternalReference $externalReference): DOMElement
                 ]
             ),
             [
-                $this->simpleDomSafeTextElement($doc, 'url', $externalReference->getUrl()),
+                $this->simpleDomSafeTextElement($doc, 'url', $anyURI),
                 $this->simpleDomSafeTextElement($doc, 'comment', $externalReference->getComment()),
                 $this->normalizeHashes($externalReference->getHashRepository()),
             ]

diff --git a/src/Core/Serialize/DOM/Normalizers/ExternalReferenceRepositoryNormalizer.php b/src/Core/Serialize/DOM/Normalizers/ExternalReferenceRepositoryNormalizer.php
@@ -46,6 +46,8 @@ public function normalize(ExternalReferenceRepository $repo): array
                 $externalReferences[] = $normalizer->normalize($externalReference);
             } catch (\DomainException $exception) {
                 continue;
+            } catch (\UnexpectedValueException $exception) {
+                continue;
             }
         }
 

diff --git a/src/Core/Validation/Errors/XmlValidationError.php b/src/Core/Validation/Errors/XmlValidationError.php
@@ -36,7 +36,7 @@ class XmlValidationError extends ValidationError
      *
      * @var object|null
      */
-    private $error;
+    private $debugError;
 
     /**
      * @internal
@@ -46,8 +46,20 @@ class XmlValidationError extends ValidationError
     public static function fromLibXMLError(LibXMLError $error): self
     {
         $i = new static($error->message);
-        $i->error = $error;
+        $i->debugError = $error;
 
         return $i;
     }
+
+    /**
+     * Accessor for debug purposes.
+     *
+     * @internal as this method is not kept backwards-compatible
+     * @codeCoverageIgnore
+     * @SuppressWarnings(PHPMD)
+     */
+    final public function debug_getError(): ?object
+    {
+        return $this->debugError;
+    }
 }
diff --git a/tests/Core/Serialize/DOM/Normalizers/DisjunctiveLicenseNormalizerTest.php b/tests/Core/Serialize/DOM/Normalizers/DisjunctiveLicenseNormalizerTest.php
@@ -75,13 +75,15 @@ public function dpNormalize(): Generator
             '<license><name>bar</name></license>',
         ];
 
-        yield 'optional url' => [
-            $this->createConfiguredMock(DisjunctiveLicenseWithName::class, [
-                'getName' => 'foo',
-                'getUrl' => 'http://foo.bar',
+        foreach (\CycloneDX\Tests\_data\XmlAnyUriData::dpEncodeAnyUri() as $name => [$rawUrl, $encodedUrl]) {
+            yield "optional url: $name" => [
+                $this->createConfiguredMock(DisjunctiveLicenseWithName::class, [
+                    'getName' => 'foo',
+                    'getUrl' => $rawUrl,
                 ]),
-            '<license><name>foo</name><url>http://foo.bar</url></license>',
-        ];
+                '<license><name>foo</name><url>'.htmlspecialchars($encodedUrl).'</url></license>',
+            ];
+        }
     }
 
     public function testNormalizeThrowsOnUnknown(): void

diff --git a/tests/Core/Serialize/DOM/Normalizers/ExternalReferenceNormalizerTest.php b/tests/Core/Serialize/DOM/Normalizers/ExternalReferenceNormalizerTest.php
@@ -192,4 +192,28 @@ public function testNormalizeHashesOmitIfNotSupported(): void
     }
 
     // endregion normalize hashes
+
+    /**
+     * @dataProvider \CycloneDX\Tests\_data\XmlAnyUriData::dpEncodeAnyUri()
+     */
+    public function testNormalizeUrlEncodeAnyUri(string $rawUrl, string $encodedUrl): void
+    {
+        $normalizerFactory = $this->createConfiguredMock(NormalizerFactory::class, [
+            'getDocument' => new \DOMDocument(),
+        ]);
+        $normalizer = new Normalizers\ExternalReferenceNormalizer($normalizerFactory);
+        $extRef = $this->createConfiguredMock(ExternalReference::class, [
+            'getUrl' => $rawUrl,
+            'getType' => 'someType',
+            'getComment' => null,
+            'getHashRepository' => null,
+        ]);
+
+        $actual = $normalizer->normalize($extRef);
+
+        self::assertStringEqualsDomNode(
+            '<reference type="someType"><url>'.htmlspecialchars($encodedUrl).'</url></reference>',
+            $actual
+        );
+    }
 }
diff --git a/tests/Core/Serialize/DOM/Normalizers/ExternalReferenceRepositoryNormalizerTest.php b/tests/Core/Serialize/DOM/Normalizers/ExternalReferenceRepositoryNormalizerTest.php
@@ -84,7 +84,11 @@ public function testNormalize(): void
         self::assertSame([$FakeExtRef], $actual);
     }
 
-    public function testNormalizeSkipsOnThrow(): void
+    /**
+     * @dataProvider dpNormalizeSkipsOnThrow
+     * @psalm-param class-string<\Exception> $exceptionClass
+     */
+    public function testNormalizeSkipsOnThrow(string $exceptionClass): void
     {
         $spec = $this->createStub(SpecInterface::class);
         $externalReferenceNormalizer = $this->createMock(Normalizers\ExternalReferenceNormalizer::class);
@@ -102,10 +106,16 @@ public function testNormalizeSkipsOnThrow(): void
 
         $externalReferenceNormalizer->expects(self::exactly(2))->method('normalize')
             ->withConsecutive([$extRef1], [$extRef2])
-            ->willThrowException(new \DomainException());
+            ->willThrowException(new $exceptionClass());
 
         $actual = $normalizer->normalize($tools);
 
         self::assertSame([], $actual);
     }
+
+    public function dpNormalizeSkipsOnThrow(): \Generator
+    {
+        yield 'DomainException' => [\DomainException::class];
+        yield 'UnexpectedValueException' => [\UnexpectedValueException::class];
+    }
 }