From 9ca143ff0993006a91ed46267bd5f3168dca4a6f Mon Sep 17 00:00:00 2001 From: Oleg Abrazhaev Date: Sun, 27 Oct 2019 13:36:04 +0100 Subject: [PATCH 1/4] extract html for FileListing into twig and add tests #14801 Signed-off-by: Oleg Abrazhaev --- libraries/classes/FileListing.php | 16 ++++---- templates/file_select_options.twig | 5 +++ test/classes/FileListingTest.php | 51 +++++++++++++++++++++++++ test/classes/_data/file_listing/one.txt | 0 test/classes/_data/file_listing/two.md | 0 5 files changed, 63 insertions(+), 9 deletions(-) create mode 100644 templates/file_select_options.twig create mode 100644 test/classes/_data/file_listing/one.txt create mode 100644 test/classes/_data/file_listing/two.md diff --git a/libraries/classes/FileListing.php b/libraries/classes/FileListing.php index a562d9ea7e..f8487725b8 100644 --- a/libraries/classes/FileListing.php +++ b/libraries/classes/FileListing.php @@ -64,15 +64,13 @@ class FileListing if ($list === false) { return false; } - $result = ''; - foreach ($list as $val) { - $result .= ' +{% endfor %} diff --git a/test/classes/FileListingTest.php b/test/classes/FileListingTest.php index 278147dea6..fa8aa7058b 100644 --- a/test/classes/FileListingTest.php +++ b/test/classes/FileListingTest.php @@ -34,6 +34,13 @@ class FileListingTest extends TestCase public function testGetDirContent(): void { $this->assertFalse($this->fileListing->getDirContent('nonexistent directory')); + + $fixturesDir = ROOT_PATH . 'test/classes/_data/file_listing'; + + $this->assertSame([ + 1 => 'one.txt', + 0 => 'two.md', + ], $this->fileListing->getDirContent($fixturesDir)); } /** @@ -41,7 +48,51 @@ class FileListingTest extends TestCase */ public function testGetFileSelectOptions(): void { + $fixturesDir = ROOT_PATH . 'test/classes/_data/file_listing'; + $this->assertFalse($this->fileListing->getFileSelectOptions('nonexistent directory')); + + $expectedHtmlWithoutActive = << + one.txt + + + +HTML; + + $this->assertSame( + $expectedHtmlWithoutActive, + $this->fileListing->getFileSelectOptions($fixturesDir) + ); + + $expectedHtmlWithActive = << + one.txt + + + +HTML; + + $this->assertSame( + $expectedHtmlWithActive, + $this->fileListing->getFileSelectOptions($fixturesDir, '', 'two.md') + ); + + $expectedFilteredHtml = << + one.txt + + +HTML; + + $this->assertSame( + $expectedFilteredHtml, + $this->fileListing->getFileSelectOptions($fixturesDir, '/.*\.txt/') + ); } /** diff --git a/test/classes/_data/file_listing/one.txt b/test/classes/_data/file_listing/one.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/classes/_data/file_listing/two.md b/test/classes/_data/file_listing/two.md new file mode 100644 index 0000000000..e69de29bb2 From e23217b1e28c8e76114651fc38927ced6cf87fd1 Mon Sep 17 00:00:00 2001 From: Oleg Abrazhaev Date: Sun, 27 Oct 2019 14:03:05 +0100 Subject: [PATCH 2/4] fix test to work on all travis platforms Signed-off-by: Oleg Abrazhaev --- test/classes/FileListingTest.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/classes/FileListingTest.php b/test/classes/FileListingTest.php index fa8aa7058b..542e3b9358 100644 --- a/test/classes/FileListingTest.php +++ b/test/classes/FileListingTest.php @@ -37,10 +37,13 @@ class FileListingTest extends TestCase $fixturesDir = ROOT_PATH . 'test/classes/_data/file_listing'; - $this->assertSame([ - 1 => 'one.txt', - 0 => 'two.md', - ], $this->fileListing->getDirContent($fixturesDir)); + $this->assertSame( + array_values([ + 'one.txt', + 'two.md', + ]), + array_values($this->fileListing->getDirContent($fixturesDir)) + ); } /** From 5a36d65114fcd09f2b34a58dc6bfdbe28c86fb5e Mon Sep 17 00:00:00 2001 From: Oleg Abrazhaev Date: Sun, 27 Oct 2019 15:19:45 +0100 Subject: [PATCH 3/4] sanitize list entries Signed-off-by: Oleg Abrazhaev --- libraries/classes/FileListing.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libraries/classes/FileListing.php b/libraries/classes/FileListing.php index f8487725b8..9d85289517 100644 --- a/libraries/classes/FileListing.php +++ b/libraries/classes/FileListing.php @@ -65,10 +65,14 @@ class FileListing return false; } + $sanitizedList = array_map(function (string $entry) { + return htmlspecialchars($entry); + }, $list); + $template = new Template(); return $template->render('file_select_options', [ - 'filesList' => $list, + 'filesList' => $sanitizedList, 'active' => $active ]); } From ba370e2311751dd1a48bd36ad5b1be9a5b846e2e Mon Sep 17 00:00:00 2001 From: Oleg Abrazhaev Date: Mon, 28 Oct 2019 19:31:15 +0100 Subject: [PATCH 4/4] fix code review Signed-off-by: Oleg Abrazhaev --- libraries/classes/FileListing.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/libraries/classes/FileListing.php b/libraries/classes/FileListing.php index 9d85289517..f8487725b8 100644 --- a/libraries/classes/FileListing.php +++ b/libraries/classes/FileListing.php @@ -65,14 +65,10 @@ class FileListing return false; } - $sanitizedList = array_map(function (string $entry) { - return htmlspecialchars($entry); - }, $list); - $template = new Template(); return $template->render('file_select_options', [ - 'filesList' => $sanitizedList, + 'filesList' => $list, 'active' => $active ]); }