Skip to content

Commit 9b1fb32

Browse files
withinboredomwithinboredom
committed
Fix asymmetric visibility validation for private(namespace)
The visibility modifiers don't form a simple linear hierarchy. Instead, they form two separate partial orders: - Inheritance axis: public is a superset of protected is a superset of private - Namespace axis: public is a superset of private(namespace) is a superset of private protected and private(namespace) operate on different axes and are incomparable: neither is a subset of the other. For asymmetric properties, the rule is: the set of callers who can write must be equal to or a superset of those who can read (C[set] is a subset of C[base]). This commit adds explicit validation to reject incompatible combinations: - protected private(namespace)(set) - rejected (incomparable) - private(namespace) protected(set) - rejected (incomparable) - private(namespace) private(set) - rejected (reversed hierarchy) Added four tests to verify the new validation logic.
1 parent f8e98aa commit 9b1fb32

6 files changed

+125
-5
lines changed

‎Zend/tests/access_modifiers/private_namespace_asymmetric_invalid_001.phpt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
Asymmetric visibility with incompatible modifiers (protected and private(namespace))
3+
--FILE--
4+
<?php
5+
6+
namespace Test;
7+
8+
// This should fail: protected and private(namespace) are incomparable
9+
class A {
10+
protected private(namespace)(set) string $prop1;
11+
}
12+
13+
?>
14+
--EXPECTF--
15+
Fatal error: Property Test\A::$prop1 has incompatible visibility modifiers: protected and private(namespace) operate on different axes (inheritance vs namespace) and cannot be combined in asymmetric visibility in %s on line %d

‎Zend/tests/access_modifiers/private_namespace_asymmetric_invalid_002.phpt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
Asymmetric visibility with incompatible modifiers (private(namespace) and protected(set))
3+
--FILE--
4+
<?php
5+
6+
namespace Test;
7+
8+
// This should fail: private(namespace) and protected(set) are incomparable
9+
class A {
10+
private(namespace) protected(set) string $prop1;
11+
}
12+
13+
?>
14+
--EXPECTF--
15+
Fatal error: Property Test\A::$prop1 has incompatible visibility modifiers: protected and private(namespace) operate on different axes (inheritance vs namespace) and cannot be combined in asymmetric visibility in %s on line %d

‎Zend/tests/access_modifiers/private_namespace_asymmetric_invalid_003.phpt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
Asymmetric visibility: private(namespace) private(set) is invalid (reversed hierarchy)
3+
--FILE--
4+
<?php
5+
6+
namespace Test;
7+
8+
// This should fail: C[private] ⊈ C[ns] (set is more restrictive than get)
9+
class A {
10+
private(namespace) private(set) string $prop1;
11+
}
12+
13+
?>
14+
--EXPECTF--
15+
Fatal error: Visibility of property Test\A::$prop1 must not be weaker than set visibility in %s on line %d

‎Zend/tests/access_modifiers/private_namespace_asymmetric_valid_combinations.phpt‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
Valid asymmetric visibility combinations with private(namespace)
3+
--FILE--
4+
<?php
5+
6+
namespace Test;
7+
8+
class A {
9+
// Valid: public base allows any set visibility
10+
public private(namespace)(set) string $prop1 = "test1";
11+
public protected(set) string $prop2 = "test2";
12+
public private(set) string $prop3 = "test3";
13+
14+
// Valid: protected base allows protected(set) or private(set) (inheritance axis only)
15+
protected protected(set) string $prop4 = "test4";
16+
protected private(set) string $prop5 = "test5";
17+
18+
// Valid: private(namespace) base allows only private(namespace)(set) (namespace axis only)
19+
private(namespace) private(namespace)(set) string $prop6 = "test6";
20+
21+
// Valid: private base allows only private(set)
22+
private private(set) string $prop7 = "test7";
23+
24+
public function test() {
25+
echo "prop1: " . $this->prop1 . "\n";
26+
echo "prop2: " . $this->prop2 . "\n";
27+
echo "prop3: " . $this->prop3 . "\n";
28+
echo "prop4: " . $this->prop4 . "\n";
29+
echo "prop5: " . $this->prop5 . "\n";
30+
echo "prop6: " . $this->prop6 . "\n";
31+
echo "prop7: " . $this->prop7 . "\n";
32+
}
33+
}
34+
35+
$a = new A();
36+
$a->test();
37+
38+
echo "All valid combinations work correctly!\n";
39+
40+
?>
41+
--EXPECT--
42+
prop1: test1
43+
prop2: test2
44+
prop3: test3
45+
prop4: test4
46+
prop5: test5
47+
prop6: test6
48+
prop7: test7
49+
All valid combinations work correctly!

‎Zend/zend_API.c‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4491,11 +4491,33 @@ ZEND_API zend_property_info *zend_declare_typed_property(zend_class_entry *ce, z
44914491
"Property with asymmetric visibility %s::$%s must have type",
44924492
ZSTR_VAL(ce->name), ZSTR_VAL(name));
44934493
}
4494-
/* Validate asymmetric visibility hierarchy: public < protected < private(namespace) < private
4495-
* Set visibility must be equal to or more restrictive than get visibility. */
4496-
uint32_t get_visibility = zend_visibility_to_set_visibility(access_type & ZEND_ACC_PPP_MASK);
4494+
/* Validate asymmetric visibility hierarchy.
4495+
*
4496+
* Visibility modifiers form two partial orders
4497+
* - Inheritance axis: public ⊇ protected ⊇ private
4498+
* - Namespace axis: public ⊇ private(namespace) ⊇ private
4499+
*
4500+
* protected and private(namespace) are incomparable (neither is a subset of the other).
4501+
*
4502+
* For asymmetric properties, the set visibility must satisfy: C[set] ⊇ C[base]
4503+
* This means the set of callers who can write must be a subset of those who can read.
4504+
*/
4505+
uint32_t get_visibility = access_type & ZEND_ACC_PPP_MASK;
44974506
uint32_t set_visibility = access_type & ZEND_ACC_PPP_SET_MASK;
4498-
if (get_visibility > set_visibility) {
4507+
4508+
/* Check for incompatible combinations (protected and private(namespace) on different axes). */
4509+
if ((get_visibility == ZEND_ACC_PROTECTED && set_visibility == ZEND_ACC_NAMESPACE_PRIVATE_SET) ||
4510+
(get_visibility == ZEND_ACC_NAMESPACE_PRIVATE && set_visibility == ZEND_ACC_PROTECTED_SET)) {
4511+
zend_error_noreturn(ce->type == ZEND_INTERNAL_CLASS ? E_CORE_ERROR : E_COMPILE_ERROR,
4512+
"Property %s::$%s has incompatible visibility modifiers: "
4513+
"protected and private(namespace) operate on different axes (inheritance vs namespace) "
4514+
"and cannot be combined in asymmetric visibility",
4515+
ZSTR_VAL(ce->name), ZSTR_VAL(name));
4516+
}
4517+
4518+
/* Check hierarchy using numeric comparison within each axis. */
4519+
uint32_t get_visibility_as_set = zend_visibility_to_set_visibility(get_visibility);
4520+
if (get_visibility_as_set > set_visibility) {
44994521
zend_error_noreturn(ce->type == ZEND_INTERNAL_CLASS ? E_CORE_ERROR : E_COMPILE_ERROR,
45004522
"Visibility of property %s::$%s must not be weaker than set visibility",
45014523
ZSTR_VAL(ce->name), ZSTR_VAL(name));

‎Zend/zend_compile.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,11 @@ typedef struct _zend_oparray_context {
216216
/* Common flags | | | */
217217
/* ============ | | | */
218218
/* | | | */
219-
/* Visibility flags (public < protected < private) | | | */
219+
/* Visibility flags | | | */
220+
/* Two partial orders (not linear): | | | */
221+
/* - Inheritance axis: public ⊇ protected ⊇ private | | | */
222+
/* - Namespace axis: public ⊇ private(namespace) ⊇ private| | | */
223+
/* protected and private(namespace) are incomparable | | | */
220224
#define ZEND_ACC_PUBLIC (1 << 0) /* | X | X | X */
221225
#define ZEND_ACC_PROTECTED (1 << 1) /* | X | X | X */
222226
#define ZEND_ACC_PRIVATE (1 << 2) /* | X | X | X */

0 commit comments

Comments
 (0)