From 0f45c4958d686e99cd774a18b9bba4d6358dbe8b Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Wed, 31 Mar 2021 18:06:57 +0200 Subject: [PATCH 1/6] Adjust test to be able to handle async file handling This also simplifies the unittest code:) --- .../unittest/handler/FilesFromTest.class.php | 129 ++++++------------ 1 file changed, 44 insertions(+), 85 deletions(-) diff --git a/src/test/php/web/unittest/handler/FilesFromTest.class.php b/src/test/php/web/unittest/handler/FilesFromTest.class.php index 9590db01..5e5874ba 100755 --- a/src/test/php/web/unittest/handler/FilesFromTest.class.php +++ b/src/test/php/web/unittest/handler/FilesFromTest.class.php @@ -56,6 +56,24 @@ private function assertResponse($expected, $response) { )); } + /** + * Returns + * + * @param web.handler.FilesFrom $files + * @param web.Request $req + * @return web.Response + */ + private function handle($files, $req) { + $res= new Response(new TestOutput()); + + try { + foreach ($files->handle($req, $res) ?? [] as $_) { } + return $res; + } finally { + $res->end(); + } + } + /** @return void */ public function tearDown() { foreach ($this->cleanup as $folder) { @@ -70,12 +88,7 @@ public function can_create() { #[Test] public function existing_file() { - $req= new Request(new TestInput('GET', '/test.html')); - $res= new Response(new TestOutput()); - - $files= (new FilesFrom($this->pathWith(['test.html' => 'Test']))); - $files->handle($req, $res); - + $files= new FilesFrom($this->pathWith(['test.html' => 'Test'])); $this->assertResponse( "HTTP/1.1 200 OK\r\n". "Accept-Ranges: bytes\r\n". @@ -85,33 +98,25 @@ public function existing_file() { "Content-Length: 4\r\n". "\r\n". "Test", - $res + $this->handle($files, new Request(new TestInput('GET', '/test.html'))) ); } #[Test] public function existing_file_unmodified_since() { - $req= new Request(new TestInput('GET', '/test.html', ['If-Modified-Since' => gmdate('D, d M Y H:i:s T', time() + 1)])); - $res= new Response(new TestOutput()); - - $files= (new FilesFrom($this->pathWith(['test.html' => 'Test']))); - $files->handle($req, $res); - + $files= new FilesFrom($this->pathWith(['test.html' => 'Test'])); $this->assertResponse( "HTTP/1.1 304 Not Modified\r\n". "\r\n", - $res + $this->handle($files, new Request(new TestInput('GET', '/test.html', [ + 'If-Modified-Since' => gmdate('D, d M Y H:i:s T', time() + 1) + ]))) ); } #[Test] public function index_html() { - $req= new Request(new TestInput('GET', '/')); - $res= new Response(new TestOutput()); - - $files= (new FilesFrom($this->pathWith(['index.html' => 'Home']))); - $files->handle($req, $res); - + $files= new FilesFrom($this->pathWith(['index.html' => 'Home'])); $this->assertResponse( "HTTP/1.1 200 OK\r\n". "Accept-Ranges: bytes\r\n". @@ -121,89 +126,63 @@ public function index_html() { "Content-Length: 4\r\n". "\r\n". "Home", - $res + $this->handle($files, new Request(new TestInput('GET', '/'))) ); } #[Test] public function redirect_if_trailing_slash_missing() { - $req= new Request(new TestInput('GET', '/preview')); - $res= new Response(new TestOutput()); - - $files= (new FilesFrom($this->pathWith(['preview' => ['index.html' => 'Home']]))); - $files->handle($req, $res); - + $files= new FilesFrom($this->pathWith(['preview' => ['index.html' => 'Home']])); $this->assertResponse( "HTTP/1.1 301 Moved Permanently\r\n". "Location: preview/\r\n". "\r\n", - $res + $this->handle($files, new Request(new TestInput('GET', '/preview'))) ); } #[Test] public function non_existant_file() { - $req= new Request(new TestInput('GET', '/test.html')); - $res= new Response(new TestOutput()); - - $files= (new FilesFrom($this->pathWith([]))); - $files->handle($req, $res); - + $files= new FilesFrom($this->pathWith([])); $this->assertResponse( "HTTP/1.1 404 Not Found\r\n". "Content-Type: text/plain\r\n". "Content-Length: 35\r\n". "\r\n". "The file '/test.html' was not found", - $res + $this->handle($files, new Request(new TestInput('GET', '/test.html'))) ); } #[Test] public function non_existant_index_html() { - $req= new Request(new TestInput('GET', '/')); - $res= new Response(new TestOutput()); - - $files= (new FilesFrom($this->pathWith([]))); - $files->handle($req, $res); - + $files= new FilesFrom($this->pathWith([])); $this->assertResponse( "HTTP/1.1 404 Not Found\r\n". "Content-Type: text/plain\r\n". "Content-Length: 26\r\n". "\r\n". "The file '/' was not found", - $res + $this->handle($files, new Request(new TestInput('GET', '/'))) ); } #[Test, Values(['/../credentials', '/static/../../credentials'])] public function cannot_access_below_path_root($uri) { - $req= new Request(new TestInput('GET', $uri)); - $res= new Response(new TestOutput()); - - $path= $this->pathWith(['credentials' => 'secret']); - $files= new FilesFrom(new Folder($path, 'webroot')); - $files->handle($req, $res); - + $files= new FilesFrom(new Folder($this->pathWith(['credentials' => 'secret']), 'webroot')); $this->assertResponse( "HTTP/1.1 404 Not Found\r\n". "Content-Type: text/plain\r\n". "Content-Length: 37\r\n". "\r\n". "The file '/credentials' was not found", - $res + $this->handle($files, new Request(new TestInput('GET', $uri))) ); } #[Test, Values([['0-3', 'Home'], ['4-7', 'page'], ['0-0', 'H'], ['4-4', 'p'], ['7-7', 'e']])] public function range_with_start_and_end($range, $result) { - $req= new Request(new TestInput('GET', '/', ['Range' => 'bytes='.$range])); - $res= new Response(new TestOutput()); - - $files= (new FilesFrom($this->pathWith(['index.html' => 'Homepage']))); - $files->handle($req, $res); - + $files= new FilesFrom($this->pathWith(['index.html' => 'Homepage'])); $this->assertResponse( "HTTP/1.1 206 Partial Content\r\n". "Accept-Ranges: bytes\r\n". @@ -214,18 +193,13 @@ public function range_with_start_and_end($range, $result) { "Content-Length: ".strlen($result)."\r\n". "\r\n". $result, - $res + $this->handle($files, new Request(new TestInput('GET', '/', ['Range' => 'bytes='.$range]))) ); } #[Test] public function range_from_offset_until_end() { - $req= new Request(new TestInput('GET', '/', ['Range' => 'bytes=4-'])); - $res= new Response(new TestOutput()); - - $files= (new FilesFrom($this->pathWith(['index.html' => 'Homepage']))); - $files->handle($req, $res); - + $files= new FilesFrom($this->pathWith(['index.html' => 'Homepage'])); $this->assertResponse( "HTTP/1.1 206 Partial Content\r\n". "Accept-Ranges: bytes\r\n". @@ -236,18 +210,13 @@ public function range_from_offset_until_end() { "Content-Length: 4\r\n". "\r\n". "page", - $res + $this->handle($files, new Request(new TestInput('GET', '/', ['Range' => 'bytes=4-']))) ); } #[Test, Values([0, 8192, 10000])] public function range_last_four_bytes($offset) { - $req= new Request(new TestInput('GET', '/', ['Range' => 'bytes=-4'])); - $res= new Response(new TestOutput()); - - $files= (new FilesFrom($this->pathWith(['index.html' => str_repeat('*', $offset).'Homepage']))); - $files->handle($req, $res); - + $files= new FilesFrom($this->pathWith(['index.html' => str_repeat('*', $offset).'Homepage'])); $this->assertResponse( "HTTP/1.1 206 Partial Content\r\n". "Accept-Ranges: bytes\r\n". @@ -258,18 +227,13 @@ public function range_last_four_bytes($offset) { "Content-Length: 4\r\n". "\r\n". "page", - $res + $this->handle($files, new Request(new TestInput('GET', '/', ['Range' => 'bytes=-4']))) ); } #[Test, Values(['bytes=0-2000', 'bytes=4-2000', 'bytes=2000-', 'bytes=2000-2001', 'bytes=2000-0', 'bytes=4-0', 'characters=0-'])] public function range_unsatisfiable($range) { - $req= new Request(new TestInput('GET', '/', ['Range' => $range])); - $res= new Response(new TestOutput()); - - $files= (new FilesFrom($this->pathWith(['index.html' => 'Homepage']))); - $files->handle($req, $res); - + $files= new FilesFrom($this->pathWith(['index.html' => 'Homepage'])); $this->assertResponse( "HTTP/1.1 416 Range Not Satisfiable\r\n". "Accept-Ranges: bytes\r\n". @@ -277,18 +241,13 @@ public function range_unsatisfiable($range) { "X-Content-Type-Options: nosniff\r\n". "Content-Range: bytes */8\r\n". "\r\n", - $res + $this->handle($files, new Request(new TestInput('GET', '/', ['Range' => $range]))) ); } #[Test] public function multi_range() { - $req= new Request(new TestInput('GET', '/', ['Range' => 'bytes=0-3,4-7'])); - $res= new Response(new TestOutput()); - - $files= (new FilesFrom($this->pathWith(['index.html' => 'Homepage']))); - $files->handle($req, $res); - + $files= new FilesFrom($this->pathWith(['index.html' => 'Homepage'])); $this->assertResponse( "HTTP/1.1 206 Partial Content\r\n". "Accept-Ranges: bytes\r\n". @@ -306,7 +265,7 @@ public function multi_range() { "Content-Range: bytes 4-7/8\r\n\r\n". "page". "\r\n--594fa07300f865fe--\r\n", - $res + $this->handle($files, new Request(new TestInput('GET', '/', ['Range' => 'bytes=0-3,4-7']))) ); } } \ No newline at end of file From 63fc3cb37f4bd4d06fceb4f030e6127a983c12ad Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Wed, 31 Mar 2021 18:08:04 +0200 Subject: [PATCH 2/6] Yield control after transferring each chunk of the given file --- src/main/php/web/handler/FilesFrom.class.php | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/main/php/web/handler/FilesFrom.class.php b/src/main/php/web/handler/FilesFrom.class.php index 07baac45..497b24bc 100755 --- a/src/main/php/web/handler/FilesFrom.class.php +++ b/src/main/php/web/handler/FilesFrom.class.php @@ -45,7 +45,7 @@ public function handle($request, $response) { $file= $target->asFile(); } - $this->serve($request, $response, $file); + return $this->serve($request, $response, $file); } /** @@ -54,11 +54,13 @@ public function handle($request, $response) { * @param web.io.Output $output * @param io.File $file * @param int $length + * @return iterable */ private function copy($output, $file, $length) { while ($length && $chunk= $file->read(min(8192, $length))) { $output->write($chunk); $length-= strlen($chunk); + yield; } } @@ -94,7 +96,14 @@ public function serve($request, $response, $target) { $mimeType= MimeType::getByFileName($file->filename); if (null === ($ranges= Ranges::in($request->header('Range'), $file->size()))) { $response->answer(200, 'OK'); - $response->transfer($file->in(), $mimeType, $file->size()); + $response->header('Content-Type', $mimeType); + + $out= $response->stream($file->size()); + $file->open(File::READ); + do { + $out->write($file->read()); + yield; + } while (!$file->eof()); return; } @@ -117,7 +126,7 @@ public function serve($request, $response, $target) { $file->seek($range->start()); $response->flush(); - $this->copy($output, $file, $range->length()); + yield from $this->copy($output, $file, $range->length()); } else { $headers= []; $trailer= "\r\n--".self::BOUNDARY."--\r\n"; @@ -140,7 +149,7 @@ public function serve($request, $response, $target) { foreach ($ranges->sets() as $i => $range) { $output->write($headers[$i]); $file->seek($range->start()); - $this->copy($output, $file, $range->length()); + yield from $this->copy($output, $file, $range->length()); } $output->write($trailer); } From 47c0d957ba8b25d950f6fbf15d60bd2fc566c5e6 Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Wed, 31 Mar 2021 18:11:48 +0200 Subject: [PATCH 3/6] Simplify code by passing range to copy() --- src/main/php/web/handler/FilesFrom.class.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/php/web/handler/FilesFrom.class.php b/src/main/php/web/handler/FilesFrom.class.php index 497b24bc..22606326 100755 --- a/src/main/php/web/handler/FilesFrom.class.php +++ b/src/main/php/web/handler/FilesFrom.class.php @@ -53,10 +53,13 @@ public function handle($request, $response) { * * @param web.io.Output $output * @param io.File $file - * @param int $length + * @param web.io.Range $range * @return iterable */ - private function copy($output, $file, $length) { + private function copy($output, $file, $range) { + $file->seek($range->start()); + + $length= $range->length(); while ($length && $chunk= $file->read(min(8192, $length))) { $output->write($chunk); $length-= strlen($chunk); @@ -124,9 +127,8 @@ public function serve($request, $response, $target) { $response->header('Content-Range', $ranges->format($range)); $response->header('Content-Length', $range->length()); - $file->seek($range->start()); $response->flush(); - yield from $this->copy($output, $file, $range->length()); + yield from $this->copy($output, $file, $range); } else { $headers= []; $trailer= "\r\n--".self::BOUNDARY."--\r\n"; @@ -148,8 +150,7 @@ public function serve($request, $response, $target) { $response->flush(); foreach ($ranges->sets() as $i => $range) { $output->write($headers[$i]); - $file->seek($range->start()); - yield from $this->copy($output, $file, $range->length()); + yield from $this->copy($output, $file, $range); } $output->write($trailer); } From a3d85e31cb927c14ff9720e53d9c6ed281100204 Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Wed, 31 Mar 2021 18:12:32 +0200 Subject: [PATCH 4/6] Ensure file is closed --- src/main/php/web/handler/FilesFrom.class.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/php/web/handler/FilesFrom.class.php b/src/main/php/web/handler/FilesFrom.class.php index 22606326..91c01bfd 100755 --- a/src/main/php/web/handler/FilesFrom.class.php +++ b/src/main/php/web/handler/FilesFrom.class.php @@ -102,11 +102,15 @@ public function serve($request, $response, $target) { $response->header('Content-Type', $mimeType); $out= $response->stream($file->size()); - $file->open(File::READ); - do { - $out->write($file->read()); - yield; - } while (!$file->eof()); + try { + $file->open(File::READ); + do { + $out->write($file->read()); + yield; + } while (!$file->eof()); + } finally { + $file->close(); + } return; } From 835ef55a73847d691c0451af3a539a1354fbad0b Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Wed, 31 Mar 2021 18:19:03 +0200 Subject: [PATCH 5/6] Ensure file and output are closed --- src/main/php/web/handler/FilesFrom.class.php | 27 +++++++++----------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/main/php/web/handler/FilesFrom.class.php b/src/main/php/web/handler/FilesFrom.class.php index 91c01bfd..d50ebdab 100755 --- a/src/main/php/web/handler/FilesFrom.class.php +++ b/src/main/php/web/handler/FilesFrom.class.php @@ -102,14 +102,15 @@ public function serve($request, $response, $target) { $response->header('Content-Type', $mimeType); $out= $response->stream($file->size()); + $file->open(File::READ); try { - $file->open(File::READ); do { $out->write($file->read()); yield; } while (!$file->eof()); } finally { $file->close(); + $out->close(); } return; } @@ -122,45 +123,41 @@ public function serve($request, $response, $target) { } $file->open(File::READ); - $output= $response->output(); $response->answer(206, 'Partial Content'); try { if ($range= $ranges->single()) { $response->header('Content-Type', $mimeType); $response->header('Content-Range', $ranges->format($range)); - $response->header('Content-Length', $range->length()); - $response->flush(); - yield from $this->copy($output, $file, $range); + $out= $response->stream($range->length()); + yield from $this->copy($out, $file, $range); } else { $headers= []; $trailer= "\r\n--".self::BOUNDARY."--\r\n"; - $length= strlen($trailer); + foreach ($ranges->sets() as $i => $range) { - $header= sprintf( + $headers[$i]= $header= sprintf( "\r\n--%s\r\nContent-Type: %s\r\nContent-Range: %s\r\n\r\n", self::BOUNDARY, $mimeType, $ranges->format($range) ); - $headers[$i]= $header; $length+= strlen($header) + $range->length(); } - $response->header('Content-Type', 'multipart/byteranges; boundary='.self::BOUNDARY); - $response->header('Content-Length', $length); - $response->flush(); + + $out= $response->stream($length); foreach ($ranges->sets() as $i => $range) { - $output->write($headers[$i]); - yield from $this->copy($output, $file, $range); + $out->write($headers[$i]); + yield from $this->copy($out, $file, $range); } - $output->write($trailer); + $out->write($trailer); } } finally { $file->close(); - $output->close(); + $out->close(); } } } \ No newline at end of file From 01d4717b127f21442db112776adc4e86fb2d0abd Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Wed, 31 Mar 2021 18:49:09 +0200 Subject: [PATCH 6/6] Use the same chunk size for all transfers --- src/main/php/web/handler/FilesFrom.class.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/php/web/handler/FilesFrom.class.php b/src/main/php/web/handler/FilesFrom.class.php index d50ebdab..96e80e8b 100755 --- a/src/main/php/web/handler/FilesFrom.class.php +++ b/src/main/php/web/handler/FilesFrom.class.php @@ -6,7 +6,8 @@ use web\io\Ranges; class FilesFrom implements Handler { - const BOUNDARY = '594fa07300f865fe'; + const BOUNDARY = '594fa07300f865fe'; + const CHUNKSIZE = 8192; private $path; @@ -60,7 +61,7 @@ private function copy($output, $file, $range) { $file->seek($range->start()); $length= $range->length(); - while ($length && $chunk= $file->read(min(8192, $length))) { + while ($length && $chunk= $file->read(min(self::CHUNKSIZE, $length))) { $output->write($chunk); $length-= strlen($chunk); yield; @@ -105,7 +106,7 @@ public function serve($request, $response, $target) { $file->open(File::READ); try { do { - $out->write($file->read()); + $out->write($file->read(self::CHUNKSIZE)); yield; } while (!$file->eof()); } finally {