Skip to content

Commit 4432083

Browse files
TimWollaTimWolla
authored
uri: Improve safety of URI object initialization (php#19648)
* uri: Inline implementation of `php_uri_implementation_set_object_handlers()` There is no one time fits all solution to initialization of the object handlers. A follow-up commit will use distinct `create_object` handlers for each parser class. Explicitly spelling out the handlers is a well-established pattern in php-src and I don't see a reason to diverge from that with an intransparent helper method. * uri: Initialize the `.internal` field of `uri_object_t` immediately upon creation This makes the objects much safer to use, since the `.parser` will always be available and matching the object. * uri: Remove `uri_parser_name` parameter of `uri_unserialize()` The parser for a given object is already known from the object itself and particularly must never change. Reassigning the value in `uri_unserialize()` is just unsafe, especially since the existing `->uri` is freed with the destructor of the reassigned parser. Just rely on the `->parser` field being set to the correct value. * uri: Remove the `uri_parser` parameter from `php_uri_instantiate_uri()` Similarly to the previous change to `uri_unserialize()`, the `->parser` must always match the object for the freeing to be safe. Given that we expect to successfully parse URIs, we can eagerly initialize the resulting URI object when using the `::parse()` methods and destruct it again when parsing fails and `null` is returned instead. Calling the destructor is safe, since `uri` will be `NULL`, which will result in a noop. The `base_url_object` must also match the object that is currently being constructed. Verify this using assertions matching the `->ce` and the `->parser`. * uri: Export the individual object handlers
1 parent 4e8058e commit 4432083

File tree

3 files changed

+64
-54
lines changed

3 files changed

+64
-54
lines changed

‎ext/uri/php_uri.c‎

Lines changed: 59 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -330,16 +330,33 @@ static zend_result pass_errors_by_ref_and_free(zval *errors_zv, zval *errors)
330330
}
331331

332332
ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2) PHPAPI void php_uri_instantiate_uri(
333-
INTERNAL_FUNCTION_PARAMETERS, const uri_parser_t *uri_parser, const zend_string *uri_str, const zend_object *base_url_object,
333+
INTERNAL_FUNCTION_PARAMETERS, const zend_string *uri_str, const zend_object *base_url_object,
334334
bool should_throw, bool should_update_this_object, zval *errors_zv
335335
) {
336+
337+
uri_object_t *uri_object;
338+
if (should_update_this_object) {
339+
uri_object = Z_URI_OBJECT_P(ZEND_THIS);
340+
} else {
341+
if (EX(func)->common.fn_flags & ZEND_ACC_STATIC) {
342+
object_init_ex(return_value, Z_CE_P(ZEND_THIS));
343+
} else {
344+
object_init_ex(return_value, Z_OBJCE_P(ZEND_THIS));
345+
}
346+
uri_object = Z_URI_OBJECT_P(return_value);
347+
}
348+
349+
const uri_parser_t *uri_parser = uri_object->internal.parser;
350+
336351
zval errors;
337352
ZVAL_UNDEF(&errors);
338353

339354
void *base_url = NULL;
340355
if (base_url_object != NULL) {
356+
ZEND_ASSERT(base_url_object->ce == uri_object->std.ce);
341357
uri_internal_t *internal_base_url = uri_internal_from_obj(base_url_object);
342358
URI_ASSERT_INITIALIZATION(internal_base_url);
359+
ZEND_ASSERT(internal_base_url->parser == uri_parser);
343360
base_url = internal_base_url->uri;
344361
}
345362

@@ -352,6 +369,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2) PHPAPI void php_uri_instantiate_uri(
352369
if (pass_errors_by_ref_and_free(errors_zv, &errors) == FAILURE) {
353370
RETURN_THROWS();
354371
}
372+
zval_ptr_dtor(return_value);
355373
RETURN_NULL();
356374
}
357375
}
@@ -361,19 +379,6 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2) PHPAPI void php_uri_instantiate_uri(
361379
RETURN_THROWS();
362380
}
363381

364-
uri_object_t *uri_object;
365-
if (should_update_this_object) {
366-
uri_object = Z_URI_OBJECT_P(ZEND_THIS);
367-
} else {
368-
if (EX(func)->common.fn_flags & ZEND_ACC_STATIC) {
369-
object_init_ex(return_value, Z_CE_P(ZEND_THIS));
370-
} else {
371-
object_init_ex(return_value, Z_OBJCE_P(ZEND_THIS));
372-
}
373-
uri_object = Z_URI_OBJECT_P(return_value);
374-
}
375-
376-
uri_object->internal.parser = uri_parser;
377382
uri_object->internal.uri = uri;
378383
}
379384

@@ -388,7 +393,7 @@ static void create_rfc3986_uri(INTERNAL_FUNCTION_PARAMETERS, bool is_constructor
388393
Z_PARAM_OBJ_OF_CLASS_OR_NULL(base_url_object, uri_rfc3986_uri_ce)
389394
ZEND_PARSE_PARAMETERS_END();
390395

391-
php_uri_instantiate_uri(INTERNAL_FUNCTION_PARAM_PASSTHRU, &php_uri_parser_rfc3986, uri_str, base_url_object, is_constructor, is_constructor, NULL);
396+
php_uri_instantiate_uri(INTERNAL_FUNCTION_PARAM_PASSTHRU, uri_str, base_url_object, is_constructor, is_constructor, NULL);
392397
}
393398

394399
PHP_METHOD(Uri_Rfc3986_Uri, parse)
@@ -475,7 +480,7 @@ static void create_whatwg_uri(INTERNAL_FUNCTION_PARAMETERS, bool is_constructor)
475480
Z_PARAM_ZVAL(errors)
476481
ZEND_PARSE_PARAMETERS_END();
477482

478-
php_uri_instantiate_uri(INTERNAL_FUNCTION_PARAM_PASSTHRU, &php_uri_parser_whatwg, uri_str, base_url_object, is_constructor, is_constructor, errors);
483+
php_uri_instantiate_uri(INTERNAL_FUNCTION_PARAM_PASSTHRU, uri_str, base_url_object, is_constructor, is_constructor, errors);
479484
}
480485

481486
PHP_METHOD(Uri_WhatWg_Url, parse)
@@ -756,11 +761,7 @@ PHP_METHOD(Uri_Rfc3986_Uri, resolve)
756761
Z_PARAM_PATH_STR(uri_str)
757762
ZEND_PARSE_PARAMETERS_END();
758763

759-
zend_object *this_object = Z_OBJ_P(ZEND_THIS);
760-
uri_internal_t *internal_uri = uri_internal_from_obj(this_object);
761-
URI_ASSERT_INITIALIZATION(internal_uri);
762-
763-
php_uri_instantiate_uri(INTERNAL_FUNCTION_PARAM_PASSTHRU, internal_uri->parser, uri_str, this_object, true, false, NULL);
764+
php_uri_instantiate_uri(INTERNAL_FUNCTION_PARAM_PASSTHRU, uri_str, Z_OBJ_P(ZEND_THIS), true, false, NULL);
764765
}
765766

766767
PHP_METHOD(Uri_Rfc3986_Uri, __serialize)
@@ -793,7 +794,7 @@ PHP_METHOD(Uri_Rfc3986_Uri, __serialize)
793794
zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &arr);
794795
}
795796

796-
static void uri_unserialize(INTERNAL_FUNCTION_PARAMETERS, const char *uri_parser_name)
797+
static void uri_unserialize(INTERNAL_FUNCTION_PARAMETERS)
797798
{
798799
HashTable *data;
799800

@@ -829,7 +830,6 @@ static void uri_unserialize(INTERNAL_FUNCTION_PARAMETERS, const char *uri_parser
829830
}
830831

831832
uri_internal_t *internal_uri = uri_internal_from_obj(object);
832-
internal_uri->parser = uri_parser_by_name(uri_parser_name, strlen(uri_parser_name));
833833
if (internal_uri->uri != NULL) {
834834
internal_uri->parser->free_uri(internal_uri->uri);
835835
}
@@ -855,7 +855,7 @@ static void uri_unserialize(INTERNAL_FUNCTION_PARAMETERS, const char *uri_parser
855855

856856
PHP_METHOD(Uri_Rfc3986_Uri, __unserialize)
857857
{
858-
uri_unserialize(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_URI_PARSER_RFC3986);
858+
uri_unserialize(INTERNAL_FUNCTION_PARAM_PASSTHRU);
859859
}
860860

861861
PHP_METHOD(Uri_Rfc3986_Uri, __debugInfo)
@@ -949,11 +949,7 @@ PHP_METHOD(Uri_WhatWg_Url, resolve)
949949
Z_PARAM_ZVAL(errors)
950950
ZEND_PARSE_PARAMETERS_END();
951951

952-
zend_object *this_object = Z_OBJ_P(ZEND_THIS);
953-
uri_internal_t *internal_uri = uri_internal_from_obj(this_object);
954-
URI_ASSERT_INITIALIZATION(internal_uri);
955-
956-
php_uri_instantiate_uri(INTERNAL_FUNCTION_PARAM_PASSTHRU, internal_uri->parser, uri_str, this_object, true, false, errors);
952+
php_uri_instantiate_uri(INTERNAL_FUNCTION_PARAM_PASSTHRU, uri_str, Z_OBJ_P(ZEND_THIS), true, false, errors);
957953
}
958954

959955
PHP_METHOD(Uri_WhatWg_Url, __serialize)
@@ -988,7 +984,7 @@ PHP_METHOD(Uri_WhatWg_Url, __serialize)
988984

989985
PHP_METHOD(Uri_WhatWg_Url, __unserialize)
990986
{
991-
uri_unserialize(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_URI_PARSER_WHATWG);
987+
uri_unserialize(INTERNAL_FUNCTION_PARAM_PASSTHRU);
992988
}
993989

994990
PHP_METHOD(Uri_WhatWg_Url, __debugInfo)
@@ -1000,17 +996,32 @@ PHP_METHOD(Uri_WhatWg_Url, __debugInfo)
1000996
RETURN_ARR(uri_get_debug_properties(object));
1001997
}
1002998

1003-
static zend_object *uri_create_object_handler(zend_class_entry *class_type)
999+
PHPAPI uri_object_t *php_uri_object_create(zend_class_entry *class_type, const uri_parser_t *parser)
10041000
{
10051001
uri_object_t *uri_object = zend_object_alloc(sizeof(*uri_object), class_type);
10061002

10071003
zend_object_std_init(&uri_object->std, class_type);
10081004
object_properties_init(&uri_object->std, class_type);
10091005

1010-
return &uri_object->std;
1006+
uri_object->internal = (uri_internal_t){
1007+
.parser = parser,
1008+
.uri = NULL,
1009+
};
1010+
1011+
return uri_object;
1012+
}
1013+
1014+
static zend_object *php_uri_object_create_rfc3986(zend_class_entry *ce)
1015+
{
1016+
return &php_uri_object_create(ce, &php_uri_parser_rfc3986)->std;
10111017
}
10121018

1013-
static void uri_free_obj_handler(zend_object *object)
1019+
static zend_object *php_uri_object_create_whatwg(zend_class_entry *ce)
1020+
{
1021+
return &php_uri_object_create(ce, &php_uri_parser_whatwg)->std;
1022+
}
1023+
1024+
PHPAPI void php_uri_object_handler_free(zend_object *object)
10141025
{
10151026
uri_object_t *uri_object = uri_object_from_obj(object);
10161027

@@ -1023,18 +1034,15 @@ static void uri_free_obj_handler(zend_object *object)
10231034
zend_object_std_dtor(&uri_object->std);
10241035
}
10251036

1026-
static zend_object *uri_clone_obj_handler(zend_object *object)
1037+
PHPAPI zend_object *php_uri_object_handler_clone(zend_object *object)
10271038
{
10281039
uri_object_t *uri_object = uri_object_from_obj(object);
10291040
uri_internal_t *internal_uri = uri_internal_from_obj(object);
10301041

10311042
URI_ASSERT_INITIALIZATION(internal_uri);
10321043

1033-
zend_object *new_object = uri_create_object_handler(object->ce);
1034-
ZEND_ASSERT(new_object != NULL);
1035-
uri_object_t *new_uri_object = uri_object_from_obj(new_object);
1036-
1037-
new_uri_object->internal.parser = internal_uri->parser;
1044+
uri_object_t *new_uri_object = uri_object_from_obj(object->ce->create_object(object->ce));
1045+
ZEND_ASSERT(new_uri_object->internal.parser == internal_uri->parser);
10381046

10391047
void *uri = internal_uri->parser->clone_uri(internal_uri->uri);
10401048
ZEND_ASSERT(uri != NULL);
@@ -1046,16 +1054,6 @@ static zend_object *uri_clone_obj_handler(zend_object *object)
10461054
return &new_uri_object->std;
10471055
}
10481056

1049-
ZEND_ATTRIBUTE_NONNULL PHPAPI void php_uri_implementation_set_object_handlers(zend_class_entry *ce, zend_object_handlers *object_handlers)
1050-
{
1051-
ce->create_object = uri_create_object_handler;
1052-
ce->default_object_handlers = object_handlers;
1053-
memcpy(object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
1054-
object_handlers->offset = XtOffsetOf(uri_object_t, std);
1055-
object_handlers->free_obj = uri_free_obj_handler;
1056-
object_handlers->clone_obj = uri_clone_obj_handler;
1057-
}
1058-
10591057
PHPAPI zend_result php_uri_parser_register(const uri_parser_t *uri_parser)
10601058
{
10611059
zend_string *key = zend_string_init_interned(uri_parser->name, strlen(uri_parser->name), true);
@@ -1076,10 +1074,20 @@ PHPAPI zend_result php_uri_parser_register(const uri_parser_t *uri_parser)
10761074
static PHP_MINIT_FUNCTION(uri)
10771075
{
10781076
uri_rfc3986_uri_ce = register_class_Uri_Rfc3986_Uri();
1079-
php_uri_implementation_set_object_handlers(uri_rfc3986_uri_ce, &uri_rfc3986_uri_object_handlers);
1077+
uri_rfc3986_uri_ce->create_object = php_uri_object_create_rfc3986;
1078+
uri_rfc3986_uri_ce->default_object_handlers = &uri_rfc3986_uri_object_handlers;
1079+
memcpy(&uri_rfc3986_uri_object_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
1080+
uri_rfc3986_uri_object_handlers.offset = XtOffsetOf(uri_object_t, std);
1081+
uri_rfc3986_uri_object_handlers.free_obj = php_uri_object_handler_free;
1082+
uri_rfc3986_uri_object_handlers.clone_obj = php_uri_object_handler_clone;
10801083

10811084
uri_whatwg_url_ce = register_class_Uri_WhatWg_Url();
1082-
php_uri_implementation_set_object_handlers(uri_whatwg_url_ce, &uri_whatwg_uri_object_handlers);
1085+
uri_whatwg_url_ce->create_object = php_uri_object_create_whatwg;
1086+
uri_whatwg_url_ce->default_object_handlers = &uri_whatwg_uri_object_handlers;
1087+
memcpy(&uri_whatwg_uri_object_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
1088+
uri_whatwg_uri_object_handlers.offset = XtOffsetOf(uri_object_t, std);
1089+
uri_whatwg_uri_object_handlers.free_obj = php_uri_object_handler_free;
1090+
uri_whatwg_uri_object_handlers.clone_obj = php_uri_object_handler_clone;
10831091

10841092
uri_comparison_mode_ce = register_class_Uri_UriComparisonMode();
10851093
uri_exception_ce = register_class_Uri_UriException(zend_ce_exception);

‎ext/uri/php_uri.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,8 @@ ZEND_ATTRIBUTE_NONNULL PHPAPI php_uri *php_uri_parse_to_struct(
205205
ZEND_ATTRIBUTE_NONNULL PHPAPI void php_uri_struct_free(php_uri *uri);
206206

207207
ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2) PHPAPI void php_uri_instantiate_uri(
208-
INTERNAL_FUNCTION_PARAMETERS, const uri_parser_t *uri_parser, const zend_string *uri_str, const zend_object *base_url_object,
208+
INTERNAL_FUNCTION_PARAMETERS, const zend_string *uri_str, const zend_object *base_url_object,
209209
bool should_throw, bool should_update_this_object, zval *errors_zv
210210
);
211211

212-
ZEND_ATTRIBUTE_NONNULL PHPAPI void php_uri_implementation_set_object_handlers(zend_class_entry *ce, zend_object_handlers *object_handlers);
213-
214212
#endif

‎ext/uri/php_uri_common.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,10 @@ static inline uri_internal_t *uri_internal_from_obj(const zend_object *object) {
159159
#define Z_URI_OBJECT_P(zv) uri_object_from_obj(Z_OBJ_P((zv)))
160160
#define Z_URI_INTERNAL_P(zv) uri_internal_from_obj(Z_OBJ_P((zv)))
161161

162+
PHPAPI uri_object_t *php_uri_object_create(zend_class_entry *class_type, const uri_parser_t *parser);
163+
PHPAPI void php_uri_object_handler_free(zend_object *object);
164+
PHPAPI zend_object *php_uri_object_handler_clone(zend_object *object);
165+
162166
#define PHP_URI_PARSER_RFC3986 "Uri\\Rfc3986\\Uri"
163167
#define PHP_URI_PARSER_WHATWG "Uri\\WhatWg\\Url"
164168
#define PHP_URI_PARSER_PHP_PARSE_URL "parse_url"

0 commit comments

Comments
 (0)