From b066ac0b231d3559af12e285be079c4bcb28052d Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 26 Apr 2025 12:40:38 +0200
Subject: [PATCH] Fix GH-18431: Registering ZIP progress callback twice doesn't
 work

Libzip already cleans up the previous callback, so when that means:
1. The callback zval being already copied over the previous one causes
   libzip to clean up the new callback object. This is the root cause.
2. Our own code to clean the old callback is redundant.

Closes GH-18432.
---
 NEWS                       |  4 ++++
 ext/zip/php_zip.c          | 10 ++--------
 ext/zip/tests/gh18431.phpt | 22 ++++++++++++++++++++++
 3 files changed, 28 insertions(+), 8 deletions(-)
 create mode 100644 ext/zip/tests/gh18431.phpt

diff --git a/NEWS b/NEWS
index bc5a597b619..c167074f0b8 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,10 @@ PHP                                                                        NEWS
 - Standard:
   . Fixed bug GH-17403 (Potential deadlock when putenv fails). (nielsdos)
 
+- Zip:
+  . Fixed bug GH-18431 (Registering ZIP progress callback twice doesn't work).
+    (nielsdos)
+
 08 May 2025, PHP 8.3.21
 
 - Core:
diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c
index 45b83aeae63..388b3485cbd 100644
--- a/ext/zip/php_zip.c
+++ b/ext/zip/php_zip.c
@@ -3048,14 +3048,11 @@ PHP_METHOD(ZipArchive, registerProgressCallback)
 
 	obj = Z_ZIP_P(self);
 
-	/* free if called twice */
-	_php_zip_progress_callback_free(obj);
-
 	/* register */
-	ZVAL_COPY(&obj->progress_callback, &fci.function_name);
 	if (zip_register_progress_callback_with_state(intern, rate, _php_zip_progress_callback, _php_zip_progress_callback_free, obj)) {
 		RETURN_FALSE;
 	}
+	ZVAL_COPY(&obj->progress_callback, &fci.function_name);
 
 	RETURN_TRUE;
 }
@@ -3093,14 +3090,11 @@ PHP_METHOD(ZipArchive, registerCancelCallback)
 
 	obj = Z_ZIP_P(self);
 
-	/* free if called twice */
-	_php_zip_cancel_callback_free(obj);
-
 	/* register */
-	ZVAL_COPY(&obj->cancel_callback, &fci.function_name);
 	if (zip_register_cancel_callback_with_state(intern, _php_zip_cancel_callback, _php_zip_cancel_callback_free, obj)) {
 		RETURN_FALSE;
 	}
+	ZVAL_COPY(&obj->cancel_callback, &fci.function_name);
 
 	RETURN_TRUE;
 }
diff --git a/ext/zip/tests/gh18431.phpt b/ext/zip/tests/gh18431.phpt
new file mode 100644
index 00000000000..d4eb89c36c6
--- /dev/null
+++ b/ext/zip/tests/gh18431.phpt
@@ -0,0 +1,22 @@
+--TEST--
+GH-18431 (Registering ZIP progress callback twice doesn't work)
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+$file = __DIR__ . '/gh18431.zip';
+$callback = var_dump(...);
+$zip = new ZipArchive;
+$zip->open($file, ZIPARCHIVE::CREATE);
+$zip->registerProgressCallback(0.5, $callback);
+$zip->registerProgressCallback(0.5, $callback);
+$zip->addFromString('foo', 'entry #1');
+?>
+--CLEAN--
+<?php
+$file = __DIR__ . '/gh18431.zip';
+@unlink($file);
+?>
+--EXPECT--
+float(0)
+float(1)