changes from feedback

Author: devnexen

ext/session/session.c

@@ -2634,7 +2634,6 @@ PHP_FUNCTION(session_start)
 {
 	zval *options = NULL;
 	zval *value;
-	zend_ulong num_idx;
 	zend_string *str_idx;
 	bool read_and_close = false;
 
@@ -2660,19 +2659,27 @@ PHP_FUNCTION(session_start)
 	/* set options */
 	if (options) {
 		if (UNEXPECTED(HT_IS_PACKED(Z_ARRVAL_P(options)))) {
-			zend_argument_type_error(1, "must be of type array with keys as string");
-			RETURN_THROWS();
 		}
 
-		ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(options), num_idx, str_idx, value) {
+		ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(options), str_idx, value) {
 			if (str_idx) {
 				switch(Z_TYPE_P(value)) {
 					case IS_STRING:
 					case IS_TRUE:
 					case IS_FALSE:
 					case IS_LONG:
 						if (zend_string_equals_literal(str_idx, "read_and_close")) {
-							zend_long tmp = zval_get_long_ex(value, true);
+							zend_long tmp;
+							if (Z_TYPE_P(value) != IS_STRING) {
+						       		tmp = zval_get_long(value);
+							} else {
+								if (is_numeric_string(Z_STRVAL_P(value), Z_STRLEN_P(value), &tmp, NULL, false) != IS_LONG) {
+									zend_type_error("%s(): Option \"%s\" value must be of type compatible with int, \"%s\" given",
+											get_active_function_name(), ZSTR_VAL(str_idx), Z_STRVAL_P(value)
+										       );
+									RETURN_THROWS();
+								}
+							}
 							read_and_close = (tmp > 0);
 						} else {
 							zend_string *tmp_val;
@@ -2689,8 +2696,10 @@ PHP_FUNCTION(session_start)
 						);
 						RETURN_THROWS();
 				}
+			} else {
+				zend_argument_value_error(1, "must be of type array with keys as string");
+				RETURN_THROWS();
 			}
-			(void) num_idx;
 		} ZEND_HASH_FOREACH_END();
 	}
 

ext/session/tests/session_start_error.phpt

@@ -19,7 +19,7 @@ $read_and_close = "false";
 
 try {
     session_start([$read_and_close]);
-} catch (TypeError $exception) {
+} catch (ValueError $exception) {
     echo $exception->getMessage() . "\n";
 }
 

ext/session/tests/session_start_read_and_close.phpt

@@ -19,7 +19,11 @@ foreach ($valuesEnablingReadAndClose as $value) {
 }
 
 foreach ($valuesDisablingReadAndClose as $value) {
-    session_start(["read_and_close" => $value]);
+    try {
+    	session_start(["read_and_close" => $value]);
+    } catch (TypeError $e) {
+    	echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+    }
     var_dump(session_status() === PHP_SESSION_ACTIVE);
     session_write_close();
 }
@@ -43,9 +47,11 @@ bool(false)
 Notice: session_start(): Ignoring session_start() because a session is already active (started from %s on line %d) in %s on line %d
 bool(true)
 bool(true)
+TypeError: session_start(): Option "read_and_close" value must be of type compatible with int, "false" given
+bool(false)
 bool(true)
 bool(true)
-bool(true)
-bool(true)
+TypeError: session_start(): Option "read_and_close" value must be of type compatible with int, "no" given
+bool(false)
 bool(true)
 TypeError: session_start(): Option "read_and_close" must be of type string|int|bool, float given