Handle zip imports regression - #874
Conversation
b802b74 to
8b4b7f7
Compare
when either a suitable importer or suitable backup handler is not found
…ws vs returning undefined + add tests
8b4b7f7 to
fe0ce64
Compare
fredrikekelund
left a comment
There was a problem hiding this comment.
LGTM 👍 Given our current circumstance, the PR is an improvement, it tests well and the code looks good.
However… If many other unzip implementations (including the built-in tools on macOS and Windows) don't have a problem with leading slashes, I would strongly argue that Studio shouldn't either. I understand the argument that the spec makes about security, but my response would simply be: "let's automatically fix those filenames instead of outright rejecting the ZIP file". @ashfame, do you know of a way to do this with yauzl, or should we consider looking into replacement libraries for upcoming versions of Studio?
Co-authored-by: Fredrik Rombach Ekelund <fredrik@f26d.dev>
|
@fredrikekelund Yep, I agree in principle of not leaving users hanging. Please see this comment where I laid down my thoughts on approaching this. I believe we need a archive handler abstraction that contains all this logic. We can continue that discussion there and figure out future next steps and let this PR be merged. |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
|
All good with the Sentry error, it's expected 👍 |
Related issues
Proposed Changes
importBackup()andimportSite()to not return undefined and just throwTesting Instructions
Pre-merge Checklist