Skip to content

Add ZipArchive::closeToString()#21497

Open
tstarling wants to merge 1 commit intophp:masterfrom
tstarling:ZipArchive-closeToString
Open

Add ZipArchive::closeToString()#21497
tstarling wants to merge 1 commit intophp:masterfrom
tstarling:ZipArchive-closeToString

Conversation

@tstarling
Copy link
Contributor

  • Add a $flags parameter to ZipArchive::openString(), by analogy with ZipArchive::open(). This allows the string to be opened read/write.
  • Have the $data parameter to ZipArchive::openString() default to an empty string, for convenience of callers that want to create an empty archive. This works on all versions of libzip since the change in 1.6.0 only applied to files, it's opt-in for generic sources.
  • Add ZipArchive::closeToString() which closes the archive and returns the resulting string. For consistency with openString(), return an empty string if the archive is empty.
  • Wrap strings passed to libzip with zip_source_function_create() instead of using zip_source_buffer_create(). This allows us to make the string writable, and simplifies memory management.

@tstarling tstarling requested a review from kocsismate as a code owner March 23, 2026 05:33
@tstarling tstarling force-pushed the ZipArchive-closeToString branch 2 times, most recently from 6ad9c14 to f8b8959 Compare March 23, 2026 06:12
@alecpl
Copy link

alecpl commented Mar 23, 2026

Imo, either closeString() or saveToString().

@tstarling tstarling force-pushed the ZipArchive-closeToString branch from f8b8959 to 2c11df3 Compare March 23, 2026 10:27
- Add a $flags parameter to ZipArchive::openString(), by analogy with
  ZipArchive::open(). This allows the string to be opened read/write.
- Have the $data parameter to ZipArchive::openString() default to an
  empty string, for convenience of callers that want to create an empty
  archive. This works on all versions of libzip since the change in
  1.6.0 only applied to files, it's opt-in for generic sources.
- Add ZipArchive::closeToString() which closes the archive and returns
  the resulting string. For consistency with openString(), return an
  empty string if the archive is empty.
- Wrap strings passed to libzip with zip_source_function_create()
  instead of using zip_source_buffer_create(). This allows us to make
  the string writable, and simplifies memory management.
@tstarling tstarling force-pushed the ZipArchive-closeToString branch from 2c11df3 to a266aa3 Compare March 24, 2026 03:11
@tstarling
Copy link
Contributor Author

@ndossche It would be good to have your review on this.

@remicollet I considered making an openString() PR for the PECL module, but it doesn't merge cleanly and I'm touching the same lines of code again here, so I decided to do this PR first.

@alecpl I think closeString() would be fine, i.e. referring to the data model, instead of referring to what it returns. The new method can only be called when the underlying data is a string. But I'll give it a day to see if there are any other comments about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants