]> granicus.if.org Git - php/commitdiff
Respect typed references in catch assignment
authorNikita Popov <nikita.ppv@gmail.com>
Tue, 26 May 2020 12:59:40 +0000 (14:59 +0200)
committerNikita Popov <nikita.ppv@gmail.com>
Tue, 26 May 2020 12:59:40 +0000 (14:59 +0200)
I decided to null out EG(exception) early here, which means only
the exception from the dtor / ref assign is preserved, and the
previous exception is not chained in. This is more robust, and
I don't think this situation is common enough to be bothered about
the precise behavior.

Zend/tests/bug53511.phpt
Zend/tests/type_declarations/typed_properties_108.phpt [new file with mode: 0644]
Zend/zend_vm_def.h
Zend/zend_vm_execute.h

index 0075655428b8de510699a0ccfd1607510c8ae2c5..583df3c2582b69981cd8027203378e1ee011f479 100644 (file)
@@ -20,12 +20,7 @@ function test() {
 test();
 echo "bug\n";
 --EXPECTF--
-Fatal error: Uncaught Exception: ops 2 in %sbug53511.php:11
-Stack trace:
-#0 %sbug53511.php(17): test()
-#1 {main}
-
-Next Exception: ops 1 in %sbug53511.php:4
+Fatal error: Uncaught Exception: ops 1 in %sbug53511.php:4
 Stack trace:
 #0 %sbug53511.php(12): Foo->__destruct()
 #1 %sbug53511.php(17): test()
diff --git a/Zend/tests/type_declarations/typed_properties_108.phpt b/Zend/tests/type_declarations/typed_properties_108.phpt
new file mode 100644 (file)
index 0000000..a2bd479
--- /dev/null
@@ -0,0 +1,47 @@
+--TEST--
+Variable assignment in catch must respect typed references
+--FILE--
+<?php
+
+class Test {
+    public int $i = 42;
+    public string $s = "str";
+}
+
+$test = new Test;
+
+$ref =& $test->i;
+try {
+    try {
+        throw new Exception("ex");
+    } catch (Exception $ref) {
+        echo "Unreachable\n";
+    }
+} catch (TypeError $e) {
+    var_dump($test->i);
+    echo $e . "\n\n";
+}
+
+$ref =& $test->s;
+try {
+    try {
+        throw new Exception("ex");
+    } catch (Exception $ref) {
+        echo "Unreachable\n";
+    }
+} catch (TypeError $e) {
+    var_dump($test->s);
+    echo $e . "\n\n";
+}
+
+?>
+--EXPECTF--
+int(42)
+TypeError: Cannot assign Exception to reference held by property Test::$i of type int in %s:%d
+Stack trace:
+#0 {main}
+
+string(3) "str"
+TypeError: Cannot assign Exception to reference held by property Test::$s of type string in %s:%d
+Stack trace:
+#0 {main}
index 8a9d74c1f8f8fb7894346e7fdb78de54450bf8a7..1039b902e7e7c02bb9dee5c3a2667b00ceb43ff6 100644 (file)
@@ -4632,17 +4632,15 @@ ZEND_VM_HANDLER(107, ZEND_CATCH, CONST, JMP_ADDR, LAST_CATCH|CACHE_SLOT)
 
        exception = EG(exception);
        ex = EX_VAR(opline->result.var);
-       if (UNEXPECTED(Z_ISREF_P(ex))) {
-               ex = Z_REFVAL_P(ex);
-       }
-       zval_ptr_dtor(ex);
-       ZVAL_OBJ(ex, EG(exception));
-       if (UNEXPECTED(EG(exception) != exception)) {
-               GC_ADDREF(EG(exception));
-               HANDLE_EXCEPTION();
-       } else {
+       {
+               /* Always perform a strict assignment. There is a reasonable expectation that if you
+                * write "catch (Exception $e)" then $e will actually be instanceof Exception. As such,
+                * we should not permit coercion to string here. */
+               zval tmp;
+               ZVAL_OBJ(&tmp, exception);
                EG(exception) = NULL;
-               ZEND_VM_NEXT_OPCODE();
+               zend_assign_to_variable(ex, &tmp, IS_TMP_VAR, /* strict */ 1);
+               ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
        }
 }
 
index 559edd070c741e0e73e755411fd90eb2bf1e82db..13d55291f9200681ee5f428bbe1260228d29e9e6 100644 (file)
@@ -3753,17 +3753,15 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CATCH_SPEC_CONST_HANDLER(ZEND_
 
        exception = EG(exception);
        ex = EX_VAR(opline->result.var);
-       if (UNEXPECTED(Z_ISREF_P(ex))) {
-               ex = Z_REFVAL_P(ex);
-       }
-       zval_ptr_dtor(ex);
-       ZVAL_OBJ(ex, EG(exception));
-       if (UNEXPECTED(EG(exception) != exception)) {
-               GC_ADDREF(EG(exception));
-               HANDLE_EXCEPTION();
-       } else {
+       {
+               /* Always perform a strict assignment. There is a reasonable expectation that if you
+                * write "catch (Exception $e)" then $e will actually be instanceof Exception. As such,
+                * we should not permit coercion to string here. */
+               zval tmp;
+               ZVAL_OBJ(&tmp, exception);
                EG(exception) = NULL;
-               ZEND_VM_NEXT_OPCODE();
+               zend_assign_to_variable(ex, &tmp, IS_TMP_VAR, /* strict */ 1);
+               ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
        }
 }