`XmlSerializer` configurable amount of spaces when `$prettyPrint=true` #304

llaville · 2023-05-31T06:46:26Z

llaville
May 31, 2023

Hello,

Because I used 4 spaces indentation (for my XML documents) rather than default 2 spaces provided by the DOMDocument, I wanted a solution to configure the XMLSerializer.

As we cannot alter indendation via DOMDocument

I propose two alternatives :

the most simple and easy way (through a regex like suggested https://stackoverflow.com/questions/3325488/php-increase-indentation-of-domdocument-savexml)
Change DOMDocument by XMLWriter that is able to configure it (for example : https://github.com/phar-io/manifest/blob/2.0.3/src/ManifestSerializer.php#L43-L44)

I've already tested solution 1 and it works as expected :

Patching:
https://github.com/CycloneDX/cyclonedx-php-library/blob/v2.1.2/src/Core/Serialization/XmlSerializer.php#L78

        $xml = $document->saveXML();
        $xml = \preg_replace('/^[ ]+(?=<)/m','$0$0', $xml);

Of course adding an option argument on class constructor to make it configurable at runtime.

What do you think ?

jkowalleck · 2023-05-31T08:37:27Z

jkowalleck
May 31, 2023
Maintainer

did you consider configuring your project so that this one bom.xml file uses 2 spaces, regardless of the other configs?
your editor might even support EditorConfig which makes this possible with ease.

0 replies

jkowalleck · 2023-05-31T08:42:32Z

jkowalleck
May 31, 2023
Maintainer

Relevant API:

cyclonedx-php-library/src/Core/Serialization/BaseSerializer.php

Lines 105 to 115 in fa0ade0

    
               /** 
        
                * Serialize a {@see realNormalize() normalized} version of a {@see \CycloneDX\Core\Models\Bom}. 
        
                * 
        
                * @param TNormalizedBom $normalizedBom a version of the Bom that was normalized for serialization 
        
                * 
        
                * @throws Exception 
        
                * 
        
                * @SuppressWarnings(PHPMD.BooleanArgumentFlag) 
        
                */ 
        
               abstract protected function realSerialize(/* TNormalizedBom */ $normalizedBom, ?bool $prettyPrint): string; 
        
               // no typehint for `$normalizedBom` parameter, as it is not actually `mixed` but a templated type.

0 replies

jkowalleck · 2023-05-31T08:57:57Z

jkowalleck
May 31, 2023
Maintainer

i think the regex solution is potentially dangerous. it could add unwanted spaces in here:

<!--- [...] -->
<description>
  <![CDATA[the text in here is free to 
  write whatever they want, without escaping...
including XML special chars like the following
  <-- this line is possible and starts with an `<`

Depending on the definition of `description` in an XSD adding spaces here can change the content.
`xs:normalizedString` VS `xs:string` - see https://www.w3schools.com/XML/schema_dtypes_string.asp

  ]]>
</description>
<!--- [...] -->

I did not use XMLWriter when I implemented XML serialization, because I wanted typed return values for the normalizer. Therefore, I did not want to create a wrapper-Class just to represent the capabilities of an XML element, which already existed in PHP as a class, and can be serialized via libXML already.

I actually considered having a configurable amount of spaces when serializing.
The goal was just not worth the effort and trade-offs.
Furthermore, the amount of spaces does not change the quality of the document, so I skipped this feature.
I thought: if anybody wanted an alternative amount of spaces, then they could use external JSON/XML document reformatters that are feature-complete and work perfect in their domain.

0 replies

llaville · 2023-05-31T14:23:50Z

llaville
May 31, 2023
Author

I propose to add (as last alternative) a tidy optional process.

Here are an example of my implementation :

<?php

namespace Bartlett\Manifests\Helper;

use CycloneDX\Core\Serialization\XmlSerializer;
use CycloneDX\Core\Serialization\DOM\NormalizerFactory;

class ManifestSerializer extends XmlSerializer
{
    protected readonly array $tidyConfig;

    public function __construct(
        NormalizerFactory $normalizerFactory,
        private string $xmlVersion = '1.0',
        private string $xmlEncoding = 'UTF-8',
        protected bool $withTidyRepair = true
    ) {
        parent::__construct($normalizerFactory, $xmlVersion, $xmlEncoding);

        $this->setTidyConfig(
            array(
                'input-xml' => true,
                'indent-attributes' => false,
                'wrap' => false,
                'indent-cdata' => true,
                'indent' => true,
                'indent-spaces' => 4
            )
        );
    }

    public function setTidyConfig(array $config)
    {
        $this->tidyConfig = $config;
    }

    protected function realSerialize(/* TNormalizedBom */ $normalizedBom, ?bool $prettyPrint): string
    {
        $document = new \DOMDocument($this->xmlVersion, $this->xmlEncoding);
        $document->appendChild(
            $document->importNode(
                $normalizedBom,
                true
            )
        );

        if (null !== $prettyPrint) {
            $document->formatOutput = $prettyPrint;
        }

        // option LIBXML_NOEMPTYTAG might lead to errors in consumers, do not use it.
        $xml = $document->saveXML();
        \assert(false !== $xml);


        if (!$this->withTidyRepair) {
            return $xml;
        }

        $clean = \tidy::repairString(
            $xml,
            $this->tidyConfig
        );

        if (\is_string($clean)) {
            return $clean;
        }

        // fallback to original version
        return $xml;
    }
}

And (example) results without tidy repair :

<?xml version="1.0" encoding="UTF-8"?>
<bom xmlns="http://cyclonedx.org/schema/bom/1.4" version="1" serialNumber="urn:uuid:4ee86da4-a5ef-4654-be46-0481c8f9080d">
  <metadata>
    <timestamp><![CDATA[2023-05-31T14:22:23Z]]></timestamp>
    <tools>
      <tool>
        <vendor><![CDATA[bartlett]]></vendor>
        <name><![CDATA[manifests]]></name>
        <version><![CDATA[dev-master]]></version>
      </tool>
    </tools>
  </metadata>
  <components>
    <component type="library" bom-ref="pkg:composer/clue/graph-composer@v1.1.0">
      <group><![CDATA[clue]]></group>
      <name><![CDATA[graph-composer]]></name>
      <version><![CDATA[v1.1.0]]></version>
      <purl><![CDATA[pkg:composer/clue/graph-composer@v1.1.0]]></purl>
      <properties>
        <property name="cdx:composer:package:sourceReference"><![CDATA[eff70fe2af7704b15cf675fcad663abe42034153]]></property>
        <property name="cdx:composer:package:distReference"><![CDATA[eff70fe2af7704b15cf675fcad663abe42034153]]></property>
        <property name="cdx:composer:package:isDevRequirement"><![CDATA[false]]></property>
      </properties>
    </component>
  </components>
  <dependencies>
    <dependency ref="pkg:composer/clue/graph-composer@v1.1.0"/>
  </dependencies>
</bom>

And (example) results with tidy repair :

<?xml version="1.0" encoding="utf-8"?>
<bom xmlns="http://cyclonedx.org/schema/bom/1.4" version="1" serialNumber="urn:uuid:39b92711-c69b-448f-9d51-f68135735507">
    <metadata>
        <timestamp>
            <![CDATA[2023-05-31T14:15:47Z]]>
        </timestamp>
        <tools>
            <tool>
                <vendor>
                    <![CDATA[bartlett]]>
                </vendor>
                <name>
                    <![CDATA[manifests]]>
                </name>
                <version>
                    <![CDATA[dev-master]]>
                </version>
            </tool>
        </tools>
    </metadata>
    <components>
        <component type="library" bom-ref="pkg:composer/clue/graph-composer@v1.1.0">
            <group>
                <![CDATA[clue]]>
            </group>
            <name>
                <![CDATA[graph-composer]]>
            </name>
            <version>
                <![CDATA[v1.1.0]]>
            </version>
            <purl>
                <![CDATA[pkg:composer/clue/graph-composer@v1.1.0]]>
            </purl>
            <properties>
                <property name="cdx:composer:package:sourceReference">
                    <![CDATA[eff70fe2af7704b15cf675fcad663abe42034153]]>
                </property>
                <property name="cdx:composer:package:distReference">
                    <![CDATA[eff70fe2af7704b15cf675fcad663abe42034153]]>
                </property>
                <property name="cdx:composer:package:isDevRequirement">
                    <![CDATA[false]]>
                </property>
            </properties>
        </component>
    </components>
    <dependencies>
        <dependency ref="pkg:composer/clue/graph-composer@v1.1.0" />
    </dependencies>
</bom>

3 replies

jkowalleck May 31, 2023
Maintainer

reminder: tidy is not part of PHP core.
Introducing this as a dependency would be a breaking change.

jkowalleck May 31, 2023
Maintainer

i think you should improve your code, so that you do not face all the issues you mentioned.
and you would not have all that copy-paste from this code, which you would have needed to attribute(licensing/legal).

here you are: a patched version of your implementation, free of copy-paste and probably without the inheritance-issues you mentioned.

<?php

namespace Bartlett\Manifests\Helper;

use CycloneDX\Core\Serialization\XmlSerializer;
use CycloneDX\Core\Serialization\DOM\NormalizerFactory;

class ManifestSerializer extends XmlSerializer
{
    protected readonly array $tidyConfig;

    public function __construct(
        NormalizerFactory $normalizerFactory,
        string $xmlVersion = '1.0',
        string $xmlEncoding = 'UTF-8',
        protected bool $withTidyRepair = true
    ) {
        parent::__construct($normalizerFactory, $xmlVersion, $xmlEncoding);

        $this->setTidyConfig(
            array(
                'input-xml' => true,
                'indent-attributes' => false,
                'wrap' => false,
                'indent-cdata' => true,
                'indent' => true,
                'indent-spaces' => 4
            )
        );
    }

    public function setTidyConfig(array $config): self
    {
        $this->tidyConfig = $config;
        return $this;
    }

    protected function realSerialize(/* DOMElement */ $normalizedBom, ?bool $prettyPrint): string
    {
        $xml = parent::realSerialize($normalizedBom, $prettyPrint);

        $clean = \tidy::repairString(
            $xml,
            $this->tidyConfig
        );
        return \is_string($clean)
          ? $clean
          : $xml;
    }
}

llaville Jun 1, 2023
Author

reminder: tidy is not part of PHP core. Introducing this as a dependency would be a breaking change.

Unless you consider it as optional at runtime.

    protected function realSerialize(/* DOMElement */ $normalizedBom, ?bool $prettyPrint): string
    {
        $xml = parent::realSerialize($normalizedBom, $prettyPrint);

        if (!\extension_loaded('tidy')) {
            return $xml;
        }

        $clean = \tidy::repairString(
            $xml,
            $this->tidyConfig
        );
        return \is_string($clean)
          ? $clean
          : $xml;
    }

llaville · 2023-05-31T14:27:30Z

llaville
May 31, 2023
Author

BTW, when class is not marked as final, I'd like to have protected visibility to properties. That will avoid inheritance issue !

3 replies

jkowalleck May 31, 2023
Maintainer

what do you mean? can you give a runnable example? like this one: https://3v4l.org/tea6D

Why would one give access to private properties? they are private for a reason, right?
Making all classes final? well, how would i mock/stub them then?

llaville Jun 1, 2023
Author

what do you mean? can you give a runnable example? like this one: https://3v4l.org/tea6D

Consider if we want to implement an XmlWriter implementation and reuse code base of your XMLSerializer. Something like this (partial code) :

class ManifestXmlSerializer extends XmlSerializer
{
    public function __construct(
        NormalizerFactory $normalizerFactory,
        string $xmlVersion = '1.0',
        string $xmlEncoding = 'UTF-8',
        protected string $indentString = '    '
    ) {
        parent::__construct($normalizerFactory, $xmlVersion, $xmlEncoding);
    }

    protected function realSerialize(/* DOMElement */ $normalizedBom, ?bool $prettyPrint): string
    {
        $xmlWriter = new \XMLWriter();
        $xmlWriter->openMemory();
        $xmlWriter->setIndent(true);
        $xmlWriter->setIndentString($this->indentString);
        $xmlWriter->startDocument($this->xmlVersion, $this->xmlEncoding);

        $xmlWriter->startElement($normalizedBom->localName);

        $xmlWriter->endElement();
        $xmlWriter->endDocument();

        return $xmlWriter->outputMemory();
    }
}

xmlVersion and xmlEncoding have private visibility and cannot be reused with $xmlWriter->startDocument($this->xmlVersion, $this->xmlEncoding);

We have then to change our own visibility at class construction to run it without warnings and false results :

    public function __construct(
        NormalizerFactory $normalizerFactory,
        protected string $xmlVersion = '1.0',
        protected string $xmlEncoding = 'UTF-8',
        protected string $indentString = '    '
    ) {
        parent::__construct($normalizerFactory, $xmlVersion, $xmlEncoding);
    }

Run script :

<?php

use CycloneDX\Core\Models\Bom;
use CycloneDX\Core\Serialization\DOM\NormalizerFactory as DomNormalizerFactory;
use CycloneDX\Core\Spec\SpecFactory;
use CycloneDX\Core\Spec\Version;

$spec = SpecFactory::makeForVersion(Version::v1dot4);

$normalizer = new DomNormalizerFactory($spec);

$serializer = new \Bartlett\Manifests\Helper\ManifestXmlSerializer($normalizer);

$bom = new Bom();

$xml = $serializer->serialize($bom, true);
echo $xml;

jkowalleck Jun 1, 2023
Maintainer

understood. caused #305
will have this implemented this week. feel free to pulrequest a solution, if you need it earlier.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`XmlSerializer` configurable amount of spaces when `$prettyPrint=true` #304

{{title}}

Replies: 5 comments 6 replies

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

Select a reply

XmlSerializer configurable amount of spaces when $prettyPrint=true #304

llaville May 31, 2023

Replies: 5 comments · 6 replies

jkowalleck May 31, 2023 Maintainer

jkowalleck May 31, 2023 Maintainer

jkowalleck May 31, 2023 Maintainer

llaville May 31, 2023 Author

jkowalleck May 31, 2023 Maintainer

jkowalleck May 31, 2023 Maintainer

llaville Jun 1, 2023 Author

llaville May 31, 2023 Author

jkowalleck May 31, 2023 Maintainer

llaville Jun 1, 2023 Author

jkowalleck Jun 1, 2023 Maintainer

`XmlSerializer` configurable amount of spaces when `$prettyPrint=true` #304

llaville
May 31, 2023

Replies: 5 comments 6 replies

jkowalleck
May 31, 2023
Maintainer

jkowalleck
May 31, 2023
Maintainer

jkowalleck
May 31, 2023
Maintainer

llaville
May 31, 2023
Author

jkowalleck May 31, 2023
Maintainer

jkowalleck May 31, 2023
Maintainer

llaville Jun 1, 2023
Author

llaville
May 31, 2023
Author

jkowalleck May 31, 2023
Maintainer

llaville Jun 1, 2023
Author

jkowalleck Jun 1, 2023
Maintainer