ext/standard/dir.c: Refactor implementation of Directory and dir functions

ext/standard/dir.c

@@ -59,39 +59,6 @@ static zend_function *dir_class_get_constructor(zend_object *object)
 	return NULL;
 }
 
-#define FETCH_DIRP() \
-	myself = getThis(); \
-	if (!myself) { \
-		ZEND_PARSE_PARAMETERS_START(0, 1) \
-			Z_PARAM_OPTIONAL \
-			Z_PARAM_RESOURCE_OR_NULL(id) \
-		ZEND_PARSE_PARAMETERS_END(); \
-		if (id) { \
-			if ((dirp = (php_stream *)zend_fetch_resource(Z_RES_P(id), "Directory", php_file_le_stream())) == NULL) { \
-				RETURN_THROWS(); \
-			} \
-		} else { \
-			if (!DIRG(default_dir)) { \
-				zend_type_error("No resource supplied"); \
-				RETURN_THROWS(); \
-			} \
-			if ((dirp = (php_stream *)zend_fetch_resource(DIRG(default_dir), "Directory", php_file_le_stream())) == NULL) { \
-				RETURN_THROWS(); \
-			} \
-		} \
-	} else { \
-		ZEND_PARSE_PARAMETERS_NONE(); \
-		zval *handle_zv = Z_DIRECTORY_HANDLE_P(myself); \
-		if (Z_TYPE_P(handle_zv) != IS_RESOURCE) { \
-			zend_throw_error(NULL, "Unable to find my handle property"); \
-			RETURN_THROWS(); \
-		} \
-		if ((dirp = (php_stream *)zend_fetch_resource_ex(handle_zv, "Directory", php_file_le_stream())) == NULL) { \
-			RETURN_THROWS(); \
-		} \
-	}
-
-
 static void php_set_default_dir(zend_resource *res)
 {
 	if (DIRG(default_dir)) {
@@ -189,21 +156,71 @@ PHP_FUNCTION(dir)
 }
 /* }}} */
 
+
+static php_stream* php_dir_get_directory_stream_from_user_arg(zval *arg)
+{
+	zend_resource *res;
+	if (arg == NULL) {
+		if (UNEXPECTED(DIRG(default_dir) == NULL)) {
+			zend_type_error("No resource supplied");
+			return NULL;
+		}
+		res = DIRG(default_dir);
+	} else {
+		ZEND_ASSERT(Z_TYPE_P(arg) == IS_RESOURCE);
+		res = Z_RES_P(arg);
+	}
+
+	if (UNEXPECTED(res->type != php_file_le_stream())) {
+		zend_argument_type_error(1, "must be a valid Directory resource");
+		return NULL;
+	}
+	php_stream *dir_stream = (php_stream*) res->ptr;
+	if (UNEXPECTED((dir_stream->flags & PHP_STREAM_FLAG_IS_DIR)) == 0) {
+		zend_argument_type_error(1, "must be a valid Directory resource");
+		return NULL;
+	}
+	return dir_stream;
+}
+
+static php_stream* php_dir_get_directory_stream_from_this(zval *this_z)
+{
+	zval *handle_zv = Z_DIRECTORY_HANDLE_P(this_z);
+	if (UNEXPECTED(Z_TYPE_P(handle_zv) != IS_RESOURCE)) {
+		zend_throw_error(NULL, "internal directory stream has been altered");
+		return NULL;
+	}
+	zend_resource *res = Z_RES_P(handle_zv);
+	/* Assume the close() method was called
+	 * (instead of the hacky case where a different resource would have been set via the ArrayObject "hack") */
+	if (UNEXPECTED(res->type != php_file_le_stream())) {
+		/* TypeError is used for BC, TODO: Use base Error in PHP 9 */
+		zend_type_error("Directory::%s(): cannot use Directory resource after it has been closed", get_active_function_name());
+		return NULL;
+	}
+	php_stream *dir_stream = (php_stream*) res->ptr;
+	if (UNEXPECTED((dir_stream->flags & PHP_STREAM_FLAG_IS_DIR)) == 0) {
+		zend_throw_error(NULL, "internal directory stream has been altered");
+		return NULL;
+	}
+	return dir_stream;
+}
+
 /* {{{ Close directory connection identified by the dir_handle */
 PHP_FUNCTION(closedir)
 {
-	zval *id = NULL, *myself;
-	php_stream *dirp;
-	zend_resource *res;
+	zval *id = NULL;
 
-	FETCH_DIRP();
+	ZEND_PARSE_PARAMETERS_START(0, 1)
+		Z_PARAM_OPTIONAL
+		Z_PARAM_RESOURCE_OR_NULL(id)
+	ZEND_PARSE_PARAMETERS_END();
 
-	if (!(dirp->flags & PHP_STREAM_FLAG_IS_DIR)) {
-		zend_argument_type_error(1, "must be a valid Directory resource");
+	php_stream *dirp = php_dir_get_directory_stream_from_user_arg(id);
+	if (UNEXPECTED(dirp == NULL)) {
 		RETURN_THROWS();
 	}
-
-	res = dirp->res;
+	zend_resource *res = dirp->res;
 	zend_list_close(dirp->res);
 
 	if (res == DIRG(default_dir)) {
@@ -212,6 +229,93 @@ PHP_FUNCTION(closedir)
 }
 /* }}} */
 
+PHP_METHOD(Directory, close)
+{
+	ZEND_PARSE_PARAMETERS_NONE();
+
+	php_stream *dirp = php_dir_get_directory_stream_from_this(ZEND_THIS);
+	if (UNEXPECTED(dirp == NULL)) {
+		RETURN_THROWS();
+	}
+
+	zend_resource *res = dirp->res;
+	zend_list_close(res);
+
+	if (res == DIRG(default_dir)) {
+		php_set_default_dir(NULL);
+	}
+}
+
+/* {{{ Rewind dir_handle back to the start */
+PHP_FUNCTION(rewinddir)
+{
+	zval *id = NULL;
+
+	ZEND_PARSE_PARAMETERS_START(0, 1)
+		Z_PARAM_OPTIONAL
+		Z_PARAM_RESOURCE_OR_NULL(id)
+	ZEND_PARSE_PARAMETERS_END();
+
+	php_stream *dirp = php_dir_get_directory_stream_from_user_arg(id);
+	if (UNEXPECTED(dirp == NULL)) {
+		RETURN_THROWS();
+	}
+
+	php_stream_rewinddir(dirp);
+}
+/* }}} */
+
+PHP_METHOD(Directory, rewind)
+{
+	ZEND_PARSE_PARAMETERS_NONE();
+
+	php_stream *dirp = php_dir_get_directory_stream_from_this(ZEND_THIS);
+	if (UNEXPECTED(dirp == NULL)) {
+		RETURN_THROWS();
+	}
+
+	php_stream_rewinddir(dirp);
+}
+
+/* {{{ Read directory entry from dir_handle */
+PHP_FUNCTION(readdir)
+{
+	zval *id = NULL;
+
+	ZEND_PARSE_PARAMETERS_START(0, 1)
+		Z_PARAM_OPTIONAL
+		Z_PARAM_RESOURCE_OR_NULL(id)
+	ZEND_PARSE_PARAMETERS_END();
+
+	php_stream *dirp = php_dir_get_directory_stream_from_user_arg(id);
+	if (UNEXPECTED(dirp == NULL)) {
+		RETURN_THROWS();
+	}
+
+	php_stream_dirent entry;
+	if (php_stream_readdir(dirp, &entry)) {
+		RETURN_STRINGL(entry.d_name, strlen(entry.d_name));
+	}
+	RETURN_FALSE;
+}
+/* }}} */
+
+PHP_METHOD(Directory, read)
+{
+	ZEND_PARSE_PARAMETERS_NONE();
+
+	php_stream *dirp = php_dir_get_directory_stream_from_this(ZEND_THIS);
+	if (UNEXPECTED(dirp == NULL)) {
+		RETURN_THROWS();
+	}
+
+	php_stream_dirent entry;
+	if (php_stream_readdir(dirp, &entry)) {
+		RETURN_STRINGL(entry.d_name, strlen(entry.d_name));
+	}
+	RETURN_FALSE;
+}
+
 #if defined(HAVE_CHROOT) && !defined(ZTS) && defined(ENABLE_CHROOT_FUNC)
 /* {{{ Change root directory */
 PHP_FUNCTION(chroot)
@@ -300,44 +404,6 @@ PHP_FUNCTION(getcwd)
 }
 /* }}} */
 
-/* {{{ Rewind dir_handle back to the start */
-PHP_FUNCTION(rewinddir)
-{
-	zval *id = NULL, *myself;
-	php_stream *dirp;
-
-	FETCH_DIRP();
-
-	if (!(dirp->flags & PHP_STREAM_FLAG_IS_DIR)) {
-		zend_argument_type_error(1, "must be a valid Directory resource");
-		RETURN_THROWS();
-	}
-
-	php_stream_rewinddir(dirp);
-}
-/* }}} */
-
-/* {{{ Read directory entry from dir_handle */
-PHP_FUNCTION(readdir)
-{
-	zval *id = NULL, *myself;
-	php_stream *dirp;
-	php_stream_dirent entry;
-
-	FETCH_DIRP();
-
-	if (!(dirp->flags & PHP_STREAM_FLAG_IS_DIR)) {
-		zend_argument_type_error(1, "must be a valid Directory resource");
-		RETURN_THROWS();
-	}
-
-	if (php_stream_readdir(dirp, &entry)) {
-		RETURN_STRINGL(entry.d_name, strlen(entry.d_name));
-	}
-	RETURN_FALSE;
-}
-/* }}} */
-
 #ifdef HAVE_GLOB
 /* {{{ Find pathnames matching a pattern */
 PHP_FUNCTION(glob)

ext/standard/dir.stub.php

@@ -98,18 +98,9 @@ final class Directory
     /** @var resource */
     public readonly mixed $handle;
 
-    /**
-     * @implementation-alias closedir
-     */
     public function close(): void {}
 
-    /**
-     * @implementation-alias rewinddir
-     */
     public function rewind(): void {}
 
-    /**
-     * @implementation-alias readdir
-     */
     public function read(): string|false {}
 }

ext/standard/tests/dir/closedir_variation2-win32-mb.phpt

@@ -47,5 +47,5 @@ NULL
 Directory Handle: resource(%d) of type (Unknown)
 
 -- Close directory handle second time: --
-closedir(): %s is not a valid Directory resource
+closedir(): Argument #1 ($dir_handle) must be a valid Directory resource
 Directory Handle: resource(%d) of type (Unknown)

ext/standard/tests/dir/closedir_variation2.phpt

@@ -41,5 +41,5 @@ NULL
 Directory Handle: resource(%d) of type (Unknown)
 
 -- Close directory handle second time: --
-closedir(): supplied resource is not a valid Directory resource
+closedir(): Argument #1 ($dir_handle) must be a valid Directory resource
 Directory Handle: resource(%d) of type (Unknown)

ext/standard/tests/dir/dir_basic-win32-mb.phpt

@@ -85,5 +85,5 @@ object(Directory)#%d (2) {
 }
 
 Test read after closing the dir:
-Directory::read(): %s is not a valid Directory resource
+Directory::read(): cannot use Directory resource after it has been closed
 Done

ext/standard/tests/dir/dir_basic.phpt

@@ -79,5 +79,5 @@ object(Directory)#%d (2) {
 }
 
 Test read after closing the dir:
-Directory::read(): supplied resource is not a valid Directory resource
+Directory::read(): cannot use Directory resource after it has been closed
 Done

ext/standard/tests/dir/rewinddir_variation2-win32-mb.phpt

@@ -42,4 +42,4 @@ resource(%d) of type (stream)
 string(%d) "%s"
 
 -- Call to rewinddir() --
-rewinddir(): %s is not a valid Directory resource
+rewinddir(): Argument #1 ($dir_handle) must be a valid Directory resource

ext/standard/tests/dir/rewinddir_variation2.phpt

@@ -36,4 +36,4 @@ resource(%d) of type (stream)
 string(%d) "%s"
 
 -- Call to rewinddir() --
-rewinddir(): supplied resource is not a valid Directory resource
+rewinddir(): Argument #1 ($dir_handle) must be a valid Directory resource

ext/standard/tests/directory/DirectoryClass_readonly_handle_by_pass_via_ArrayObject.phpt

@@ -0,0 +1,34 @@
+--TEST--
+Changing Directory::$handle property
+--FILE--
+<?php
+
+$d = dir(__DIR__);
+$ao = new ArrayObject($d);
+
+try {
+    $ao['handle'] = STDERR;
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+var_dump($d->handle);
+
+try {
+    $s = $d->read();
+    var_dump($s);
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    unset($ao['handle']);
+    var_dump($d->handle);
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+?>
+--EXPECT--
+resource(3) of type (stream)
+Error: internal directory stream has been altered
+Error: Typed property Directory::$handle must not be accessed before initialization

ext/standard/tests/directory/DirectoryClass_reflection_create_instance_no_construct.phpt

@@ -27,4 +27,4 @@ object(Directory)#2 (0) {
   ["handle"]=>
   uninitialized(mixed)
 }
-Error: Unable to find my handle property
+Error: internal directory stream has been altered