tommitytom / RetroPlug

A frontend for the SameBoy GameBoy emulator, with a focus on music creation. It runs standalone and can be used as an audio plugin (VST) in your favourite DAW!
MIT License
304 stars 15 forks source link

Using ZIP format for plugin chunk data is problematic #25

Open sagamusix opened 2 years ago

sagamusix commented 2 years ago

Hello. I'm developing OpenMPT, and one of our users had trouble using this VST plugin in OpenMPT: After using it in a track and then trying to reload the track, the whole track failed load. This happens because OpenMPT can load zipped module files, so it first tries to read a file as a ZIP file, and if successful it tries to load a file from that ZIP file as a module. If the file wasn't a ZIP file, it will directly try to load it as a module instead.

What's the problem? Well, ZIP is a peculiar format. Some background information on that, in case you weren't aware: Most people would probably expect that a ZIP file must start with the "PK" magic bytes, but they don't really need to be at the start of the file. To read a ZIP file, you start by finding the central directory, which is placed towards the end of the file. It may not be at the exact end, because a comment text up to 64KB in length may follow. So a typical implementation will seek backwards within the last 64KB of the file to find the central directory (and it seems like many if not all ZIP libraries don't check if the declared length of that comment is actually identical to the number of byte past the end of the central directory). This mechanism is also the reason why you can open a self-extracting ZIP file (i.e. an EXE with embedded ZIP) in pretty much any software that can handle ZIP files - it will simply not care that there's an EXE stub in front of the ZIP.

As a consequence of this design, any music project file of any DAW where the plugin chunk dump by this plugin is placed close to the last 64KB of the project file is at the same time by definition also a valid ZIP file! And as a result of that, any software that, like OpenMPT, can open both regular ZIP files and also its own music project files will end up in a situation where a file is technically both a ZIP file and a music project file. I will try to find a workaround for OpenMPT, but as other DAWs could still encounter similar issues.

As a conclusion, I think it would make sense if RetroPlug didn't give a complete ZIP file as plugin chunk data to the host. I could think of several workarounds:

tommitytom commented 2 years ago

Projects can be saved and loaded outside of the VST (there is a standalone verison too), and it's handy beacuse the user can open the project directly in a zip application and extract the files themselves. Honestly this seems more like a strange OpenMPT architecture issue rather than a RetroPlug issue. This is a passion project and I just don't have the time or resources to completely change the project format beacuse of an issue with a single host that should probably be doing something differently anyway.

tommitytom commented 2 years ago

@sagamusix I have reconsidered this. I came across a bit brash, I'm sorry, I had just woken up and hadn't had my morning cup of tea! I understand the problem. Since it's only the VST build that will have this issue I guess I could XOR encrypt.. or maybe I could just change the PK to something else, like RP? Then it's easy for me to migrate because I can just check the chunk for PK or RP, and just load either as a zip

sagamusix commented 2 years ago

Hi, thanks for reconsidering. The problem (and that's what so unintuitive about ZIP files!) is in fact not the PK at the beginning of the chunk - it's the last PK, which denotes the "end of central directory" block. So if you fix that one up insead of the first one, it could work! The offset should always be the same relative to the file end (unless there is a global comment, which you aren't writing, so you can assume a fixed offset).

sagamusix commented 2 years ago

I did a bit more testing, and while blanking out the last PK in the file for example is enough to make minizip (the ZIP reader OpenMPT uses) give up, 7-zip will still reliably detect such files as being valid ZIP files. Maybe just XORing the whole block is the safest solution.

sagamusix commented 2 years ago

As an update from OpenMPT's side, I modified our loading code now so that it first tries to load files as module files, and only if that fails it tries again to read them as ZIP files. Still, as it's unlikely that OpenMPT is the only application affected by this quirk, so some workaround on the plugin side would still be a good idea.

Just to proof a point, here's an OpenMPT module using RetroPlug. If you have 7-zip installed, you can right-click the MPTM file, open with 7-zip, and it will appear as a valid zip file: retroplug.zip - the biggest issue here is that software that isn't even aware of what MPTM files are may erroneously process this file.