Warning to error promotion for set(raw)cookie()

ext/standard/head.c

@@ -76,36 +76,41 @@ PHPAPI int php_header(void)
 	}
 }
 
-PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain, int secure, int httponly, zend_string *samesite, int url_encode)
+#define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", and \"\\014\""
+PHPAPI zend_result php_setcookie(zend_string *name, zend_string *value, time_t expires,
+	zend_string *path, zend_string *domain, bool secure, bool httponly,
+	zend_string *samesite, bool url_encode)
 {
 	zend_string *dt;
 	sapi_header_line ctr = {0};
-int result;
+zend_result result;
 	smart_str buf = {0};
 
 	if (!ZSTR_LEN(name)) {
-zend_error( E_WARNING, "Cookie names must not be empty");
+zend_argument_value_error(1, "cannot be empty");
  return FAILURE;
-	} else if (strpbrk(ZSTR_VAL(name), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
- zend_error(E_WARNING, "Cookie names cannot contain any of the following '=,; \\t\\r\\n\\013\\014'" );
+	}
+	if (strpbrk(ZSTR_VAL(name), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
+ zend_argument_value_error(1, "cannot contain \"=\", " ILLEGAL_COOKIE_CHARACTER);
  return FAILURE;
 	}
-
 	if (!url_encode && value &&
  strpbrk(ZSTR_VAL(value), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
-zend_error(E_WARNING, "Cookie values cannot contain any of the following ',; \\t\\r\\n\\013\\014'" );
+zend_argument_value_error(2, "cannot contain " ILLEGAL_COOKIE_CHARACTER);
  return FAILURE;
 	}
 
 	if (path && strpbrk(ZSTR_VAL(path), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
- zend_error(E_WARNING, "Cookie paths cannot contain any of the following ',; \\t\\r\\n\\013\\014'" );
+ zend_value_error("%s(): \"path\" option cannot contain " ILLEGAL_COOKIE_CHARACTER,
+ get_active_function_name());
  return FAILURE;
 	}
-
 	if (domain && strpbrk(ZSTR_VAL(domain), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
- zend_error(E_WARNING, "Cookie domains cannot contain any of the following ',; \\t\\r\\n\\013\\014'" );
+ zend_value_error("%s(): \"domain\" option cannot contain " ILLEGAL_COOKIE_CHARACTER,
+ get_active_function_name());
  return FAILURE;
 	}
+	/* Should check value of SameSite? */
 
 	if (value == NULL || ZSTR_LEN(value) == 0) {
  /*
@@ -142,7 +147,8 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires,
  if (!p || *(p + 5) != ' ') {
  zend_string_free(dt);
  smart_str_free(&buf);
- zend_error(E_WARNING, "Expiry date cannot have a year greater than 9999");
+ zend_value_error("%s(): \"expires\" option cannot have a year greater than 9999",
+ get_active_function_name());
  return FAILURE;
  }
 
@@ -186,49 +192,40 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires,
 	return result;
 }
 
-static void php_head_parse_cookie_options_array(zval *options, zend_long *expires, zend_string **path, zend_string **domain, zend_bool *secure, zend_bool *httponly, zend_string **samesite) {
-	int found = 0;
+static zend_result php_head_parse_cookie_options_array(zval *options, zend_long *expires, zend_string **path,
+ zend_string **domain, zend_bool *secure, zend_bool *httponly, zend_string **samesite)
+{
 	zend_string *key;
 	zval *value;
 
 	ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(options), key, value) {
- if (key) {
- if (zend_string_equals_literal_ci(key, "expires")) {
- *expires = zval_get_long(value);
- found++;
- } else if (zend_string_equals_literal_ci(key, "path")) {
- *path = zval_get_string(value);
- found++;
- } else if (zend_string_equals_literal_ci(key, "domain")) {
- *domain = zval_get_string(value);
- found++;
- } else if (zend_string_equals_literal_ci(key, "secure")) {
- *secure = zval_is_true(value);
- found++;
- } else if (zend_string_equals_literal_ci(key, "httponly")) {
- *httponly = zval_is_true(value);
- found++;
- } else if (zend_string_equals_literal_ci(key, "samesite")) {
- *samesite = zval_get_string(value);
- found++;
- } else {
- php_error_docref(NULL, E_WARNING, "Unrecognized key '%s' found in the options array", ZSTR_VAL(key));
- }
+ if (!key) {
+ zend_value_error("%s(): option array cannot have numeric keys", get_active_function_name());
+ return FAILURE;
+ }
+ if (zend_string_equals_literal_ci(key, "expires")) {
+ *expires = zval_get_long(value);
+ } else if (zend_string_equals_literal_ci(key, "path")) {
+ *path = zval_get_string(value);
+ } else if (zend_string_equals_literal_ci(key, "domain")) {
+ *domain = zval_get_string(value);
+ } else if (zend_string_equals_literal_ci(key, "secure")) {
+ *secure = zval_is_true(value);
+ } else if (zend_string_equals_literal_ci(key, "httponly")) {
+ *httponly = zval_is_true(value);
+ } else if (zend_string_equals_literal_ci(key, "samesite")) {
+ *samesite = zval_get_string(value);
  } else {
- php_error_docref(NULL, E_WARNING, "Numeric key found in the options array");
+ zend_value_error("%s(): option \"%s\" is invalid", get_active_function_name(), ZSTR_VAL(key));
+ return FAILURE;
  }
 	} ZEND_HASH_FOREACH_END();
-
-	/* Array is not empty but no valid keys were found */
-	if (found == 0 && zend_hash_num_elements(Z_ARRVAL_P(options)) > 0) {
- php_error_docref(NULL, E_WARNING, "No valid options were found in the given array");
-	}
+	return SUCCESS;
 }
 
-/* {{{ setcookie(string name [, string value [, array options]])
- Send a cookie */
-PHP_FUNCTION(setcookie)
+static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
 {
+	/* to handle overloaded function array|int */
 	zval *expires_or_options = NULL;
 	zend_string *name, *value = NULL, *path = NULL, *domain = NULL, *samesite = NULL;
 	zend_long expires = 0;
@@ -248,24 +245,27 @@ PHP_FUNCTION(setcookie)
 	if (expires_or_options) {
  if (Z_TYPE_P(expires_or_options) == IS_ARRAY) {
  if (UNEXPECTED(ZEND_NUM_ARGS() > 3)) {
- php_error_docref(NULL, E_WARNING, "Cannot pass arguments after the options array");
- RETURN_FALSE;
+ zend_argument_count_error("%s(): Expects exactly 3 arguments when argument #3 "
+ "($expires_or_options) is an array", get_active_function_name());
+ RETURN_THROWS();
+ }
+ if (FAILURE == php_head_parse_cookie_options_array(expires_or_options, &expires, &path,
+ &domain, &secure, &httponly, &samesite)) {
+ goto cleanup;
  }
- php_head_parse_cookie_options_array(expires_or_options, &expires, &path, &domain, &secure, &httponly, &samesite);
  } else {
  expires = zval_get_long(expires_or_options);
  }
 	}
 
-	if (!EG(exception)) {
- if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, 1) == SUCCESS) {
- RETVAL_TRUE;
- } else {
- RETVAL_FALSE;
- }
+	if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, !is_raw) == SUCCESS) {
+ RETVAL_TRUE;
+	} else {
+ RETVAL_FALSE;
 	}
 
 	if (expires_or_options && Z_TYPE_P(expires_or_options) == IS_ARRAY) {
+ cleanup:
  if (path) {
  zend_string_release(path);
  }
@@ -277,59 +277,20 @@ PHP_FUNCTION(setcookie)
  }
 	}
 }
+
+/* {{{ setcookie(string name [, string value [, array options]])
+ Send a cookie */
+PHP_FUNCTION(setcookie)
+{
+	php_setcookie_common(INTERNAL_FUNCTION_PARAM_PASSTHRU, false);
+}
 /* }}} */
 
 /* {{{ setrawcookie(string name [, string value [, array options]])
  Send a cookie with no url encoding of the value */
 PHP_FUNCTION(setrawcookie)
 {
-	zval *expires_or_options = NULL;
-	zend_string *name, *value = NULL, *path = NULL, *domain = NULL, *samesite = NULL;
-	zend_long expires = 0;
-	zend_bool secure = 0, httponly = 0;
-
-	ZEND_PARSE_PARAMETERS_START(1, 7)
- Z_PARAM_STR(name)
- Z_PARAM_OPTIONAL
- Z_PARAM_STR(value)
- Z_PARAM_ZVAL(expires_or_options)
- Z_PARAM_STR(path)
- Z_PARAM_STR(domain)
- Z_PARAM_BOOL(secure)
- Z_PARAM_BOOL(httponly)
-	ZEND_PARSE_PARAMETERS_END();
-
-	if (expires_or_options) {
- if (Z_TYPE_P(expires_or_options) == IS_ARRAY) {
- if (UNEXPECTED(ZEND_NUM_ARGS() > 3)) {
- php_error_docref(NULL, E_WARNING, "Cannot pass arguments after the options array");
- RETURN_FALSE;
- }
- php_head_parse_cookie_options_array(expires_or_options, &expires, &path, &domain, &secure, &httponly, &samesite);
- } else {
- expires = zval_get_long(expires_or_options);
- }
-	}
-
-	if (!EG(exception)) {
- if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, 0) == SUCCESS) {
- RETVAL_TRUE;
- } else {
- RETVAL_FALSE;
- }
-	}
-
-	if (expires_or_options && Z_TYPE_P(expires_or_options) == IS_ARRAY) {
- if (path) {
- zend_string_release(path);
- }
- if (domain) {
- zend_string_release(domain);
- }
- if (samesite) {
- zend_string_release(samesite);
- }
-	}
+	php_setcookie_common(INTERNAL_FUNCTION_PARAM_PASSTHRU, true);
 }
 /* }}} */
 

ext/standard/head.h

@@ -28,6 +28,8 @@
 extern PHP_RINIT_FUNCTION(head);
 
 PHPAPI int php_header(void);
-PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain, int secure, int httponly, zend_string *samesite, int url_encode);
+PHPAPI zend_result php_setcookie(zend_string *name, zend_string *value, time_t expires,
+	zend_string *path, zend_string *domain, bool secure, bool httponly,
+	zend_string *samesite, bool url_encode);
 
 #endif

ext/standard/tests/network/bug69523.phpt

@@ -2,7 +2,11 @@
 setcookie() allows empty cookie name
 --FILE--
 <?php
-setcookie('', 'foo');
+try {
+ setcookie('', 'foo');
+} catch (\ValueError $e) {
+ echo $e->getMessage() . \PHP_EOL;
+}
 ?>
---EXPECTF--
-Warning: Cookie names must not be empty in %s on line %d
+--EXPECT--
+setcookie(): Argument #1 ($name) cannot be empty

ext/standard/tests/network/bug69948.phpt

@@ -2,17 +2,21 @@
 Bug #69948 (path/domain are not sanitized for special characters in setcookie)
 --FILE--
 <?php
-var_dump(
- setcookie('foo', 'bar', 0, 'asdf;asdf'),
- setcookie('foo', 'bar', 0, '/', 'foobar; secure')
-);
+try {
+ var_dump(setcookie('foo', 'bar', 0, 'asdf;asdf'));
+} catch (\ValueError $e) {
+ echo $e->getMessage() . \PHP_EOL;
+}
+try {
+ var_dump(setcookie('foo', 'bar', 0, '/', 'foobar; secure'));
+} catch (\ValueError $e) {
+ echo $e->getMessage() . \PHP_EOL;
+}
+
 ?>
 ===DONE===
 --EXPECTHEADERS--
---EXPECTF--
-Warning: Cookie paths cannot contain any of the following ',; \t\r\n\013\014' in %s on line %d
-
-Warning: Cookie domains cannot contain any of the following ',; \t\r\n\013\014' in %s on line %d
-bool(false)
-bool(false)
+--EXPECT--
+setcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
+setcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
 ===DONE===

ext/standard/tests/network/setcookie_array_option_error.phpt

@@ -0,0 +1,69 @@
+--TEST--
+setcookie() array variant error tests
+--INI--
+date.timezone=UTC
+--FILE--
+<?php
+
+ob_start();
+
+// Unrecognized key and no valid keys
+try {
+ setcookie('name', 'value', ['unknown_key' => 'only']);
+} catch (\ValueError $e) {
+ echo $e->getMessage() . "\n";
+}
+// Numeric key and no valid keys
+try {
+ setcookie('name2', 'value2', [0 => 'numeric_key']);
+} catch (\ValueError $e) {
+ echo $e->getMessage() . "\n";
+}
+// Unrecognized key
+try {
+ setcookie('name3', 'value3', ['path' => '/path/', 'foo' => 'bar']);
+} catch (\ValueError $e) {
+ echo $e->getMessage() . "\n";
+}
+// Invalid expiration date
+// To go above year 9999: 60 * 60 * 24 * 365 * 9999
+try {
+ setcookie('name', 'value', ['expires' => 315328464000]);
+} catch (\ValueError $e) {
+ echo $e->getMessage() . "\n";
+}
+// Invalid path key content
+try {
+ setcookie('name', 'value', ['path' => '/;/']);
+} catch (\ValueError $e) {
+ echo $e->getMessage() . "\n";
+}
+// Invalid domain key content
+try {
+ setcookie('name', 'value', ['path' => '/path/', 'domain' => 'ba;r']);
+} catch (\ValueError $e) {
+ echo $e->getMessage() . "\n";
+}
+
+// Arguments after options array (will not be set)
+try {
+ setcookie('name4', 'value4', [], "path", "domain.tld", true, true);
+} catch (\ArgumentCountError $e) {
+ echo $e->getMessage() . "\n";
+}
+
+var_dump(headers_list());
+--EXPECTHEADERS--
+
+--EXPECTF--
+setcookie(): option "unknown_key" is invalid
+setcookie(): option array cannot have numeric keys
+setcookie(): option "foo" is invalid
+setcookie(): "expires" option cannot have a year greater than 9999
+setcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
+setcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
+setcookie(): Expects exactly 3 arguments when argument #3 ($expires_or_options) is an array
+array(1) {
+ [0]=>
+ string(%s) "X-Powered-By: PHP/%s"
+}