From 65eb26842591b10087804c77d9df200ac2345f13 Mon Sep 17 00:00:00 2001
From: Timm Friebe <thekid@thekid.de>
Date: Sun, 5 Jan 2025 22:28:14 +0100
Subject: [PATCH] Prevent double-closing file when serving partial content

---
 ChangeLog.md                                  |  3 ++
 src/main/php/web/io/StaticContent.class.php   |  2 +-
 .../unittest/io/StaticContentTest.class.php   | 33 +++++++++++++++++--
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/ChangeLog.md b/ChangeLog.md
index a9543428..a7e4eca7 100644
--- a/ChangeLog.md
+++ b/ChangeLog.md
@@ -3,6 +3,9 @@ Web change log
 
 ## ?.?.? / ????-??-??
 
+* Added logic to prevent double-closing file when serving partial content
+  (@thekid)
+
 ## 4.5.1 / 2024-09-29
 
 * Fixed error *[] operator not supported for strings* when handling array
diff --git a/src/main/php/web/io/StaticContent.class.php b/src/main/php/web/io/StaticContent.class.php
index f5aa6d0f..045f12e0 100755
--- a/src/main/php/web/io/StaticContent.class.php
+++ b/src/main/php/web/io/StaticContent.class.php
@@ -153,7 +153,7 @@ public function serve($request, $response, $target, $mimeType= null) {
         $out->write($trailer);
       }
     } finally {
-      $file->close();
+      $file->isOpen() && $file->close();
       $out->close();
     }
   }
diff --git a/src/test/php/web/unittest/io/StaticContentTest.class.php b/src/test/php/web/unittest/io/StaticContentTest.class.php
index 89430759..656ff7b5 100755
--- a/src/test/php/web/unittest/io/StaticContentTest.class.php
+++ b/src/test/php/web/unittest/io/StaticContentTest.class.php
@@ -15,14 +15,15 @@ class StaticContentTest {
    * @param  ?io.File $file
    * @param  ?string $mimeType
    * @param  ?web.io.TestInput $input
+   * @param  function(Iterator): void $writer
    * @return string
    */
-  private function serve($content, $file, $mimeType= null, $input= null) {
+  private function serve($content, $file, $mimeType= null, $input= null, $writer= 'iterator_count') {
     $res= new Response(new TestOutput());
     $req= new Request($input ?? new TestInput('GET', '/'));
 
     try {
-      foreach ($content->serve($req, $res, $file, $mimeType) ?? [] as $_) { }
+      $writer($content->serve($req, $res, $file, $mimeType));
     } finally {
       $res->end();
     }
@@ -274,4 +275,32 @@ public function range_unsatisfiable($range) {
       ]))
     );
   }
+
+  #[Test]
+  public function file_closed_after_first_chunk() {
+    $chunk= StaticContent::CHUNKSIZE;
+    $headers= ['Range' => "bytes=0-{$chunk}"];
+    $file= (new TempFile(self::class))->containing(str_repeat('*', $chunk + 1));
+
+    // Close files after having written
+    $writer= function($it) use($file) {
+      $it->next();
+      $file->close();
+      while ($it->valid()) $it->next();
+    };
+
+    // Assert first chunk has been written
+    Assert::equals(
+      "HTTP/1.1 206 Partial Content\r\n".
+      "Accept-Ranges: bytes\r\n".
+      "Last-Modified: <Date>\r\n".
+      "X-Content-Type-Options: nosniff\r\n".
+      "Content-Type: application/octet-stream\r\n".
+      "Content-Range: bytes 0-{$chunk}/".($chunk + 1)."\r\n".
+      "Content-Length: ".($chunk + 1)."\r\n".
+      "\r\n".
+      str_repeat('*', $chunk + 1),
+      $this->serve(new StaticContent(), $file, null, new TestInput('GET', '/', $headers), $writer)
+    );
+  }
 }
\ No newline at end of file