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