search

brimworks / lua-zip

Lua binding to libzip.
80 stars 25 forks source link

Add open_memory routine #15

Open bigcrush opened 4 years ago

bigcrush commented 4 years ago

Routine lets create zip archive in memory. Then we can send zip over network for example without saving it to disk.

bigcrush commented 4 years ago

MisterDA, I followed your advice. Now I need create new PR?

MisterDA commented 4 years ago

No, but you can rebase and force push if you want to, but that's not a big deal to me. Thank you for contributing! @brimworks what do you think?

brimworks commented 4 years ago

Sorry for the delay, things are pretty busy for me, so I probably won't be able to give this a great deal of attention until the weekend.

With that said, I did give a quick look at the code and I wonder if we could improve the API a bit. In particular, calling a method with a string that is either "file" or "string" argument seems a bit unexpected. Perhaps we could only have an API that takes the string argument, and avoid doing a file read? Users of the API should be able to use their own code to "slurp" the file into memory.

So, the new API would only support this:

local zip_arc = zip.open_memory([str [, flags]])

By removing support for a filename, it will make it easier to sandbox (in case a sandbox implementer wants to disable filesystem access and/or used in a non-blocking async context). It also slightly reduces the code we need to maintain.

Also, instead of reusing the ARCHIVE_MT, perhaps we could have a new meta-table, perhaps named ARCHIVE_MEMORY_MT. This ARCHIVE_MEMORY_MT could have a single user data that encapsulates both the struct zip* and zip_source_t* pointers. As long as the struct zip* comes first, then all existing ARCHIVE_MT methods should work, but then we could override the close() method with a memory specific method that returns a string for the content of the zip file.

If you are open to this idea, I should be able to get some time over the weekend to flesh this idea out a bit more. Or, if you want to take a stab, that would be great!

Thanks! -Brian

bigcrush commented 4 years ago

I like the idea about new metatable. While feeling some inconvenience with "__source_memory" your idea is nicer to me.

Open_memory prototype was made just like that to follow a pattern with :add, :replace methods which accept ...zip_source. But now it seems to me it makes no sense as we work fully in memory

Thanks!