search

anacronw / multer-s3

multer storage engine for amazon s3
MIT License
660 stars 190 forks source link

Add support for SVGs #75

Closed wgminer closed 6 years ago

wgminer commented 7 years ago

Closed my previous pull request and simply added the is-svg package as a fallback for when the file type is not found.

wgminer commented 7 years ago

Good point. Updated.

AnderbergE commented 6 years ago

Is there any plan to merge this request? :)

LinusU commented 6 years ago

~~Published as 2.9.0, sorry for the delay 🙈 ~~

Hmm, apparently I cannot publish releases, both 2.8.0 and 2.9.0 is not published to npm...

@badunk could you add me on npm? I'm linusu there...

AnderbergE commented 6 years ago

The proposed change does actually not do what it states in all conditions.

Problem: very large svg files are treated as not being svg. Reason: when the "firstChunk" doesn't contain the entire svg data, the end-tag, that is the </svg> does not exist in it. isSvg looks for that tag, but fails to find it and says that the file is not an svg.

I am not confident enough in node to propose a solution, but I believe the entire file needs to be read and sent as input to isSvg.

LinusU commented 6 years ago

Hmm, yeah, maybe something like this:

const regex = /^\s*(?:<\?xml[^>]*>\s*)?(?:<!doctype svg[^>]*\s*(?:\[?(?:\s*<![^>]*>\s*)*\]?)*[^>]*>\s*)?<svg[^>]*>/i;

This will look just for the opening tag

AnderbergE commented 6 years ago

That is literally the way I did it. Works for me so far 👍

LinusU commented 6 years ago

Do you want to submit a PR? ☺️

AnderbergE commented 6 years ago

Sure, I'll give it a go :)