Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Introduce auto-discovery of sanitizer settings #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ohader
Copy link
Contributor

@ohader ohader commented Aug 26, 2021

Based on given SVG content, auto-discovery allows to resolve settings minifyXML and removeXMLTag settings - without explicitly declaring them.

$sanitizer = new Sanitizer();
$sanitizer->setAutoDiscover(true);
$sanitizer->sanitize('<?xml version="1.0" encoding="UTF-8"?> <svg><text>test</text></svg>);
  • since <?xml version="1.0" encoding="UTF-8"?> is given → removeXMLTag = false
  • since <svg><text>test</text></svg> is in a single line → minifyXML = true

Related: #51

$svgContent = (string) $svgContent;
// in case content does not start with XML prolog, keep it that way
if (!is_bool($this->removeXMLTag)
&& !preg_match('#^\s*<\?xml[^>]+\?>#im', $svgContent)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion; regex via / not #

Copy link
Contributor Author

@ohader ohader Aug 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would not really matter here, but I don't have a strong opinion here... so, fine to change it 😉

Based on given SVG content, auto-discovery allows to resolve
settings `minifyXML` and `removeXMLTag` settings - without
explicitly declaring them.

```php
$sanitizer = new Sanitizer();
$sanitizer->setAutoDiscover(true);
$sanitizer->sanitize($svgContent);
```
@ohader ohader force-pushed the feature-auto-discover branch from cf82abd to 73511dd Compare August 26, 2021 16:27
@darylldoyle
Copy link
Owner

I like this idea but am slightly concerned with setting minifyXML and removeXMLTag to null defaults.

My concern is that it's a potential BC break and could cause users to overlook issues. A workaround could be to do something like the following to make sure the defaults are set:

protected function autoDiscover($svgContent)
{
    if (!$this->autoDiscover) {
        // Set default if it's not already been set.
        if (!is_bool($this->removeXMLTag)) {
            $this->removeXMLTag = false;
        }
        // Set default if it's not already been set.
        if (!is_bool($this->minifyXML)) {
            $this->minifyXML = false;
        }

        return;
    }
    $svgContent = (string) $svgContent;
    // in case content does not start with XML prolog, keep it that way
    if (!is_bool($this->removeXMLTag)
        && !preg_match('/^\s*<\?xml[^>]+\?>/im', $svgContent)
    ) {
        $this->removeXMLTag = true;
    }
    // in case `<svg>...</svg>` does not have any vertical spaces, keep it that way
    $start = strpos($svgContent, '<svg');
    $end = strrpos($svgContent, '</svg>');
    $outerHtml = $end > $start ? substr($svgContent, $start, $end - $start) : null;
    if (!is_bool($this->minifyXML)
        && $outerHtml !== null
        && count(preg_split('/\v/m', $outerHtml)) === 1
    ) {
        $this->minifyXML = true;
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants