Add option to enable large SVGs

[kirtangajjar]

Description of the Change

PR adds support of large SVG files to the plugin. It also adds a setting for it to let the users toggle this behaviour.

How to test the Change

Verify GH action run on this PR and it works properly.

Changelog Entry

Added - New setting that when turned on, allows large SVG files (roughly 10MB or greater) to be uploaded and sanitized properly

Closes #178

Credits

Props @kirtangajjar, @faisal-alvi, @dkotter

Checklist:

• I agree to follow this project's Code of Conduct.
• I have updated the documentation accordingly.
• I have added tests to cover my change.
• All new and existing tests pass.

[github-actions]

@kirtangajjar thanks for the PR! Could you please fill out the PR template with description, changelog, and credits information so that we can properly review and merge this?

[faisal-alvi]

@kirtangajjar Thanks for the PR. I tested and found the setting works incorrectly.

When "Allow large SVG files" is disabled, large files are actually uploading! I tested with this Phisical_Map_of_the_World_-_January_2015.svg (13.5 MB)

And, when "Allow large SVG files" is enabled, it throws an error and the upload fails with a note!

Can you please check what is causing this? you can use the same SVG I tested with

[kirtangajjar]

@faisal-alvi Thanks for the feedback appreciate it. Even I found that many time we can upload larger files without that flag, but for some files we just are not able to upload them without that flag. i.e. This portrait image won't work without that flag. I found this image in this issue discussion. Many big files would work without the flag. I wasn't able to find any image which causes issue with the toggle on so I kept it on.

https://github.com/10up/safe-svg/assets/8456197/6805e259-26b5-45d1-af71-cbdc7135740c

[faisal-alvi]

I wasn't able to find any image which causes issue with the toggle on so I kept it on.

@kirtangajjar have tried with the https://commons.wikimedia.org/wiki/Category:Large_SVG_files#/media/File:Political_map_of_the_World_(January_2015).svg file? I can reproduce the issue with this SVG file. The issue (as mentioned above) is that I can NOT upload the SVG when "Allow large SVG files" is enabled !? but I can, when it is disabled! I'm ready to connect 1:1 to test and discuss this in detail.

[kirtangajjar]

@faisal-alvi That's really weird. I downloaded the exact same Image and it was working fine for me. Let's connect on it on a week where I have some OSP time.

[faisal-alvi]

Sure @kirtangajjar! Ping me once you are available for a call.

[faisal-alvi]

@kirtangajjar we discussed over a call that you'll add a custom check to prevent SVG files larger than 10MB. The setAllowHugeFiles() function produces inconsistent results, sometimes allowing files over 10MB when it shouldn't and vice versa.

[faisal-alvi]

@kirtangajjar is this ready for review?

[dkotter]

I've gone ahead and made some updates here to clean up the code but this looks good to me. Also investigated the issues reported on this PR.

When "Allow large SVG files" is disabled, large files are actually uploading

This is fine, as it's not us determining if those are allowed, it's the loadXML method that does this. Seems the limit it places isn't exactly 10MB but is roughly around there, so some SVGs slightly larger than that are allowed even with this setting off.

And, when "Allow large SVG files" is enabled, it throws an error and the upload fails with a note

So the problem here is the setAllowHugeFiles method we use was introduced in v0.18.0 of enshrined/svg-sanitize, but we didn't update to that version until September, which is after this PR was created and tested. If you test this PR on v0.16.0 of enshrined/svg-sanitize, you will get this same failure but it isn't an issue with the file size, it's because we're trying to use the setAllowHugeFiles method which doesn't exist. My guess is when Kirtan was building this, he updated that version locally but didn't commit that, so when Faisal tested, it was using the old version.

I've merged develop into this PR which includes that update and now in testing that file, things work as expected.

we discussed over a call that you'll add a custom check to prevent SVG files larger than 10MB. The setAllowHugeFiles() function produces inconsistent results, sometimes allowing files over 10MB when it shouldn't and vice versa.

I don't think this extra check is needed. The only thing setAllowHugeFiles does is set the LIBXML_PARSEHUGE flag in the loadXML call. Any inconsistencies of what constitutes a large file is then up to PHP to decide, which I think is fine. And as mentioned above, the issue with a file being rejected had nothing to do with setAllowHugeFiles not working, it's because that method didn't exist in the version we were testing on.