-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ CTA ] エスケープ処置&テスト調整 #1167
[ CTA ] エスケープ処置&テスト調整 #1167
Conversation
@@ -478,9 +478,6 @@ public static function render_cta_content( $id ) { | |||
// リセットしないと$postが改変されたままでコメント欄が表示されなくなるなどの弊害が発生する. | |||
wp_reset_postdata(); | |||
|
|||
// wp_kses_post でエスケープすると outerブロックが出力するstyle属性を無効化されるので使わないように. | |||
// 出力時にエスケープしたいが、wp_kses_post だと style属性が無効化される / wp_kses でも allow_html で opacity を許可しても無視・削除される。 | |||
// 結局本文欄はどのみち HTML ブロックで <script>alert(0)</script> 入れられ標準でXSSは実行可能なので、ここでは処理していない。 | |||
return self::safe_kses_post( do_blocks( do_shortcode( $content ) ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$tags['style'] = array( | ||
'type' => true, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtdkei スタイルタグの許可を追加
array( | ||
'name' => 'class and style', | ||
'input' => '<div class="class-name" style="margin-bottom:3rem">CTA</div>', | ||
'expected' => '<div class="class-name" style="margin-bottom:3rem">CTA</div>', | ||
), | ||
array( | ||
'name' => 'allow style tag', | ||
'input' => '<div>CTA</div><style>.target-class { background-image: url(http://localhost:8888/image.jpg);}</style>', | ||
'expected' => '<div>CTA</div><style>.target-class { background-image: url(http://localhost:8888/image.jpg);}</style>', | ||
), | ||
array( | ||
'name' => 'allow style tag with media query', | ||
'input' => '<div>CTA</div><style>@media screen and (max-width: 575.98px) {.target-class { | ||
background-image: url(http://localhost:8888/image.jpg); | ||
background-position: 50% 50%!important;}}</style>', | ||
'expected' => '<div>CTA</div><style>@media screen and (max-width: 575.98px) {.target-class { | ||
background-image: url(http://localhost:8888/image.jpg); | ||
background-position: 50% 50%!important;}}</style>', | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtdkei クラス名、スタイルの直書き、styleタグがエスケープされてしまわないかのテスト追加
); | ||
|
||
foreach ( $test_cases as $case ) { | ||
$actual = Vk_Call_To_Action::safe_kses_post( $case['input'] ); | ||
$this->assertEquals( $case['expected'], $actual ); | ||
$this->assertEquals( $case['expected'], $actual, $case['name'] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtdkei テストがコケた時にコケた場所がわかりやすいようにテスト項目名追加
@kurudrive |
チケットへのリンク / 変更の理由(元のissueがあればリンクを貼り付ければOK)
@mtdkei 軽くコメントだけ確認しておいてくださいー。