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

SVGs have a 10mb size Limit #178

Open
1 task done
darylldoyle opened this issue Mar 19, 2024 · 2 comments · May be fixed by #201
Open
1 task done

SVGs have a 10mb size Limit #178

darylldoyle opened this issue Mar 19, 2024 · 2 comments · May be fixed by #201
Assignees
Labels
help wanted Extra attention is needed type:enhancement New feature or request.
Milestone

Comments

@darylldoyle
Copy link
Collaborator

darylldoyle commented Mar 19, 2024

Describe the bug

We received a report in https://wordpress.org/support/topic/file-couldnt-be-sanitized-error/ stating that certain files couldn't be uploaded. After investigating the issue, we discovered that files larger than 10mb were being rejected due to the DOMDocument limit. We found a solution in version 0.18.0 of enshrined/svg-sanitize that allows us to bypass this limit using $this->sanitizer->setAllowHugeFiles( true );.

However, this solution disables the max recursion limits within the XML parser. As it is unlikely that users will regularly upload SVG files larger than 10mb, I suggest adding this option to the settings instead. When enabled, it sets setAllowHugeFiles() to true, allowing users who need to upload large files to do so without affecting everyone else.

Steps to Reproduce

  1. Install plugin.
  2. Try to upload an SVG that's larger than 10mb.
  3. See issue.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@darylldoyle darylldoyle added the type:bug Something isn't working. label Mar 19, 2024
@jeffpaul jeffpaul moved this from Incoming to To Do in Open Source Practice Mar 19, 2024
@jeffpaul jeffpaul added this to the 2.3.0 milestone Mar 19, 2024
@jeffpaul jeffpaul added type:enhancement New feature or request. help wanted Extra attention is needed and removed type:bug Something isn't working. labels Mar 19, 2024
@kirtangajjar kirtangajjar linked a pull request May 19, 2024 that will close this issue
4 tasks
@faisal-alvi faisal-alvi moved this from To Do to In Progress in Open Source Practice May 20, 2024
@jeffpaul jeffpaul modified the milestones: 2.2.5, 2.3.0 Jun 27, 2024
@manojsiddoji
Copy link

After looking into the issue, we found that files larger than 10MB encounter upload issues due to a limit within the DOMDocument.

We’ve identified a solution available in version 0.18.0 of enshrined/svg-sanitize, which includes an option to bypass this file size limit by setting $this->sanitizer->setAllowHugeFiles(true);. However, using this setting disables XML parser recursion limits, which could affect stability if left on by default.

Given that large SVG files (over 10MB) are rare, we suggest adding a setting to enable this feature only when needed. This way, users who require larger file uploads can enable setAllowHugeFiles(), while regular uploads remain unaffected.

@jeffpaul
Copy link
Member

@darylldoyle @faisal-alvi FYI on feedback/insight on this that might impact the approach in the current PR ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed type:enhancement New feature or request.
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

4 participants