From dadceea8f585dfe54709a6ca500b9715e1a080ae Mon Sep 17 00:00:00 2001
From: sam marshall <s.marshall@open.ac.uk>
Date: Mon, 6 Feb 2023 15:21:46 +0000
Subject: [PATCH] MDL-77149 core_files: Network filesystem (Amazon EFS) can
 warn

If you delete a file with a hash and then create another file with
the same hash, sometimes on EFS filesystems while trying to create
the new file, it returns true to the file_exists check even though
the file doesn't exist, but then fails other calls.

This change makes Moodle tolerate that behaviour.
---
 lib/filestorage/file_system_filedir.php | 46 ++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/lib/filestorage/file_system_filedir.php b/lib/filestorage/file_system_filedir.php
index 19418514937..75338755316 100644
--- a/lib/filestorage/file_system_filedir.php
+++ b/lib/filestorage/file_system_filedir.php
@@ -352,8 +352,9 @@ class file_system_filedir extends file_system {
 
         $newfile = true;
 
-        if (file_exists($hashfile)) {
-            if (filesize($hashfile) === $filesize) {
+        $hashsize = self::check_file_exists_and_get_size($hashfile);
+        if ($hashsize !== null) {
+            if ($hashsize === $filesize) {
                 return array($contenthash, $filesize, false);
             }
             if (file_storage::hash_from_path($hashfile) === $contenthash) {
@@ -411,6 +412,42 @@ class file_system_filedir extends file_system {
         return array($contenthash, $filesize, $newfile);
     }
 
+    /**
+     * Checks if the file exists and gets its size. This function avoids a specific issue with
+     * networked file systems if they incorrectly report the file exists, but then decide it doesn't
+     * as soon as you try to get the file size.
+     *
+     * @param string $hashfile File to check
+     * @return int|null Null if the file does not exist, or the result of filesize(), or -1 if error
+     */
+    protected static function check_file_exists_and_get_size(string $hashfile): ?int {
+        if (!file_exists($hashfile)) {
+            // The file does not exist, return null.
+            return null;
+        }
+
+        // In some networked file systems, it's possible that file_exists will return true when
+        // the file doesn't exist (due to caching), but filesize will then return false because
+        // it doesn't exist.
+        $hashsize = @filesize($hashfile);
+        if ($hashsize !== false) {
+            // We successfully got a file size. Return it.
+            return $hashsize;
+        }
+
+        // If we can't get the filesize, let's check existence again to see if we really
+        // for sure think it exists.
+        clearstatcache();
+        if (!file_exists($hashfile)) {
+            // The file doesn't exist any more, so return null.
+            return null;
+        }
+
+        // It still thinks the file exists, but filesize failed, so we had better return an invalid
+        // value for filesize.
+        return -1;
+    }
+
     /**
      * Add a file with the supplied content to the file system.
      *
@@ -433,8 +470,9 @@ class file_system_filedir extends file_system {
 
         $newfile = true;
 
-        if (file_exists($hashfile)) {
-            if (filesize($hashfile) === $filesize) {
+        $hashsize = self::check_file_exists_and_get_size($hashfile);
+        if ($hashsize !== null) {
+            if ($hashsize === $filesize) {
                 return array($contenthash, $filesize, false);
             }
             if (file_storage::hash_from_path($hashfile) === $contenthash) {