Fix ZipArchive callback being called after executor has shut down

free_obj() for objects referenced in the main symbol table may be called only
once the executor has already shut down. php_zip_cancel_callback() may attempt
to invoke a user callback, which will terminate the process because user code is
not expected to be executed at this point. We solve this by calling the callback
in dtor_obj(), which is called earlier in the shutdown sequence.

dtor_obj() will actually attempt to call it again if the object was
reinitialized in the destructor, so we also avoid calling the callback when the
executor has shut down in the first place. This should never matter in practice.

Closes GH-19602

Author: iluuu1994

NEWS

@@ -29,4 +29,8 @@ PHP                                                                        NEWS
   . Fixed bug GH-19926 (reset internal pointer earlier while splicing array
     while COW violation flag is still set). (alexandre-daubois)
 
+- Zip:
+  . Fixed ZipArchive callback being called after executor has shut down.
+    (ilutov)
+
 <<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>

ext/zip/php_zip.c

@@ -1002,6 +1002,21 @@ static void php_zip_cancel_callback_free(void *ptr)
 }
 #endif
 
+static void php_zip_object_dtor(zend_object *object)
+{
+	zend_objects_destroy_object(object);
+
+	ze_zip_object *intern = php_zip_fetch_object(object);
+
+	if (intern->za) {
+		if (zip_close(intern->za) != 0) {
+			php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za));
+			zip_discard(intern->za);
+		}
+		intern->za = NULL;
+	}
+}
+
 static void php_zip_object_free_storage(zend_object *object) /* {{{ */
 {
 	ze_zip_object * intern = php_zip_fetch_object(object);
@@ -2995,6 +3010,10 @@ PHP_METHOD(ZipArchive, getStream)
 #ifdef HAVE_PROGRESS_CALLBACK
 static void php_zip_progress_callback(zip_t *arch, double state, void *ptr)
 {
+	if (!EG(active)) {
+		return;
+	}
+
 	zval cb_args[1];
 	ze_zip_object *obj = ptr;
 
@@ -3041,6 +3060,10 @@ static int php_zip_cancel_callback(zip_t *arch, void *ptr)
 	zval cb_retval;
 	ze_zip_object *obj = ptr;
 
+	if (!EG(active)) {
+		return 0;
+	}
+
 	zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL);
 	if (Z_ISUNDEF(cb_retval)) {
 		/* Cancel if an exception has been thrown */
@@ -3128,6 +3151,7 @@ static PHP_MINIT_FUNCTION(zip)
 	memcpy(&zip_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
 	zip_object_handlers.offset = XtOffsetOf(ze_zip_object, zo);
 	zip_object_handlers.free_obj = php_zip_object_free_storage;
+	zip_object_handlers.dtor_obj = php_zip_object_dtor;
 	zip_object_handlers.clone_obj = NULL;
 	zip_object_handlers.get_property_ptr_ptr = php_zip_get_property_ptr_ptr;
 

ext/zip/tests/ZipArchive_bailout.phpt

@@ -0,0 +1,28 @@
+--TEST--
+ZipArchive destructor should be called on bailout
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+
+function &cb() {}
+
+$file = __DIR__ . '/gh18907.zip';
+$zip = new ZipArchive;
+$zip->open($file, ZIPARCHIVE::CREATE);
+$zip->registerCancelCallback(cb(...));
+$zip->addFromString('test', 'test');
+$fusion = $zip;
+
+?>
+--CLEAN--
+<?php
+$file = __DIR__ . '/gh18907.zip';
+@unlink($file);
+?>
+--EXPECTF--
+Notice: Only variable references should be returned by reference in %s on line %d
+
+Notice: Only variable references should be returned by reference in %s on line %d
+
+Notice: Only variable references should be returned by reference in %s on line %d

ext/zip/tests/ZipArchive_destruct.phpt

@@ -0,0 +1,28 @@
+--TEST--
+Leaking ZipArchive destructor
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+
+class MyZipArchive extends ZipArchive {
+    public function __destruct() {
+        global $leak;
+        $leak = $this;
+    }
+}
+
+new MyZipArchive;
+$file = __DIR__ . '/ZipArchive_destruct.zip';
+$leak->open($file, ZIPARCHIVE::CREATE);
+$leak->addFromString('test', 'test');
+
+?>
+===DONE===
+--CLEAN--
+<?php
+$file = __DIR__ . '/ZipArchive_destruct.zip';
+@unlink($file);
+?>
+--EXPECT--
+===DONE===