darylldoyle / svg-sanitizer

A PHP SVG/XML Sanitizer
GNU General Public License v2.0
465 stars 68 forks source link

XML header lines removed #11

Closed angrybrad closed 7 years ago

angrybrad commented 7 years ago

If you have an SVG files that begins with <?xml version="1.0" encoding="utf-8"?>, then Sanitizer will remove that when saving the clean XML elements back out to the XML file.

It happens in this line:

$clean = $this->xmlDocument->saveXML($this->xmlDocument->documentElement, LIBXML_NOEMPTYTAG);

The reason is that all of the XML header info (verison, encoding, etc.) is saved on $this->xmlDocument. Changing that line to:

$clean = $this->xmlDocument->saveXML($this->xmlDocument, LIBXML_NOEMPTYTAG);

Makes it behave as you'd expect. Just making sure that wasn't a conscious design decision before I submit a pull request.

darylldoyle commented 7 years ago

Hey @takobell,

Unless I'm overlooking something silly because it's late then I see no reason we can't enable the XML declaration to be returned.

I think originally because I was basing the library on DOMPurify I copied the way their library works. As theirs is all inline SVG, the XML declaration isn't required and is therefore stripped but I see how it could be useful in this lib.

Test data will need updating to take this change into account, are you happy to do that? If not then feel free to make a PR for the change and I'll update the tests :)

angrybrad commented 7 years ago

PR: https://github.com/darylldoyle/svg-sanitizer/pull/12

Tried to get phpunit up and running to update the tests, but was getting tons of random errors.

darylldoyle commented 7 years ago

Thanks @takobell, tests passed fine once updated and it's now been merged into 0.6.0 :)

angrybrad commented 7 years ago

Thanks, @darylldoyle!