More return value handling fixes

Author: ndossche

ext/phar/phar_object.c

@@ -1351,10 +1351,13 @@ static phar_entry_data *phar_build_entry_data(char *fname, size_t fname_len, cha
 		zval rv;
 		zend_call_method_with_0_params(Z_OBJ_P(file_info), Z_OBJCE_P(file_info), NULL, "getMTime", &rv);
 
+		if (Z_ISREF(rv)) {
+			zend_unwrap_reference(&rv);
+		}
+
 		if (UNEXPECTED(Z_TYPE(rv) != IS_LONG)) {
-			/* Either an exception happened, or the function returned false to indicate failure. */
-			ZEND_ASSERT(Z_TYPE(rv) == IS_UNDEF || Z_TYPE(rv) == IS_FALSE);
-			*error = estrdup("getMTime() failed");
+			/* Either it's a tentative type failure, an exception happened, or the function returned false to indicate failure. */
+			*error = estrdup("getMTime() must return an int");
 			return NULL;
 		}
 

ext/phar/tests/buildFromIterator_user_overrides/getMTime.phpt

@@ -12,11 +12,6 @@ N. Dossche
 <?php
 
 class MySplFileInfo extends SplFileInfo {
-    public function getPathname(): string {
-        echo "[Pathname]\n";
-        return parent::getPathname();
-    }
-
     public function getMTime(): int|false {
         echo "[MTime]\n";
         return 123;
@@ -74,7 +69,6 @@ $workdir = __DIR__.'/001';
 ?>
 --EXPECTF--
 [ Found: %shello.txt ]
-[Pathname]
 [MTime]
 object(PharFileInfo)#%d (2) {
   ["pathName":"SplFileInfo":private]=>

ext/phar/tests/buildFromIterator_user_overrides/getMTime_byRef.phpt

@@ -0,0 +1,70 @@
+--TEST--
+buildFromIterator with user overrides - getMTime() by ref
+--EXTENSIONS--
+phar
+--INI--
+phar.readonly=0
+phar.require_hash=0
+--CREDITS--
+Arne Blankerts
+N. Dossche
+--FILE--
+<?php
+
+class MySplFileInfo extends SplFileInfo {
+    public function &getMTime(): int|false {
+        static $data = 123;
+        echo "[MTime]\n";
+        return $data;
+    }
+}
+
+class MyIterator extends RecursiveDirectoryIterator {
+    public function current(): SplFileInfo {
+        echo "[ Found: " . parent::current()->getPathname() . " ]\n";
+        return new MySplFileInfo(parent::current()->getPathname());
+    }
+}
+
+$workdir = __DIR__.'/007';
+mkdir($workdir . '/content', recursive: true);
+file_put_contents($workdir . '/content/hello.txt', "Hello world.");
+
+$phar = new \Phar($workdir . '/test.phar');
+$phar->startBuffering();
+$phar->buildFromIterator(
+    new RecursiveIteratorIterator(
+        new MyIterator($workdir . '/content', FilesystemIterator::SKIP_DOTS)
+    ),
+    $workdir
+);
+$phar->stopBuffering();
+
+
+$result = new \Phar($workdir . '/test.phar', 0, 'test.phar');
+var_dump($result['content/hello.txt']);
+var_dump($result['content/hello.txt']->getATime());
+var_dump($result['content/hello.txt']->getMTime());
+var_dump($result['content/hello.txt']->getCTime());
+
+?>
+--CLEAN--
+<?php
+$workdir = __DIR__.'/007';
+@unlink($workdir . '/test.phar');
+@unlink($workdir . '/content/hello.txt');
+@rmdir($workdir . '/content');
+@rmdir($workdir);
+?>
+--EXPECTF--
+[ Found: %shello.txt ]
+[MTime]
+object(PharFileInfo)#%d (2) {
+  ["pathName":"SplFileInfo":private]=>
+  string(%d) "phar://%shello.txt"
+  ["fileName":"SplFileInfo":private]=>
+  string(%d) "hello.txt"
+}
+int(123)
+int(123)
+int(123)

ext/phar/tests/buildFromIterator_user_overrides/getMTime_errors.phpt

@@ -36,6 +36,14 @@ class MySplFileInfo3 extends SplFileInfo {
     }
 }
 
+class MySplFileInfo4 extends SplFileInfo {
+    #[\ReturnTypeWillChange]
+    public function getMTime(): string {
+        echo "[MTime]\n";
+        return "wrong type";
+    }
+}
+
 class MyIterator extends RecursiveDirectoryIterator {
     public function current(): SplFileInfo {
         static $counter = 0;
@@ -47,6 +55,8 @@ class MyIterator extends RecursiveDirectoryIterator {
             return new MySplFileInfo2(parent::current()->getPathname());
         } else if ($counter === 3) {
             return new MySplFileInfo3(parent::current()->getPathname());
+        } else if ($counter === 4) {
+            return new MySplFileInfo4(parent::current()->getPathname());
         }
     }
 }
@@ -55,7 +65,7 @@ $workdir = __DIR__.'/002';
 mkdir($workdir . '/content', recursive: true);
 file_put_contents($workdir . '/content/hello.txt', "Hello world.");
 
-for ($i = 0; $i < 3; $i++) {
+for ($i = 0; $i < 4; $i++) {
     echo "--- Iteration $i ---\n";
     try {
         $phar = new \Phar($workdir . "/test$i.phar");
@@ -80,6 +90,7 @@ for ($i = 0; $i < 3; $i++) {
 <?php
 $workdir = __DIR__.'/002';
 @unlink($workdir . '/content/hello.txt');
+@unlink($workdir . '/test3.phar');
 @unlink($workdir . '/test2.phar');
 @unlink($workdir . '/test1.phar');
 @unlink($workdir . '/test0.phar');
@@ -94,9 +105,13 @@ Entry content%chello.txt cannot be created: timestamp is limited to 32-bit
 --- Iteration 1 ---
 [ Found: %shello.txt ]
 [MTime]
-Entry content%chello.txt cannot be created: getMTime() failed
+Entry content/hello.txt cannot be created: getMTime() must return an int
 --- Iteration 2 ---
 [ Found: %shello.txt ]
 [MTime]
-Entry content/hello.txt cannot be created: getMTime() failed
+Entry content/hello.txt cannot be created: getMTime() must return an int
 Previous: Throwing an exception inside getMTime()
+--- Iteration 3 ---
+[ Found: /run/media/niels/MoreData/php-src-multitasking/ext/phar/tests/buildFromIterator_user_overrides/002/content/hello.txt ]
+[MTime]
+Entry content/hello.txt cannot be created: getMTime() must return an int