ext/zip: add ZipArchive::openString method#21205
ext/zip: add ZipArchive::openString method#21205tstarling wants to merge 6 commits intophp:masterfrom
Conversation
ZIP_RDONLY was introduced in libzip 1.0.0 which is required since a979e9f
Signed-off-by: Soner Sayakci <s.sayakci@shopware.com>
Fixes to php#14078: * Rename ZipArchive::openBuffer() to ::openString(). * For consistency with ::open(), return int|bool, don't throw an exception on error. Provide error information via existing properties and accessors. * Fix memory leak when ::openString() is called but ::close() is not called. Add test. * Fix memory leak when a call to ::open() is followed by a call to ::openString(). Add test. * Let libzip own the source, don't call zip_source_keep(). * Share buffer handling with ZipArchive::addFromString(). Elsewhere: * If there is an error from zip_close() during a call to ZipArchive::open(), emit a warning but proceed to open the archive, don't return early. Add test. * When buffers are saved by ZipArchive::addFromString(), release them in ZipArchive::close() and ::open(), don't accumulate buffers until the free_obj handler is called. * Factor out buffer handling and reuse it in ZipArchive::openString()
ext/zip/php_zip.stub.php
Outdated
| /** @tentative-return-type */ | ||
| public function open(string $filename, int $flags = 0): bool|int {} | ||
|
|
||
| public function openString(string $data): bool|int {} |
There was a problem hiding this comment.
it looks like the return is actually true|int and it is impossible to return false
| php_zipobj_close(ze_obj); | ||
|
|
||
| /* open for write without option to empty the archive */ | ||
| #ifdef ZIP_RDONLY |
There was a problem hiding this comment.
the ZIP_RDONLY changes appear unrelated - if you restore #21195 I'm happy to review that, but ideally the addition here of openString() should not include other refactoring/changes that are unneeded
There was a problem hiding this comment.
OK, restored. It's not unrelated, it's needed because @shyim's patch was unconditionally using ZIP_RDONLY, which would have been an error if the guards were still needed. I would have submitted it separately anyway, except that it's a merge conflict, since both change the hash of php_zip.stub.php.
@ndossche is definitely the most qualified and it would be great to have his review. My work here carries on from his Regarding this commit and future directions: the read-only restriction is somewhat arbitrary -- we could provide an interface allowing a zip archive to be modified in memory, although I'm not sure exactly what that would look like. Maybe there would be a getter for the modified string. Then |
|
Since We could then have a default for the first parameter of |
5 participants
Co-authored-by: @shyim
Forked from #14078