ext/uri: Refactors builder error handling

ext/uri/php_uri_common.c

@@ -40,24 +40,20 @@ static zend_string *get_known_string_by_property_name(php_uri_property_name prop
 	}
 }
 
-zend_result php_uri_pass_errors_by_ref_and_free(zval *errors_zv, zval *errors)
+ZEND_ATTRIBUTE_NONNULL_ARGS(2) zend_result php_uri_pass_errors_by_ref_and_free(zval *errors_zv, HashTable *errors)
 {
-	ZEND_ASSERT(Z_TYPE_P(errors) == IS_UNDEF || Z_TYPE_P(errors) == IS_ARRAY);
+	zval errors_tmp;
+	ZVAL_ARR(&errors_tmp, errors);
 
-	/* There was no error during parsing */
-	if (Z_ISUNDEF_P(errors)) {
-		return SUCCESS;
-	}
-
-	/* The errors parameter is an array, but the pass-by ref argument stored by
-	 * errors_zv was not passed - the URI implementation either doesn't support
-	 * returning additional error information, or the caller is not interested in it */
+	/* The pass-by ref argument stored by errors_zv was not passed - the URI
+	 * implementation either doesn't support returning additional error information,
+	 * or the caller is not interested in it */
 	if (errors_zv == NULL) {
-		zval_ptr_dtor(errors);
+		zval_ptr_dtor(&errors_tmp);
 		return SUCCESS;
 	}
 
-	ZEND_TRY_ASSIGN_REF_TMP(errors_zv, errors);
+	ZEND_TRY_ASSIGN_REF_TMP(errors_zv, &errors_tmp);
 	if (EG(exception)) {
 		return FAILURE;
 	}

ext/uri/php_uri_common.h

@@ -188,7 +188,7 @@ static inline const php_uri_property_handler *php_uri_parser_property_handler_by
 	}
 }
 
-zend_result php_uri_pass_errors_by_ref_and_free(zval *errors_zv, zval *errors);
+ZEND_ATTRIBUTE_NONNULL_ARGS(2) zend_result php_uri_pass_errors_by_ref_and_free(zval *errors_zv, HashTable *errors);
 void php_uri_property_read_helper(INTERNAL_FUNCTION_PARAMETERS, php_uri_property_name property_name, php_uri_component_read_mode component_read_mode);
 void php_uri_property_write_str_helper(INTERNAL_FUNCTION_PARAMETERS, php_uri_property_name property_name);
 void php_uri_property_write_str_or_null_helper(INTERNAL_FUNCTION_PARAMETERS, php_uri_property_name property_name);

ext/uri/tests/whatwg/builder/build_error_soft_errors_reset_without_base.phpt

@@ -3,6 +3,14 @@ Test Uri\WhatWg\UrlBuilder::build() - error - clears soft errors when an excepti
 --FILE--
 <?php
 
+$referenceErrors = [];
+
+try {
+    new Uri\WhatWg\Url("ht\ttps://");
+} catch (Throwable $e) {
+    $referenceErrors = $e->errors;
+}
+
 $builder = new Uri\WhatWg\UrlBuilder();
 $builder->setScheme("ht\ttps");
 $builder->setHost(null);
@@ -13,6 +21,10 @@ try {
 } catch (Throwable $e) {
     echo $e::class, ': ', $e->getMessage(), "\n";
     var_dump($e->errors);
+    var_dump(
+        array_map(static fn($error) => $error->type, $e->errors)
+        === array_map(static fn($error) => $error->type, $referenceErrors)
+    );
 }
 
 var_dump($softErrors);
@@ -24,21 +36,22 @@ array(2) {
   [0]=>
   object(Uri\WhatWg\UrlValidationError)#%d (%d) {
     ["context"]=>
-    string(4) "	tps"
+    string(0) ""
     ["type"]=>
-    enum(Uri\WhatWg\UrlValidationErrorType::InvalidUrlUnit)
+    enum(Uri\WhatWg\UrlValidationErrorType::HostMissing)
     ["failure"]=>
-    bool(false)
+    bool(true)
   }
   [1]=>
   object(Uri\WhatWg\UrlValidationError)#%d (%d) {
     ["context"]=>
-    string(0) ""
+    string(4) "	tps"
     ["type"]=>
-    enum(Uri\WhatWg\UrlValidationErrorType::HostMissing)
+    enum(Uri\WhatWg\UrlValidationErrorType::InvalidUrlUnit)
     ["failure"]=>
-    bool(true)
+    bool(false)
   }
 }
+bool(true)
 array(0) {
 }

ext/uri/tests/whatwg/builder/build_success_multiple_validation_warnings_with_base.phpt

@@ -0,0 +1,68 @@
+--TEST--
+Test Uri\WhatWg\UrlBuilder::build() - success - multiple validation warnings with base URL
+--FILE--
+<?php
+
+$base = new Uri\WhatWg\Url('https://example.com/base/path');
+
+$errors = [];
+$referenceErrors = [];
+
+$reference = new Uri\WhatWg\Url('//127.0.0.1.\newPath', $base, $referenceErrors);
+
+$url = new Uri\WhatWg\UrlBuilder()
+    ->setHost('127.0.0.1.')
+    ->setPath('\newPath')
+    ->build($base, $errors);
+
+var_dump($url->toAsciiString());
+var_dump($url);
+var_dump($errors);
+var_dump($errors == $referenceErrors);
+var_dump($url->equals(new Uri\WhatWg\Url($url->toAsciiString())));
+var_dump($url->equals($reference, Uri\UriComparisonMode::IncludeFragment));
+
+?>
+--EXPECTF--
+string(25) "https://127.0.0.1/newPath"
+object(Uri\WhatWg\Url)#%d (%d) {
+  ["scheme"]=>
+  string(5) "https"
+  ["username"]=>
+  NULL
+  ["password"]=>
+  NULL
+  ["host"]=>
+  string(9) "127.0.0.1"
+  ["port"]=>
+  NULL
+  ["path"]=>
+  string(8) "/newPath"
+  ["query"]=>
+  NULL
+  ["fragment"]=>
+  NULL
+}
+array(2) {
+  [0]=>
+  object(Uri\WhatWg\UrlValidationError)#%d (%d) {
+    ["context"]=>
+    string(8) "\newPath"
+    ["type"]=>
+    enum(Uri\WhatWg\UrlValidationErrorType::InvalidReverseSoldius)
+    ["failure"]=>
+    bool(false)
+  }
+  [1]=>
+  object(Uri\WhatWg\UrlValidationError)#%d (%d) {
+    ["context"]=>
+    string(0) ""
+    ["type"]=>
+    enum(Uri\WhatWg\UrlValidationErrorType::Ipv4EmptyPart)
+    ["failure"]=>
+    bool(false)
+  }
+}
+bool(true)
+bool(true)
+bool(true)

ext/uri/tests/whatwg/builder/build_success_soft_errors_with_base.phpt

@@ -25,7 +25,8 @@ array(2) {
   [0]=>
   object(Uri\WhatWg\UrlValidationError)#%d (%d) {
     ["context"]=>
-    string(2) "	b"
+    string(2) "
+y"
     ["type"]=>
     enum(Uri\WhatWg\UrlValidationErrorType::InvalidUrlUnit)
     ["failure"]=>
@@ -34,8 +35,7 @@ array(2) {
   [1]=>
   object(Uri\WhatWg\UrlValidationError)#%d (%d) {
     ["context"]=>
-    string(2) "
-y"
+    string(2) "	b"
     ["type"]=>
     enum(Uri\WhatWg\UrlValidationErrorType::InvalidUrlUnit)
     ["failure"]=>

ext/uri/tests/whatwg/builder/build_success_validation_warning_order_with_base.phpt

@@ -0,0 +1,67 @@
+--TEST--
+Test Uri\WhatWg\UrlBuilder::build() - success - validation warning order with base URL
+--FILE--
+<?php
+
+$base = new Uri\WhatWg\Url('https://example.com/base/path');
+
+$errors = [];
+
+$reference = new Uri\WhatWg\Url('//newUser:newPass@127.0.0.1.\newPath', $base);
+
+$url = new Uri\WhatWg\UrlBuilder()
+    ->setUsername('newUser')
+    ->setPassword('newPass')
+    ->setHost('127.0.0.1.')
+    ->setPath('\newPath')
+    ->build($base, $errors);
+
+var_dump($url->toAsciiString());
+var_dump($url);
+var_dump($errors);
+var_dump($url->equals(new Uri\WhatWg\Url($url->toAsciiString())));
+var_dump($url->equals($reference, Uri\UriComparisonMode::IncludeFragment));
+
+?>
+--EXPECTF--
+string(41) "https://newUser:newPass@127.0.0.1/newPath"
+object(Uri\WhatWg\Url)#%d (%d) {
+  ["scheme"]=>
+  string(5) "https"
+  ["username"]=>
+  string(7) "newUser"
+  ["password"]=>
+  string(7) "newPass"
+  ["host"]=>
+  string(9) "127.0.0.1"
+  ["port"]=>
+  NULL
+  ["path"]=>
+  string(8) "/newPath"
+  ["query"]=>
+  NULL
+  ["fragment"]=>
+  NULL
+}
+array(2) {
+  [0]=>
+  object(Uri\WhatWg\UrlValidationError)#%d (%d) {
+    ["context"]=>
+    string(8) "\newPath"
+    ["type"]=>
+    enum(Uri\WhatWg\UrlValidationErrorType::InvalidReverseSoldius)
+    ["failure"]=>
+    bool(false)
+  }
+  [1]=>
+  object(Uri\WhatWg\UrlValidationError)#%d (%d) {
+    ["context"]=>
+    string(0) ""
+    ["type"]=>
+    enum(Uri\WhatWg\UrlValidationErrorType::Ipv4EmptyPart)
+    ["failure"]=>
+    bool(false)
+  }
+}
+bool(true)
+bool(true)

ext/uri/tests/whatwg/builder/build_success_validation_warning_order_without_authority_with_base.phpt

@@ -0,0 +1,71 @@
+--TEST--
+Test Uri\WhatWg\UrlBuilder::build() - success - validation warning order without authority and with base URL
+--FILE--
+<?php
+
+$base = new Uri\WhatWg\Url('https://example.com/base/path');
+
+$errors = [];
+$referenceErrors = [];
+
+$reference = new Uri\WhatWg\Url('/foo\bar#%', $base, $referenceErrors);
+
+$url = new Uri\WhatWg\UrlBuilder()
+    ->setPath('/foo\bar')
+    ->setFragment('%')
+    ->build($base, $errors);
+
+var_dump($url->toAsciiString());
+var_dump($url);
+var_dump($errors);
+var_dump(
+    array_map(static fn($error) => $error->type, $errors)
+    === array_map(static fn($error) => $error->type, $referenceErrors)
+);
+var_dump($url->equals(new Uri\WhatWg\Url($url->toAsciiString())));
+var_dump($url->equals($reference, Uri\UriComparisonMode::IncludeFragment));
+
+?>
+--EXPECTF--
+string(29) "https://example.com/foo/bar#%"
+object(Uri\WhatWg\Url)#%d (%d) {
+  ["scheme"]=>
+  string(5) "https"
+  ["username"]=>
+  NULL
+  ["password"]=>
+  NULL
+  ["host"]=>
+  string(11) "example.com"
+  ["port"]=>
+  NULL
+  ["path"]=>
+  string(8) "/foo/bar"
+  ["query"]=>
+  NULL
+  ["fragment"]=>
+  string(1) "%"
+}
+array(2) {
+  [0]=>
+  object(Uri\WhatWg\UrlValidationError)#%d (%d) {
+    ["context"]=>
+    string(1) "%"
+    ["type"]=>
+    enum(Uri\WhatWg\UrlValidationErrorType::InvalidUrlUnit)
+    ["failure"]=>
+    bool(false)
+  }
+  [1]=>
+  object(Uri\WhatWg\UrlValidationError)#%d (%d) {
+    ["context"]=>
+    string(4) "\bar"
+    ["type"]=>
+    enum(Uri\WhatWg\UrlValidationErrorType::InvalidReverseSoldius)
+    ["failure"]=>
+    bool(false)
+  }
+}
+bool(true)
+bool(true)
+bool(true)

ext/uri/tests/whatwg/builder/build_success_validation_warning_order_without_base.phpt

@@ -0,0 +1,67 @@
+--TEST--
+Test Uri\WhatWg\UrlBuilder::build() - success - validation warning order without base URL
+--FILE--
+<?php
+
+$errors = [];
+$referenceErrors = [];
+
+$reference = new Uri\WhatWg\Url('https://127.0.0.1.\newPath', null, $referenceErrors);
+
+$url = new Uri\WhatWg\UrlBuilder()
+    ->setScheme('https')
+    ->setHost('127.0.0.1.')
+    ->setPath('\newPath')
+    ->build(null, $errors);
+
+var_dump($url->toAsciiString());
+var_dump($url);
+var_dump($errors);
+var_dump($errors == $referenceErrors);
+var_dump($url->equals(new Uri\WhatWg\Url($url->toAsciiString())));
+var_dump($url->equals($reference, Uri\UriComparisonMode::IncludeFragment));
+
+?>
+--EXPECTF--
+string(25) "https://127.0.0.1/newPath"
+object(Uri\WhatWg\Url)#%d (%d) {
+  ["scheme"]=>
+  string(5) "https"
+  ["username"]=>
+  NULL
+  ["password"]=>
+  NULL
+  ["host"]=>
+  string(9) "127.0.0.1"
+  ["port"]=>
+  NULL
+  ["path"]=>
+  string(8) "/newPath"
+  ["query"]=>
+  NULL
+  ["fragment"]=>
+  NULL
+}
+array(2) {
+  [0]=>
+  object(Uri\WhatWg\UrlValidationError)#%d (%d) {
+    ["context"]=>
+    string(8) "\newPath"
+    ["type"]=>
+    enum(Uri\WhatWg\UrlValidationErrorType::InvalidReverseSoldius)
+    ["failure"]=>
+    bool(false)
+  }
+  [1]=>
+  object(Uri\WhatWg\UrlValidationError)#%d (%d) {
+    ["context"]=>
+    string(0) ""
+    ["type"]=>
+    enum(Uri\WhatWg\UrlValidationErrorType::Ipv4EmptyPart)
+    ["failure"]=>
+    bool(false)
+  }
+}
+bool(true)
+bool(true)
+bool(true)