search

ryancramerdesign / ProcessWire

Our repository has moved to https://github.com/processwire – please head there for the latest version.
https://processwire.com
Other
727 stars 201 forks source link

SVG without width and height attributes is invalid and won't upload #802

Closed adrianbj closed 9 years ago

adrianbj commented 9 years ago

Hey Ryan,

Haven't thought through how best to deal with this yet, but you can read about it here: https://processwire.com/talk/topic/8398-svg-images/

In this case the svg tag of the image looks like:

<svg version="1.1" id="Livello_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
     viewBox="0 0 1200 500" enable-background="new 0 0 1200 500" xml:space="preserve">

Manually adding in the width and height makes everything work, but perhaps you could extract them from the viewBox if they don't exist?

ryancramerdesign commented 9 years ago

Adrian the SVG support is your baby. :) I'll divert to whatever you think is best here. Is viewBox a universal attribute, always present, in the same format, etc.?

adrianbj commented 9 years ago

Sorry for the delay on this Ryan. So basically I fixed the issue of no width/height tags by setting them to 100% for both. It turns out Illustrator CC has a new option for responsive SVGs when saving - all this does is leave off the width/height attributes. I though about the viewbox, but this is not really appropriate, and also not required by the SVG spec. The 100% option allows for resizing of the admin thumb if that setting is checked and it also allows for resizing the image when embedded into an RTE - once resizing is initiated, pixel values take over.

I also added an SVG sanitizer method. I am using a modified version of this: https://github.com/alister-/SVG-Sanitizer

It was broken in it's current form, which wasn't encouraging, but it didn't take much to get it working. I couldn't find any other options out there, and from some reading around, it sounds like a pretty important thing to take care of, and also using a whitelist approach seems to be recommended approach (which this script uses). If you want to do some reading, here are two useful items: http://www.ei.rub.de/media/hgi/veroeffentlichungen/2011/10/19/svgSecurity-ccs11.pdf https://www.hackinparis.com/slides/hip2k11/09-TheForbiddenImage.pdf

You'll notice that the whitelist is editable for each file/image field. All you have to do is add "svg" to the list of allowed file extensions, unfocus that field, and a new one "SVG Santitizer Whitelist" will appear, populated with the default list. As I mentioned in my PM to you earlier today, I think there are times when it is useful to be able to edit this on a field by field basis (to disallow embedded raster images, certain raster effects, etc).

Anyway, please let me know what you think of these changes and if you'd like me to make any specific changes, or of course feel free to change things yourself :)

adrianbj commented 9 years ago

Thanks Ryan - looks like the no dimensions issue stuff is all sorted along with the sanitizing!