Make WordPress Core

Opened 8 months ago

Closed 8 months ago

Last modified 8 months ago

#59467 closed defect (bug) (fixed)

Close ZIP archives before returning in `_unzip_file_ziparchive()`.

Reported by: costdev's profile costdev Owned by: costdev's profile costdev
Milestone: 6.4 Priority: normal
Severity: normal Version: 3.0
Component: Filesystem API Keywords: has-patch
Focuses: performance Cc:

Description (last modified by costdev)

In _unzip_file_ziparchive(), the archive is opened, but there are several returns that don't close the archive beforehand.

From the PHP manual on ZipArchive::close():

This method is automatically called at the end of the script.
Reference


and from the documentation for the underlying zip_close() from libzip:

If successful, archive is freed. Otherwise archive is left unchanged and must still be freed.
Reference


As this function is used during installation and upgrade processes which are memory-intensive, let's free the archive before each of the return statements, rather than waiting until the end of the script.

Note: The return that gets used as a result of true !== $zopen does not need a call to $z->close(), as freeing the archive is handled internally by libzip's zip_open().


Change History (6)

This ticket was mentioned in PR #5322 on WordPress/wordpress-develop by @costdev.


8 months ago
#1

There are several early returns in _unzip_file_ziparchive() which don't close the archive prior to returning.

As this function is used in installation and upgrade processes which are memory-intensive, this calls ZipArchive::close() to free the archive prior to each early return. This excludes the first return which is a result of a failure to open the archive, which is freed internally when the failure occurs.

References:

#2 @costdev
8 months ago

  • Description modified (diff)

#3 @afragen
8 months ago

Looks good from our previous discussion. 😉

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


8 months ago

#5 @costdev
8 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 56735:

Filesystem API: Free the archive in _unzip_file_ziparchive().

There are several early returns in _unzip_file_ziparchive() which don't close the archive prior to returning.

As this function is used in installation and upgrade processes which are memory-intensive, this calls ZipArchive::close() to free the archive prior to each early return. This excludes the first return which is a result of a failure to open the archive, which is freed internally when the failure occurs.

References:

Follow-up to: [13005], [13006], [13015], [13221], [14346] [25779].

Props azaozz, afragen, joemcgill, costdev.
Fixes #59467

@costdev commented on PR #5322:


8 months ago
#6

Committed in r56735.

Note: See TracTickets for help on using tickets.