Skip to content

Commit 94df2f6

Browse files
nikicnikic
committed
Fix bug #65006
The "callable name" may be the same for multiple distinct callables. The code already worked around this for the case of instance methods, but there are other cases in which callable names clash, such as the use of self:: reported in the referenced bug. Rather than trying to generate a unique name for callables, compare the content of the alfi structures. This is less efficient if there are many autoload functions, but autoload *registration* does not need to be particularly efficient. As a side-effect, this no longer permits unregistering non-callables.
1 parent 034741f commit 94df2f6

File tree

6 files changed

+151
-148
lines changed

6 files changed

+151
-148
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ PHP NEWS
163163
- SPL:
164164
. Fixed bug #71236 (Second call of spl_autoload_register() does nothing if it
165165
has no arguments). (Nikita)
166+
. Fixed bug #65006 (spl_autoload_register fails with multiple callables using
167+
self, same method). (Nikita)
166168

167169
- Standard:
168170
. Implemented FR #78638 (__PHP_Incomplete_Class should be final). (Laruence)

‎ext/spl/php_spl.c

Lines changed: 93 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -369,13 +369,11 @@ PHP_FUNCTION(spl_autoload_extensions)
369369
typedef struct {
370370
zend_function *func_ptr;
371371
zend_object *obj;
372-
zval closure;
372+
zend_object *closure;
373373
zend_class_entry *ce;
374374
} autoload_func_info;
375375

376-
static void autoload_func_info_dtor(zval *element)
377-
{
378-
autoload_func_info *alfi = (autoload_func_info*)Z_PTR_P(element);
376+
static void autoload_func_info_destroy(autoload_func_info *alfi) {
379377
if (alfi->obj) {
380378
zend_object_release(alfi->obj);
381379
}
@@ -384,12 +382,43 @@ static void autoload_func_info_dtor(zval *element)
384382
zend_string_release_ex(alfi->func_ptr->common.function_name, 0);
385383
zend_free_trampoline(alfi->func_ptr);
386384
}
387-
if (!Z_ISUNDEF(alfi->closure)) {
388-
zval_ptr_dtor(&alfi->closure);
385+
if (alfi->closure) {
386+
zend_object_release(alfi->closure);
389387
}
390388
efree(alfi);
391389
}
392390

391+
static void autoload_func_info_zval_dtor(zval *element)
392+
{
393+
autoload_func_info_destroy(Z_PTR_P(element));
394+
}
395+
396+
static autoload_func_info *autoload_func_info_from_fci(
397+
zend_fcall_info *fci, zend_fcall_info_cache *fcc) {
398+
autoload_func_info *alfi = emalloc(sizeof(autoload_func_info));
399+
alfi->ce = fcc->calling_scope;
400+
alfi->func_ptr = fcc->function_handler;
401+
alfi->obj = fcc->object;
402+
if (alfi->obj) {
403+
GC_ADDREF(alfi->obj);
404+
}
405+
if (Z_TYPE(fci->function_name) == IS_OBJECT) {
406+
alfi->closure = Z_OBJ(fci->function_name);
407+
GC_ADDREF(alfi->closure);
408+
} else {
409+
alfi->closure = NULL;
410+
}
411+
return alfi;
412+
}
413+
414+
static zend_bool autoload_func_info_equals(
415+
const autoload_func_info *alfi1, const autoload_func_info *alfi2) {
416+
return alfi1->func_ptr == alfi2->func_ptr
417+
&& alfi1->obj == alfi2->obj
418+
&& alfi1->ce == alfi2->ce
419+
&& alfi1->closure == alfi2->closure;
420+
}
421+
393422
static zend_class_entry *spl_perform_autoload(zend_string *class_name, zend_string *lc_name) {
394423
if (!SPL_G(autoload_functions)) {
395424
return NULL;
@@ -454,17 +483,29 @@ PHP_FUNCTION(spl_autoload_call)
454483
zend_hash_rehash(ht); \
455484
} while (0)
456485

486+
static Bucket *spl_find_registered_function(autoload_func_info *find_alfi) {
487+
if (!SPL_G(autoload_functions)) {
488+
return NULL;
489+
}
490+
491+
autoload_func_info *alfi;
492+
ZEND_HASH_FOREACH_PTR(SPL_G(autoload_functions), alfi) {
493+
if (autoload_func_info_equals(alfi, find_alfi)) {
494+
return _p;
495+
}
496+
} ZEND_HASH_FOREACH_END();
497+
return NULL;
498+
}
499+
457500
/* {{{ proto bool spl_autoload_register([mixed autoload_function [, bool throw [, bool prepend]]])
458501
Register given function as autoloader */
459502
PHP_FUNCTION(spl_autoload_register)
460503
{
461-
zend_string *func_name;
462-
zend_string *lc_name;
463504
zend_bool do_throw = 1;
464505
zend_bool prepend = 0;
465-
autoload_func_info alfi;
466506
zend_fcall_info fci = {0};
467507
zend_fcall_info_cache fcc;
508+
autoload_func_info *alfi;
468509

469510
ZEND_PARSE_PARAMETERS_START(0, 3)
470511
Z_PARAM_OPTIONAL
@@ -480,94 +521,45 @@ PHP_FUNCTION(spl_autoload_register)
480521

481522
if (!SPL_G(autoload_functions)) {
482523
ALLOC_HASHTABLE(SPL_G(autoload_functions));
483-
zend_hash_init(SPL_G(autoload_functions), 1, NULL, autoload_func_info_dtor, 0);
524+
zend_hash_init(SPL_G(autoload_functions), 1, NULL, autoload_func_info_zval_dtor, 0);
525+
/* Initialize as non-packed hash table for prepend functionality. */
526+
zend_hash_real_init_mixed(SPL_G(autoload_functions));
484527
}
485528

486529
/* If first arg is not null */
487530
if (ZEND_FCI_INITIALIZED(fci)) {
488-
alfi.ce = fcc.calling_scope;
489-
alfi.func_ptr = fcc.function_handler;
490-
alfi.obj = !(alfi.func_ptr->common.fn_flags & ZEND_ACC_STATIC) ? fcc.object : NULL;
491-
492531
if (fcc.function_handler->type == ZEND_INTERNAL_FUNCTION &&
493532
fcc.function_handler->internal_function.handler == zif_spl_autoload_call) {
494533
zend_argument_value_error(1, "must not be the spl_autoload_call() function");
495534
RETURN_THROWS();
496535
}
497536

498-
/* fci.function_name contains the callable zval */
499-
func_name = zend_get_callable_name(&fci.function_name);
500-
if (Z_TYPE(fci.function_name) == IS_OBJECT) {
501-
ZVAL_COPY(&alfi.closure, &fci.function_name);
502-
lc_name = zend_string_alloc(ZSTR_LEN(func_name) + sizeof(uint32_t), 0);
503-
zend_str_tolower_copy(ZSTR_VAL(lc_name), ZSTR_VAL(func_name), ZSTR_LEN(func_name));
504-
memcpy(ZSTR_VAL(lc_name) + ZSTR_LEN(func_name), &Z_OBJ_HANDLE(fci.function_name), sizeof(uint32_t));
505-
ZSTR_VAL(lc_name)[ZSTR_LEN(lc_name)] = '\0';
506-
} else {
507-
ZVAL_UNDEF(&alfi.closure);
508-
/* Skip leading \ */
509-
if (ZSTR_VAL(func_name)[0] == '\\') {
510-
lc_name = zend_string_alloc(ZSTR_LEN(func_name) - 1, 0);
511-
zend_str_tolower_copy(ZSTR_VAL(lc_name), ZSTR_VAL(func_name) + 1, ZSTR_LEN(func_name) - 1);
512-
} else {
513-
lc_name = zend_string_tolower(func_name);
514-
}
515-
}
516-
zend_string_release(func_name);
517-
518-
if (zend_hash_exists(SPL_G(autoload_functions), lc_name)) {
519-
if (!Z_ISUNDEF(alfi.closure)) {
520-
Z_DELREF_P(&alfi.closure);
521-
}
522-
goto skip;
523-
}
524-
525-
if (alfi.obj) {
526-
/* add object id to the hash to ensure uniqueness, for more reference look at bug #40091 */
527-
lc_name = zend_string_extend(lc_name, ZSTR_LEN(lc_name) + sizeof(uint32_t), 0);
528-
memcpy(ZSTR_VAL(lc_name) + ZSTR_LEN(lc_name) - sizeof(uint32_t), &alfi.obj->handle, sizeof(uint32_t));
529-
ZSTR_VAL(lc_name)[ZSTR_LEN(lc_name)] = '\0';
530-
GC_ADDREF(alfi.obj);
531-
}
532-
533-
if (UNEXPECTED(alfi.func_ptr == &EG(trampoline))) {
537+
alfi = autoload_func_info_from_fci(&fci, &fcc);
538+
if (UNEXPECTED(alfi->func_ptr == &EG(trampoline))) {
534539
zend_function *copy = emalloc(sizeof(zend_op_array));
535540

536-
memcpy(copy, alfi.func_ptr, sizeof(zend_op_array));
537-
alfi.func_ptr->common.function_name = NULL;
538-
alfi.func_ptr = copy;
539-
}
540-
if (zend_hash_add_mem(SPL_G(autoload_functions), lc_name, &alfi, sizeof(autoload_func_info)) == NULL) {
541-
if (alfi.obj && !(alfi.func_ptr->common.fn_flags & ZEND_ACC_STATIC)) {
542-
GC_DELREF(alfi.obj);
543-
}
544-
if (!Z_ISUNDEF(alfi.closure)) {
545-
Z_DELREF(alfi.closure);
546-
}
547-
if (UNEXPECTED(alfi.func_ptr->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE)) {
548-
zend_string_release_ex(alfi.func_ptr->common.function_name, 0);
549-
zend_free_trampoline(alfi.func_ptr);
550-
}
551-
}
552-
if (prepend && SPL_G(autoload_functions)->nNumOfElements > 1) {
553-
/* Move the newly created element to the head of the hashtable */
554-
HT_MOVE_TAIL_TO_HEAD(SPL_G(autoload_functions));
541+
memcpy(copy, alfi->func_ptr, sizeof(zend_op_array));
542+
alfi->func_ptr->common.function_name = NULL;
543+
alfi->func_ptr = copy;
555544
}
556-
skip:
557-
zend_string_release_ex(lc_name, 0);
558545
} else {
559-
autoload_func_info spl_alfi;
560-
spl_alfi.func_ptr = zend_hash_str_find_ptr(
546+
alfi = emalloc(sizeof(autoload_func_info));
547+
alfi->func_ptr = zend_hash_str_find_ptr(
561548
CG(function_table), "spl_autoload", sizeof("spl_autoload") - 1);
562-
spl_alfi.obj = NULL;
563-
spl_alfi.ce = NULL;
564-
ZVAL_UNDEF(&spl_alfi.closure);
565-
zend_hash_add_mem(SPL_G(autoload_functions), spl_alfi.func_ptr->common.function_name,
566-
&spl_alfi, sizeof(autoload_func_info));
567-
if (prepend && SPL_G(autoload_functions)->nNumOfElements > 1) {
568-
/* Move the newly created element to the head of the hashtable */
569-
HT_MOVE_TAIL_TO_HEAD(SPL_G(autoload_functions));
570-
}
549+
alfi->obj = NULL;
550+
alfi->ce = NULL;
551+
alfi->closure = NULL;
552+
}
553+
554+
if (spl_find_registered_function(alfi)) {
555+
autoload_func_info_destroy(alfi);
556+
RETURN_TRUE;
557+
}
558+
559+
zend_hash_next_index_insert_ptr(SPL_G(autoload_functions), alfi);
560+
if (prepend && SPL_G(autoload_functions)->nNumOfElements > 1) {
561+
/* Move the newly created element to the head of the hashtable */
562+
HT_MOVE_TAIL_TO_HEAD(SPL_G(autoload_functions));
571563
}
572564

573565
RETURN_TRUE;
@@ -577,68 +569,29 @@ PHP_FUNCTION(spl_autoload_register)
577569
Unregister given function as autoloader */
578570
PHP_FUNCTION(spl_autoload_unregister)
579571
{
580-
zend_string *func_name = NULL;
581-
char *error = NULL;
582-
zend_string *lc_name;
583-
zval *zcallable;
584-
int success = FAILURE;
585-
zend_object *obj_ptr;
572+
zend_fcall_info fci;
586573
zend_fcall_info_cache fcc;
587574

588-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &zcallable) == FAILURE) {
575+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "f", &fci, &fcc) == FAILURE) {
589576
RETURN_THROWS();
590577
}
591578

592-
if (!zend_is_callable_ex(zcallable, NULL, IS_CALLABLE_CHECK_SYNTAX_ONLY, &func_name, &fcc, &error)) {
593-
zend_throw_exception_ex(spl_ce_LogicException, 0, "Unable to unregister invalid function (%s)", error);
594-
if (error) {
595-
efree(error);
596-
}
597-
if (func_name) {
598-
zend_string_release_ex(func_name, 0);
599-
}
600-
RETURN_FALSE;
601-
}
602-
obj_ptr = fcc.object;
603-
if (error) {
604-
efree(error);
605-
}
606-
607-
if (Z_TYPE_P(zcallable) == IS_OBJECT) {
608-
lc_name = zend_string_alloc(ZSTR_LEN(func_name) + sizeof(uint32_t), 0);
609-
zend_str_tolower_copy(ZSTR_VAL(lc_name), ZSTR_VAL(func_name), ZSTR_LEN(func_name));
610-
memcpy(ZSTR_VAL(lc_name) + ZSTR_LEN(func_name), &Z_OBJ_HANDLE_P(zcallable), sizeof(uint32_t));
611-
ZSTR_VAL(lc_name)[ZSTR_LEN(lc_name)] = '\0';
612-
} else {
613-
/* Skip leading \ */
614-
if (ZSTR_VAL(func_name)[0] == '\\') {
615-
lc_name = zend_string_alloc(ZSTR_LEN(func_name) - 1, 0);
616-
zend_str_tolower_copy(ZSTR_VAL(lc_name), ZSTR_VAL(func_name) + 1, ZSTR_LEN(func_name) - 1);
617-
} else {
618-
lc_name = zend_string_tolower(func_name);
619-
}
579+
if (zend_string_equals_literal(
580+
fcc.function_handler->common.function_name, "spl_autoload_call")) {
581+
/* Don't destroy the hash table, as we might be iterating over it right now. */
582+
zend_hash_clean(SPL_G(autoload_functions));
583+
RETURN_TRUE;
620584
}
621-
zend_string_release_ex(func_name, 0);
622585

623-
if (SPL_G(autoload_functions)) {
624-
if (zend_string_equals_literal(lc_name, "spl_autoload_call")) {
625-
/* Don't destroy the hash table, as we might be iterating over it right now. */
626-
zend_hash_clean(SPL_G(autoload_functions));
627-
success = SUCCESS;
628-
} else {
629-
/* remove specific */
630-
success = zend_hash_del(SPL_G(autoload_functions), lc_name);
631-
if (success != SUCCESS && obj_ptr) {
632-
lc_name = zend_string_extend(lc_name, ZSTR_LEN(lc_name) + sizeof(uint32_t), 0);
633-
memcpy(ZSTR_VAL(lc_name) + ZSTR_LEN(lc_name) - sizeof(uint32_t), &obj_ptr->handle, sizeof(uint32_t));
634-
ZSTR_VAL(lc_name)[ZSTR_LEN(lc_name)] = '\0';
635-
success = zend_hash_del(SPL_G(autoload_functions), lc_name);
636-
}
637-
}
586+
autoload_func_info *alfi = autoload_func_info_from_fci(&fci, &fcc);
587+
Bucket *p = spl_find_registered_function(alfi);
588+
autoload_func_info_destroy(alfi);
589+
if (p) {
590+
zend_hash_del_bucket(SPL_G(autoload_functions), p);
591+
RETURN_TRUE;
638592
}
639593

640-
zend_string_release_ex(lc_name, 0);
641-
RETURN_BOOL(success == SUCCESS);
594+
RETURN_FALSE;
642595
} /* }}} */
643596

644597
/* {{{ proto false|array spl_autoload_functions()
@@ -654,9 +607,11 @@ PHP_FUNCTION(spl_autoload_functions)
654607
array_init(return_value);
655608
if (SPL_G(autoload_functions)) {
656609
ZEND_HASH_FOREACH_PTR(SPL_G(autoload_functions), alfi) {
657-
if (!Z_ISUNDEF(alfi->closure)) {
658-
Z_ADDREF(alfi->closure);
659-
add_next_index_zval(return_value, &alfi->closure);
610+
if (alfi->closure) {
611+
zval obj_zv;
612+
ZVAL_OBJ(&obj_zv, alfi->closure);
613+
Z_ADDREF(obj_zv);
614+
add_next_index_zval(return_value, &obj_zv);
660615
} else if (alfi->func_ptr->common.scope) {
661616
zval tmp;
662617

‎ext/spl/php_spl.stub.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ function spl_autoload_functions(): array {}
1818

1919
function spl_autoload_register(?callable $autoload_function = null, bool $throw = true, bool $prepend = false): bool {}
2020

21-
function spl_autoload_unregister($autoload_function): bool {}
21+
function spl_autoload_unregister(callable $autoload_function): bool {}
2222

2323
function spl_classes(): array {}
2424

‎ext/spl/php_spl_arginfo.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_spl_autoload_register, 0, 0, _IS
3535
ZEND_END_ARG_INFO()
3636

3737
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_spl_autoload_unregister, 0, 1, _IS_BOOL, 0)
38-
ZEND_ARG_INFO(0, autoload_function)
38+
ZEND_ARG_TYPE_INFO(0, autoload_function, IS_CALLABLE, 0)
3939
ZEND_END_ARG_INFO()
4040

4141
#define arginfo_spl_classes arginfo_spl_autoload_functions

‎ext/spl/tests/bug65006.phpt

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
Bug #65006: spl_autoload_register fails with multiple callables using self, same method
3+
--FILE--
4+
<?php
5+
6+
class first {
7+
public static function init() {
8+
spl_autoload_register(array('self','load'));
9+
}
10+
public static function load($class) {}
11+
}
12+
13+
class second {
14+
public static function init() {
15+
spl_autoload_register(array('self','load'));
16+
}
17+
public static function load($class){}
18+
}
19+
20+
first::init();
21+
second::init();
22+
var_dump(spl_autoload_functions());
23+
24+
?>
25+
--EXPECT--
26+
array(2) {
27+
[0]=>
28+
array(2) {
29+
[0]=>
30+
string(5) "first"
31+
[1]=>
32+
string(4) "load"
33+
}
34+
[1]=>
35+
array(2) {
36+
[0]=>
37+
string(6) "second"
38+
[1]=>
39+
string(4) "load"
40+
}
41+
}

0 commit comments

Comments
 (0)