Review fixes + optimize property reads + fix leak

Author: kocsismate

ext/uri/php_uri.c

@@ -47,8 +47,8 @@ zend_class_entry *php_uri_ce_whatwg_url_validation_error;
 static zend_object_handlers object_handlers_rfc3986_uri;
 static zend_object_handlers object_handlers_whatwg_uri;
 
-typedef bool (*php_uri_string_component_validator)(const zend_string *component);
-typedef bool (*php_uri_long_component_validator)(zend_long component);
+typedef bool (*php_uri_component_validator_string)(const zend_string *component);
+typedef bool (*php_uri_component_validator_long)(zend_long component);
 
 static const zend_module_dep uri_deps[] = {
 	ZEND_MOD_REQUIRED("lexbor")
@@ -1060,7 +1060,7 @@ PHP_METHOD(Uri_Rfc3986_UriBuilder, reset)
 	zend_update_property_null(php_uri_ce_rfc3986_uri_builder, Z_OBJ_P(ZEND_THIS), ZEND_STRL("scheme"));
 	zend_update_property_null(php_uri_ce_rfc3986_uri_builder, Z_OBJ_P(ZEND_THIS), ZEND_STRL("userinfo"));
 	zend_update_property_null(php_uri_ce_rfc3986_uri_builder, Z_OBJ_P(ZEND_THIS), ZEND_STRL("host"));
-	zend_update_property_stringl(php_uri_ce_rfc3986_uri_builder, Z_OBJ_P(ZEND_THIS), ZEND_STRL("path"), "", 0);
+	zend_update_property_str(php_uri_ce_rfc3986_uri_builder, Z_OBJ_P(ZEND_THIS), ZEND_STRL("path"), ZSTR_EMPTY_ALLOC());
 	zend_update_property_null(php_uri_ce_rfc3986_uri_builder, Z_OBJ_P(ZEND_THIS), ZEND_STRL("query"));
 	zend_update_property_null(php_uri_ce_rfc3986_uri_builder, Z_OBJ_P(ZEND_THIS), ZEND_STRL("fragment"));
 
@@ -1069,7 +1069,7 @@ PHP_METHOD(Uri_Rfc3986_UriBuilder, reset)
 
 ZEND_ATTRIBUTE_NONNULL static void php_uri_builder_set_component_string(
 	INTERNAL_FUNCTION_PARAMETERS, const char *name, size_t name_length,
-	const php_uri_string_component_validator validator
+	const php_uri_component_validator_string validator
 ) {
 	zend_string *component;
 
@@ -1089,7 +1089,7 @@ ZEND_ATTRIBUTE_NONNULL static void php_uri_builder_set_component_string(
 
 ZEND_ATTRIBUTE_NONNULL static void php_uri_builder_set_component_string_or_null(
 	INTERNAL_FUNCTION_PARAMETERS, const char *name, size_t name_length,
-	const php_uri_string_component_validator validator
+	const php_uri_component_validator_string validator
 ) {
 	zend_string *component;
 
@@ -1113,7 +1113,7 @@ ZEND_ATTRIBUTE_NONNULL static void php_uri_builder_set_component_string_or_null(
 
 ZEND_ATTRIBUTE_NONNULL_ARGS(1) static void php_uri_builder_set_component_long_or_null(
 	INTERNAL_FUNCTION_PARAMETERS, const char *name, size_t name_length,
-	const php_uri_long_component_validator validator
+	const php_uri_component_validator_long validator
 ) {
 	zend_long component;
 	bool component_is_null;
@@ -1209,15 +1209,14 @@ PHP_METHOD(Uri_Rfc3986_UriBuilder, build)
 	ZEND_PARSE_PARAMETERS_END();
 
 	zend_object *obj = Z_OBJ_P(ZEND_THIS);
-	zval tmp;
 
-	const zval *scheme = zend_read_property(obj->ce, obj, ZEND_STRL("scheme"), false, &tmp);
-	const zval *userinfo = zend_read_property(obj->ce, obj, ZEND_STRL("userinfo"), false, &tmp);
-	const zval *host = zend_read_property(obj->ce, obj, ZEND_STRL("host"), false, &tmp);
-	const zval *port = zend_read_property(obj->ce, obj, ZEND_STRL("port"), false, &tmp);
-	const zval *path = zend_read_property(obj->ce, obj, ZEND_STRL("path"), false, &tmp);
-	const zval *query = zend_read_property(obj->ce, obj, ZEND_STRL("query"), false, &tmp);
-	const zval *fragment = zend_read_property(obj->ce, obj, ZEND_STRL("fragment"), false, &tmp);
+	const zval *scheme = OBJ_PROP_NUM(obj, 0);
+	const zval *userinfo = OBJ_PROP_NUM(obj, 1);
+	const zval *host = OBJ_PROP_NUM(obj, 2);
+	const zval *port = OBJ_PROP_NUM(obj, 3);
+	const zval *path = OBJ_PROP_NUM(obj, 4);
+	const zval *query = OBJ_PROP_NUM(obj, 5);
+	const zval *fragment = OBJ_PROP_NUM(obj, 6);
 
 	zend_string *uri_str = php_uri_parser_rfc3986_recompose_from_zval(scheme, userinfo, host, port, path, query, fragment);
 	if (uri_str == NULL) {

ext/uri/uri_parser_rfc3986.c

@@ -723,19 +723,20 @@ ZEND_ATTRIBUTE_NONNULL bool php_uri_parser_rfc3986_validate_port(zend_long port)
 	const char *p = ZSTR_VAL(tmp);
 	const size_t len = ZSTR_LEN(tmp);
 
-	return uriIsWellFormedPortA(p, p + len);
+	bool result = uriIsWellFormedPortA(p, p + len);
+	zend_string_release(tmp);
+
+	return result;
 }
 
 ZEND_ATTRIBUTE_NONNULL bool php_uri_parser_rfc3986_validate_path(const zend_string *path)
 {
 	const char *p = ZSTR_VAL(path);
 	const size_t len = ZSTR_LEN(path);
 
-	/*
-	 It's checked during the build() method whether the path begins with a "/" when there's a host
-	 that's why a false hasHost argument is passed to uriIsWellFormedPathA() for now.
-	*/
-	return uriIsWellFormedPathA(p, p + len, false);
+	/* The build() method checks whether the path begins with a "/" when there's a host.
+	 * In order to skip doing the same check, a false hasHost argument is passed to uriIsWellFormedPathA(). */
+	return uriIsWellFormedPathA(p, p + len, /* hasHost */ false);
 }
 
 ZEND_ATTRIBUTE_NONNULL bool php_uri_parser_rfc3986_validate_query(const zend_string *query)
@@ -756,18 +757,26 @@ ZEND_ATTRIBUTE_NONNULL bool php_uri_parser_rfc3986_validate_fragment(const zend_
 
 ZEND_ATTRIBUTE_NONNULL zend_string *php_uri_parser_rfc3986_recompose_from_zval(
 	const zval *scheme, const zval *userinfo, const zval *host, const zval *port,
-	const zval *path, const zval *query, const zval *fragment)
-{
-	/* The userinfo and port components can only ever be present if the host is present */
-	if ((Z_TYPE_P(userinfo) != IS_NULL || Z_TYPE_P(port) != IS_NULL) && Z_TYPE_P(host) == IS_NULL) {
-		zend_throw_exception(php_uri_ce_invalid_uri_exception, "The URI must contain a host if either the userinfo or the port component is present", 0);
-		return NULL;
-	}
+	const zval *path, const zval *query, const zval *fragment
+) {
+	if (Z_TYPE_P(host) == IS_NULL) {
+		/* The path must not begin with "//" if the URI doesn't contain a host */
+		if (zend_string_starts_with_literal(Z_STR_P(path), "//")) {
+			zend_throw_exception(php_uri_ce_invalid_uri_exception, "The path must not begin with \"//\" when the URI doesn't contain a host", 0);
+			return NULL;
+		}
 
-	/* The path must be either empty or begin with a "/" if the URI contains a host */
-	if (Z_TYPE_P(host) != IS_NULL && (Z_STRLEN_P(path) > 0 && Z_STRVAL_P(path)[0] != '/')) {
-		zend_throw_exception(php_uri_ce_invalid_uri_exception, "A path must be either empty or begin with \"/\" when the URI contains a host", 0);
-		return NULL;
+		/* The userinfo and port components must not be present if the URI doesn't contain a host */
+		if (Z_TYPE_P(userinfo) != IS_NULL || Z_TYPE_P(port) != IS_NULL) {
+			zend_throw_exception(php_uri_ce_invalid_uri_exception, "The URI must contain a host if either the userinfo or the port component is present", 0);
+			return NULL;
+		}
+	} else {
+		/* The path must be either empty or begin with a "/" if the URI contains a host */
+		if (Z_STRLEN_P(path) > 0 && Z_STRVAL_P(path)[0] != '/') {
+			zend_throw_exception(php_uri_ce_invalid_uri_exception, "A path must be either empty or begin with \"/\" when the URI contains a host", 0);
+			return NULL;
+		}
 	}
 
 	/* The first segment of the path must not contain ":" if the URI doesn't contain a scheme */
@@ -783,12 +792,6 @@ ZEND_ATTRIBUTE_NONNULL zend_string *php_uri_parser_rfc3986_recompose_from_zval(
 		}
 	}
 
-	/* The path must not begin with "//" if the URI doesn't contain a host */
-	if (Z_TYPE_P(host) == IS_NULL && Z_STRLEN_P(path) >= 2 && Z_STRVAL_P(path)[0] == '/' && Z_STRVAL_P(path)[1] == '/') {
-		zend_throw_exception(php_uri_ce_invalid_uri_exception, "The path must not begin with \"//\" when the URI doesn't contain a host", 0);
-		return NULL;
-	}
-
 	/* result = "" */
 	smart_str uri_str = {0};