[テーマ管理] テーマ名に半角カッコを含むテーマがあると画面がエラーになる問題を修正しました - #2486
Merged
Conversation
create() / saveName() / generate() の3箇所に同じルールとメッセージを コピペしていたため、themes.ini に書き出せない文字の定義を THEME_NAME_NG_REGEX / THEME_NAME_NG_MESSAGE の定数と getThemeNameRules() に集約し、makeThemesIni() の近くに配置した。 refs #2465
テーマ設定ファイルにテーマ名がない場合のフォールバックを ?? で 書いていたが、これは getThemeName() が持つロジックの再実装だったため、 getThemeName() の戻り値に themes を足す形に変更した。 テーマ名の決定ルールが getThemeName() の1箇所に集約される。 refs #2465
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
テーマ名に半角カッコ
( )を含むテーマが存在すると、ページ管理の初期表示がシステムエラーになる不具合を修正しました。原因
public/themes/**/themes.iniのtheme_nameに半角カッコがダブルクォートなしで書かれていると、parse_ini_file()がsyntax error, unexpected '('の警告を出してfalseを返します。Connect-CMS はPluginBase::__construct()でset_error_handler(ccErrorHandler)を設定しており、警告をErrorExceptionに変換して throw するため、壊れたthemes.iniが1つあるだけで画面全体がシステムエラーになっていました。作り込みの元はテーマ管理側で、
theme_nameを素のままthemes.iniに書き出していたことです(バリデーションもrequiredのみ)。そのため、テーマ管理の画面から半角カッコ入りのテーマ名を登録すると、その直後から復旧できなくなる状態でした。影響範囲はIssue記載のページ管理だけではなく、
PluginBase::getThemes()を使う以下の画面すべてです。変更内容
Issueに記載いただいた恒久対策案1・2の両方を実施しました。既存サイトには既に壊れた
themes.iniが存在しうるため、片方だけでは不十分と判断しています。1. 読み込み側の堅牢化(対策案2)
App\Utilities\File\FileUtilsにparseIniFile()を追加し、themes.iniを読む5箇所を置き換えました。parse_ini_file()にINI_SCANNER_RAWを指定し、ダブルクォートなしの ini 予約文字も読めるようにしていますis_array()チェックを行い、失敗時は空配列を返してログに warning を出しますgetThemes()で、グループ用themes.iniにtheme_nameが無い場合に未定義キー参照となっていた箇所も修正しました置き換え箇所
PluginBase::getThemes()(第1階層・グループ配下の2箇所)ThemeManage::index()/ThemeManage::getUserThemeName()SiteManage(サイト設定書PDF出力)なお
ThemeManage::index()とSiteManageはファイル存在チェックが無かったため、themes.iniの無いディレクトリがあるだけでもエラーになる状態でしたが、こちらも同時に解消されます。2. 書き出し側の対応(対策案1)
makeThemesIni()に共通化し、theme_nameを必ずダブルクォートで囲むようにしましたtheme_nameのバリデーションにnot_regexを追加し、ini の値として表現できない"と\を弾くようにしました(半角カッコは正しく扱えるため許可しています)3. テスト追加
tests/Unit/Utilities/File/FileUtilsTest.php:escapeIniValue()のテストを追加tests/Unit/Utilities/File/FileUtilsParseIniFileTest.php(新規): 既存フォーマットの後方互換7パターン、ini予約文字、ダブルクォート付き、書き出し→読み込みの往復、壊れたファイル、存在しないパスtests/Unit/Plugins/PluginBaseGetThemesTest.php(新規): 半角カッコ入りthemes.iniがあってもgetThemes()が例外を投げないこと既存 themes.ini への影響(後方互換)
INI_SCANNER_RAWの指定で既存の読み込み結果が変わらないことを、PHP実機で確認しています。theme_name = BlueBlueBlue(同一)theme_name = カスタムテーマ1theme_name = Users グループ+theme_dir = grouptheme_name = clear-steelblue_01.2theme_name = Blue(前後空白)BlueBlue(同一);コメント行ありtheme_name = theme_user_02 (clear-steelblue)theme_user_02 (clear-steelblue)(本件の修正)結果が変わるのは
theme_nameがon/true/null/ PHP定義済み定数と完全一致する場合のみで、現行が値を1や空文字に変換してしまっていたものが、正しい文字列として読めるようになる方向の差異です。「今まで読めていたものが読めなくなる」パターンはありません。既存の壊れた
themes.iniもINI_SCANNER_RAWによってそのまま正しく読めるようになるため、修復用のバッチやマイグレーションは不要です。テーマ管理の「名称変更」から保存し直せば、ダブルクォート付きに書き直されます。確認内容
tests/Unit/Utilities/File/: OK (28 tests, 28 assertions)tests/Unit/Plugins/PluginBaseGetThemesTest.php: OK (3 tests, 7 assertions)themes.iniがダブルクォート付きで保存され、ページ管理・ページ編集・サイト管理・テーマ管理・テーマチェンジャー(公開画面)・サイト設定書PDF がいずれもエラーにならないこと"や\を含むテーマ名がバリデーションで弾かれ、ファイルが書き換わらないことDefaults配下28件、ユーザ・テーマ)の表示名が従来通りであること、テーマの適用表示が従来通りであることthemes.iniを手置きした場合も、エラーにならずテーマ名が正しく表示されることthemes.iniがあっても画面が落ちず、ディレクトリ名にフォールバックしてログに warning が出ることレビュー完了希望日
不具合対応ですが、暫定対処(該当 themes.ini をダブルクォートで囲む)があるため、急ぎではありません。
関連Pull requests/Issues
参考
INI_SCANNER_RAWについてDB変更の有無
無し
チェックリスト