fix #4303 【メール】マルチチェックボックスの各選択肢が自動でspanタグで囲まれる問題を解決

[katokaisya]

・入力画面にヘルプも追加しています。

[Copilot]

Pull request overview

このプルリクエストは、baserCMS のメールフォーム機能において、マルチチェックボックスの各選択肢が自動的に span タグで囲まれる問題（Issue #4303）を解決することを目的としています。ユーザーがカスタムのラッピング要素（例：div タグやクラス付き要素）を指定できるよう、templateVars.tag オプションのサポートを追加しています。

Changes:

• MailformHelper.php に templateVars.tag オプションの処理ロジックを追加
• 管理画面のメールフィールドフォームに、templateVars.tag オプションの使用方法を説明するヘルプテキストを追加

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
plugins/bc-mail/src/View/Helper/MailformHelper.php	マルチチェックボックスで templateVars.tag オプションが指定された場合の処理ロジックを追加。有効なタグ名（div, span, p, li, dt, dd, label）のバリデーションを実装
plugins/bc-admin-third/templates/plugin/BcMail/Admin/element/MailFields/form.php	オプション欄にヘルプアイコンとヘルプテキストを追加。placeholder、empty、templateVars.tag の3つのオプションの使用方法を記載

[Copilot]

セキュリティ上の懸念：$attributes['templateVars.tag'] の値がそのままHTMLに出力される可能性があります。ユーザーが div onclick="alert('XSS')" のような悪意のある入力を行った場合、XSS脆弱性につながる可能性があります。

タグ名と属性を分離し、属性値を適切にエスケープする処理を追加することを推奨します。または、属性の指定を許可せず、タグ名のみを受け入れるように仕様を変更することも検討してください。

[Copilot]

有効なタグに一致しない場合の処理が不完全です。ループ内でbreakが実行されなかった場合（つまり、有効なタグが見つからなかった場合）、$attributes['templateVars']['tag'] はデフォルトの 'span' のままになりますが、無効な入力に対する明示的な処理やログ記録がありません。

無効な値が入力された場合は、警告ログを記録するか、デフォルト値を使用していることをユーザーに通知することを検討してください。

[Copilot]

ヘルプテキストの例に誤解を招く可能性があります。「div class="checkbox"」という例は、クラス属性を含むタグを指定できることを示唆していますが、実際のコード実装ではこれが適切に処理されていません（セキュリティとロジックの問題については他のコメントを参照）。

実装が修正されるまで、ヘルプテキストの例を単純なタグ名のみに変更することを推奨します。例：「templateVars.tag|div」

Suggested change

	<dd>マルチチェックボックスでそれぞれのインプットタグをラッピングする要素を指定します。例）templateVars.tag|div class="checkbox"（div class="checkbox"で囲む）<br>※デフォルトはspanタグでラッピングされます。</dd>
	<dd>マルチチェックボックスでそれぞれのインプットタグをラッピングする要素を指定します。例）templateVars.tag|div（divタグで囲む）<br>※デフォルトはspanタグでラッピングされます。</dd>

[Copilot]

新しく追加された templateVars.tag オプション処理機能に対するユニットテストが不足しています。以下のテストケースを追加することを推奨します：

1. 有効なタグ名（例：'div', 'span', 'p'）が正しく処理されることを確認
2. タグ名とクラス属性を含む入力（例：'div class="checkbox"'）の処理を確認
3. 無効なタグ名が入力された場合のデフォルト動作を確認
4. templateVars.tag が指定されていない場合のデフォルト動作を確認

MailformHelperTest.php に testControlWithTemplateVarsTag のようなテストメソッドを追加してください。

[Copilot]

行185の代入が不適切です。$attributes['templateVars']['tag'] に $attributes['templateVars.tag'] をそのまま代入していますが、これでは検証の意味がありません。代わりに、検証されたタグ名（例：$validTag）またはクラスを含む全体の文字列を適切に処理する必要があります。

例えば、ユーザーが templateVars.tag|div class="checkbox" と入力した場合、div class="checkbox" 全体を代入するのではなく、タグ名とクラスを分離して処理することを検討してください。

Suggested change

	foreach ($validTags as $validTag) {
	// templateVars.tagが有効なタグ名から始まっている場合、$attributes['templateVars']['tag'] にセット
	if (str_starts_with($attributes['templateVars.tag'], $validTag)) {
	$attributes['templateVars']['tag'] = $attributes['templateVars.tag'];
	$rawTemplateTag = $attributes['templateVars.tag'];
	foreach ($validTags as $validTag) {
	// templateVars.tagが有効なタグ名から始まっている場合、タグ名とクラス属性を分離して処理
	if (str_starts_with($rawTemplateTag, $validTag)) {
	// 検証済みのタグ名のみをtemplateVars['tag']にセット
	$attributes['templateVars']['tag'] = $validTag;
	// 先頭のタグ名以降の文字列を取得（例: ' class="checkbox"'）
	$rest = trim(substr($rawTemplateTag, strlen($validTag)));
	if ($rest !== '') {
	// class属性が指定されている場合は抽出して$attributes['class']にマージ
	if (preg_match('/class\s*=\s*([\'"])(.*?)\1/i', $rest, $classMatches)) {
	$newClass = $classMatches[2];
	if ($newClass !== '') {
	if (!empty($attributes['class'])) {
	if (is_array($attributes['class'])) {
	$existingClass = implode(' ', $attributes['class']);
	} else {
	$existingClass = (string)$attributes['class'];
	}
	$attributes['class'] = trim($existingClass . ' ' . $newClass);
	} else {
	$attributes['class'] = $newClass;
	}
	}
	}
	}

[katokaisya]

ラッピング要素付与するはずの属性がinputタグに付与されてしまうため、
Copilotの指摘は間違っています。（CopilotがCakePHPの仕様を正しく理解できていない）

出力するべき要素（ラッピング要素に指定した属性を付与）

<div class="checkbox">
	<input type="checkbox" name="agree[]" value="同意します。" id="agree-同意します。" class="" required="required">
	<label for="agree-同意します。" class="">同意します。</label>
</div>

Copilotの指摘のように　タグと付与属性を分けた場合の出力（ラッピング要素ではなく、inputタグに属性が付与されてしまい、全く意味をなさない）

<div class="">
	<input type="checkbox" name="agree[]" value="同意します。" id="agree-同意します。" class="checkbox" required="required">
	<label for="agree-同意します。" class="">同意します。</label>
</div>

[katokaisya]

ヘルプテキストの例に誤解を招く可能性があります。「div class="checkbox"」という例は、クラス属性を含むタグを指定できることを示唆していますが、実際のコード実装ではこれが適切に処理されていません

これも全く的外れの指摘のため、完全に無視して構いません。
ラッピング要素にclassやdata属性を付与できるのもcakePHPの仕様です。
セキュリティ上懸念のある、on属性などを除外しておけば、問題ありません、

[ryuring]

設定名を checkboxWrapTag にする

[katokaisya]

@ryuring

設定名を checkboxWrapTagに変更してます。
お手数ですが、ご確認の上マージお願いします。

[ryuring]

@katokaisya ありがとうございます！