diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9501566..6da4321 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -3,22 +3,21 @@ on: branches: - main - master - - availability pull_request: branches: - main - master - - availability name: tests jobs: setup: name: setup - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest steps: - name: Slack Notification on Start uses: rtCamp/action-slack-notify@v2.2.0 + if: env.SLACK_WEBHOOK != '' env: SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_TESTS }} SLACK_CHANNEL: notify-nc3-tests @@ -28,7 +27,7 @@ jobs: tests: name: tests needs: setup - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest strategy: matrix: php: [ '7.1', '7.2', '7.3', '7.4' ] @@ -44,6 +43,7 @@ jobs: MYSQL_VERSION: ${{ matrix.mysql }} MYSQL_ROOT_PASSWORD: root MYSQL_DATABASE: cakephp_test + COMPOSER_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: - uses: actions/checkout@v2 @@ -91,46 +91,53 @@ jobs: docker-compose exec -T nc3app bash /opt/scripts/app-build.sh - name: phpcs (PHP CodeSniffer) + if: always() run: | cd ${NC3_DOCKER_DIR} docker-compose exec -T nc3app bash /opt/scripts/phpcs.sh - name: phpmd (PHP Mess Detector) + if: always() run: | cd ${NC3_DOCKER_DIR} docker-compose exec -T nc3app bash /opt/scripts/phpmd.sh - name: phpcpd (PHP Copy/Paste Detector) + if: always() run: | cd ${NC3_DOCKER_DIR} docker-compose exec -T nc3app bash /opt/scripts/phpcpd.sh - name: gjslint (JavaScript Style Check) + if: always() run: | cd ${NC3_DOCKER_DIR} docker-compose exec -T nc3app bash /opt/scripts/gjslint.sh - name: phpdoc (PHP Documentor) + if: always() run: | cd ${NC3_DOCKER_DIR} docker-compose exec -T nc3app bash /opt/scripts/phpdoc.sh - name: phpunit (PHP UnitTest) + if: always() run: | cd ${NC3_DOCKER_DIR} docker-compose exec -T nc3app bash /opt/scripts/phpunit.sh sudo -s chmod a+w -R ${NC3_BUILD_DIR}/build - - name: push coveralls - env: - COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }} - COVERALLS_FLAG_NAME: ${{ matrix.php }} - run: | - cd ${NC3_BUILD_DIR} - ls -la ${NC3_BUILD_DIR} - vendors/bin/php-coveralls --coverage_clover=build/logs/clover.xml -v +# - name: push coveralls +# env: +# COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }} +# COVERALLS_FLAG_NAME: ${{ matrix.php }} +# run: | +# cd ${NC3_BUILD_DIR} +# ls -la ${NC3_BUILD_DIR} +# vendors/bin/php-coveralls --coverage_clover=build/logs/clover.xml -v - name: docker-compose remove + if: always() run: | cd ${NC3_DOCKER_DIR} docker-compose rm -f @@ -138,7 +145,7 @@ jobs: # テスト失敗時はこちらのステップが実行される - name: Slack Notification on Failure uses: rtCamp/action-slack-notify@v2.2.0 - if: failure() + if: env.SLACK_WEBHOOK != '' && failure() env: SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_TESTS }} SLACK_CHANNEL: notify-nc3-tests @@ -147,13 +154,13 @@ jobs: teardown: name: teardown - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest needs: tests steps: # テスト成功時はこちらのステップが実行される - name: Slack Notification on Success - if: success() uses: rtCamp/action-slack-notify@v2.2.0 + if: env.SLACK_WEBHOOK != '' && success() env: SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_TESTS }} SLACK_CHANNEL: notify-nc3-tests diff --git a/Controller/Component/DownloadComponent.php b/Controller/Component/DownloadComponent.php index a168350..14e4d56 100644 --- a/Controller/Component/DownloadComponent.php +++ b/Controller/Component/DownloadComponent.php @@ -80,10 +80,11 @@ public function doDownload($contentId, $options = array()) { * * @param int $uploadFileId UploadFile ID * @param array $options オプション field : ダウンロードのフィールド名, size: nullならオリジナル thumb, small, medium, big + * @param string $pluginKey プラグインキー * @return CakeResponse|null * @throws ForbiddenException */ - public function doDownloadByUploadFileId($uploadFileId, $options = array()) { + public function doDownloadByUploadFileId($uploadFileId, $options = [], $pluginKey = 'wysiwyg') { if (isset($options['size'])) { $size = $options['size']; unset($options['size']); @@ -101,10 +102,11 @@ public function doDownloadByUploadFileId($uploadFileId, $options = array()) { $UploadFile = ClassRegistry::init('Files.UploadFile'); $file = $UploadFile->findById($uploadFileId); - if (! $file) { + if (! $file || $file['UploadFile']['plugin_key'] !== $pluginKey) { //データがない＝リンク切れ。リンク切れの場合、ログアウトしないようにするため、メッセージを追加 throw new ForbiddenException('Not found file'); } + return $this->_downloadUploadFile($file, $size, $options); } @@ -217,7 +219,10 @@ protected function _downloadUploadFile($file, $size, $options) { } // Download カウントアップ - $UploadFile->countUp($file); + $pluginKey = $file['UploadFile']['plugin_key'] ?? null; + if ($pluginKey !== 'wysiwyg') { + $UploadFile->countUp($file); + } return $this->_controller->response; } diff --git a/Test/Case/Controller/Component/DownloadComponent/DoDownloadByUploadFileIdTest.php b/Test/Case/Controller/Component/DownloadComponent/DoDownloadByUploadFileIdTest.php index 589efea..a067a36 100644 --- a/Test/Case/Controller/Component/DownloadComponent/DoDownloadByUploadFileIdTest.php +++ b/Test/Case/Controller/Component/DownloadComponent/DoDownloadByUploadFileIdTest.php @@ -109,7 +109,7 @@ public function testDoDownloadByUploadFileId() { ->will($this->returnValue(true)); $UploadFileMock->uploadBasePath = $uploadBasePath; - $this->controller->Download->doDownloadByUploadFileId($fileId); + $this->controller->Download->doDownloadByUploadFileId($fileId, [], 'site_manager'); } /** @@ -159,7 +159,7 @@ public function testFixAvatarDownloadProblem() { // コンテンツキーが入ってるとブロックガードで例外が発生してた。-> DownloadComponent修正で例外発生しなくなったのを確認 // @codingStandardsIgnoreStart NOTICE無視して例外発生するのを確認したかったので@でエラー抑止してます。 - @$this->controller->Download->doDownloadByUploadFileId($fileId); + @$this->controller->Download->doDownloadByUploadFileId($fileId, [], 'users'); // @codingStandardsIgnoreEnd } } diff --git a/Test/Case/Utility/TemporaryFile/MemoryLeakTest.php b/Test/Case/Utility/TemporaryFile/MemoryLeakTest.php new file mode 100644 index 0000000..262069b --- /dev/null +++ b/Test/Case/Utility/TemporaryFile/MemoryLeakTest.php @@ -0,0 +1,117 @@ +markTestSkipped(); + } + +/** + * testMemoryLeakForKeepFiles + * + * @return void + * @SuppressWarnings(PHPMD.UnusedLocalVariable) + */ + public function testMemoryLeakForKeepFiles() { + $max = 1000; + App::uses('TemporaryFolder', 'Files.Utility'); + $before = $this->__reportMemory('before'); + for ($i = 0; $i < $max; $i++) { + $files[] = new TemporaryFile(); + } + $this->__reportMemory('after ', $before); + } + +/** + * testMemoryLeakWithDelete + * + * @return void + * @SuppressWarnings(PHPMD.UnusedLocalVariable) + */ + public function testMemoryLeakWithDelete() { + $max = 1000; + App::uses('TemporaryFolder', 'Files.Utility'); + $before = $this->__reportMemory('before'); + for ($i = 0; $i < $max; $i++) { + $file = new TemporaryFile(); + $files[] = $file; + $file->delete(); + } + $this->__reportMemory('after ', $before); + } + +/** + * testMemoryLeakWithDeleteAndOverwrite + * + * @return void + */ + public function testMemoryLeakWithDeleteAndOverwrite() { + $max = 1000; + App::uses('TemporaryFolder', 'Files.Utility'); + $before = $this->__reportMemory('before'); + for ($i = 0; $i < $max; $i++) { + $folder = new TemporaryFile(); + $folder->delete(); + } + $this->__reportMemory('after ', $before); + } + +/** + * testMemoryLeakWithOverwrite + * + * @return void + * @SuppressWarnings(PHPMD.UnusedLocalVariable) + */ + public function testMemoryLeakWithOverwrite() { + $max = 1000; + App::uses('TemporaryFolder', 'Files.Utility'); + $before = $this->__reportMemory('before'); + for ($i = 0; $i < $max; $i++) { + $file = new TemporaryFile(); + // ここでテンポラリファイルを使った処理を想定 + } + $this->__reportMemory('after ', $before); + } + +/** + * __reportMemory + * + * @param string $string + * @param int|null $before + * @return int + */ + private function __reportMemory(string $string, int $before = null) { + $current = memory_get_usage(false); + print $string . ':' . number_format($current); + if ($before !== null) { + print "\n"; + print $string . '(diff):' . number_format($current - $before); + } + print "\n"; + return $current; + } +} diff --git a/Test/Case/Utility/TemporaryFolder/MemoryLeakTest.php b/Test/Case/Utility/TemporaryFolder/MemoryLeakTest.php new file mode 100644 index 0000000..a4a555b --- /dev/null +++ b/Test/Case/Utility/TemporaryFolder/MemoryLeakTest.php @@ -0,0 +1,105 @@ +markTestSkipped(); + } + +/** + * testMemoryLeakForKeepFolders + * メモリ使用量確認用コード。テスト対象外 + * + * @return void + * @SuppressWarnings(PHPMD.UnusedLocalVariable) + */ + public function testMemoryLeakForKeepFolders() { + $max = 1000; + App::uses('TemporaryFolder', 'Files.Utility'); + $before = $this->__reportMemory('before'); + for ($i = 0; $i < $max; $i++) { + $folders[] = new TemporaryFolder(); + } + $this->__reportMemory('after ', $before); + } + +/** + * testMemoryLeakWithDelete + * メモリ使用量確認用コード。テスト対象外 + * + * @return void + * @SuppressWarnings(PHPMD.UnusedLocalVariable) + */ + public function testMemoryLeakWithDelete() { + $max = 1000; + App::uses('TemporaryFolder', 'Files.Utility'); + $before = $this->__reportMemory('before'); + for ($i = 0; $i < $max; $i++) { + $folder = new TemporaryFolder(); + $folders[] = $folder; + $folder->delete(); + } + $this->__reportMemory('after ', $before); + } + +/** + * testMemoryLeakWithDeleteAndOverwrite + * メモリ使用量確認用コード。テスト対象外 + * + * @return void + * @SuppressWarnings(PHPMD.UnusedLocalVariable) + */ + public function testMemoryLeakWithDeleteAndOverwrite() { + $max = 1000; + App::uses('TemporaryFolder', 'Files.Utility'); + $before = $this->__reportMemory('before'); + for ($i = 0; $i < $max; $i++) { + $folder = new TemporaryFolder(); + $folder->delete(); + } + $this->__reportMemory('after ', $before); + } + +/** + * __reportMemory + * + * @param string $string + * @param int|null $before + * @return int + */ + private function __reportMemory(string $string, int $before = null) { + $current = memory_get_usage(false); + print $string . ':' . number_format($current); + if ($before !== null) { + print "\n"; + print $string . '(diff):' . number_format($current - $before); + } + print "\n"; + return $current; + } +} diff --git a/Test/Case/Utility/TemporaryFolder/TemporaryFolderTest.php b/Test/Case/Utility/TemporaryFolder/TemporaryFolderTest.php index 55dc695..9d796c0 100644 --- a/Test/Case/Utility/TemporaryFolder/TemporaryFolderTest.php +++ b/Test/Case/Utility/TemporaryFolder/TemporaryFolderTest.php @@ -46,11 +46,6 @@ public function testDelete() { $path = $tempFolder->path; $tempFolder->delete(); $this->assertFalse(file_exists($path)); - - //デストラクタ廃止したのでこのテストも廃止 - //$tempFolder2 = new TemporaryFolder(); - //$path2 = $tempFolder2->path; - //unset($tempFolder2); - //$this->assertFalse(file_exists($path2)); } + } diff --git a/Utility/TemporaryFile.php b/Utility/TemporaryFile.php index 155b0e3..20808b1 100644 --- a/Utility/TemporaryFile.php +++ b/Utility/TemporaryFile.php @@ -16,6 +16,16 @@ */ class TemporaryFile extends File { +/** + * @var array このクラスで作成されたテンポラリファイルのリスト + */ + private static $__filePaths = []; + +/** + * @var boola register_shutdown_functionに登録済みか + */ + private static $__isRegisteredShutdownFunction = false; + /** * @var TemporaryFolder テンポラリファイルを配置するテンポラリフォルダ */ @@ -33,8 +43,42 @@ public function __construct($folderPath = null) { } $fileName = Security::hash(mt_rand() . microtime(), 'md5'); - register_shutdown_function(array($this, 'delete')); + $path = $folderPath . DS . $fileName; + + self::$__filePaths[] = $path; + if (!self::$__isRegisteredShutdownFunction) { + register_shutdown_function([TemporaryFile::CLASS, 'deleteAll']); + self::$__isRegisteredShutdownFunction = true; + } + parent::__construct($path, true); + } + +/** + * 削除 + * + * @return void + */ + public function delete() { + $key = array_search($this->path, self::$__filePaths); + if ($key !== false) { + unset(self::$__filePaths[$key]); + if ($this->_tmpFolder instanceof TemporaryFolder) { + $this->_tmpFolder->delete(); + } + } + parent::delete(); + } - parent::__construct($folderPath . DS . $fileName, true); +/** + * 全テンポラリファイル削除 + * + * @return void + */ + public static function deleteAll() { + foreach (self::$__filePaths as $path) { + $file = new File($path); + $file->delete(); + } + self::$__filePaths = []; } } diff --git a/Utility/TemporaryFolder.php b/Utility/TemporaryFolder.php index cb801d0..2f97862 100644 --- a/Utility/TemporaryFolder.php +++ b/Utility/TemporaryFolder.php @@ -24,6 +24,16 @@ */ class TemporaryFolder extends Folder { +/** + * @var string[] このクラスで作成されたテンポラリフォルダリスト + */ + private static $__folderPaths = []; + +/** + * @var bool register_shutdown_functionに登録済みか + */ + private static $__isRegisteredShutdownFunction = false; + /** * TemporaryFolder constructor. */ @@ -32,16 +42,41 @@ public function __construct() { $path .= Security::hash(mt_rand() . microtime(), 'md5'); //$mode = '0775'; // ε(　　　　 v ﾟωﾟ)　＜パーミッションいくつが適切だ？ $mode = false; // とりあえずデフォルトのまま - register_shutdown_function(array($this, 'delete')); + self::$__folderPaths[] = $path; + if (!self::$__isRegisteredShutdownFunction) { + register_shutdown_function([TemporaryFolder::CLASS, 'deleteAll']); + self::$__isRegisteredShutdownFunction = true; + } parent::__construct($path, true, $mode); } /** - * デストラクタ + * 削除 + * + * @param string|null $path 削除対象パス + * @return bool + */ + public function delete($path = null) { + $path = $path ?? $this->path; + if ($path) { + $key = array_search($path, self::$__folderPaths); + if ($key !== false) { + unset(self::$__folderPaths[$key]); + } + } + return parent::delete($path); + } + +/** + * 全テンポラリフォルダの削除 * * @return void */ - //public function __destruct() { - // $this->delete(); - //} -} \ No newline at end of file + public static function deleteAll() { + $folder = new Folder(); + foreach (self::$__folderPaths as $path) { + $folder->delete($path); + } + self::$__folderPaths = []; + } +} diff --git a/Utility/UnZip.php b/Utility/UnZip.php index f37bfe6..90b373b 100644 --- a/Utility/UnZip.php +++ b/Utility/UnZip.php @@ -126,10 +126,10 @@ protected function _extractWithZipArchiveClass($path) { $zip->setPassword($this->_password); } $index = 0; - while ($zipEntry = $zip->statIndex($index)) { + while ($zipEntry = $zip->statIndex($index, ZipArchive::FL_ENC_RAW)) { $zipEntryName = $zipEntry['name']; $destName = mb_convert_encoding($zipEntry['name'], $encodeCharset, 'auto'); - if ($zip->renameName($zipEntryName, $destName) === false) { + if ($zip->renameIndex($index, $destName) === false) { return false; } if ($zip->extractTo($path, $destName) === false) { diff --git a/VERSION.txt b/VERSION.txt index fa7adc7..86fb650 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -3.3.5 +3.3.7 diff --git a/View/Helper/FilesFormHelper.php b/View/Helper/FilesFormHelper.php index c35a19d..465cc6f 100644 --- a/View/Helper/FilesFormHelper.php +++ b/View/Helper/FilesFormHelper.php @@ -109,7 +109,7 @@ public function uploadFile($fieldName, $options = array()) { if (isset($this->_uploadFileNames[$fieldName])) { if ($filename) { - $output .= $this->_uploadFileNames[$fieldName]; + $output .= h($this->_uploadFileNames[$fieldName]); } if ($remove) { $output .= $this->NetCommonsForm->checkbox(