Bug(バグ) #2360
#1373 で実施したメール投稿の例外を受ける処理が不適切である
100%
Description
Overview (概要)¶
http://redmine.openpne.jp/issues/1372#note-17 の指摘点をコピー
opMailSend クラス内で例外を止めてリダイレクト処理までしてしまう変更は不適切だと思います。このような変更をしてしまうと、opMailSend クラスを利用するコード側でメール送信失敗時の例外処理を記述することができなくなります。 その意味では #1373 で行われた 3.4 向けの変更も不適切だと思います。(もちろん意図しない 500 エラーが発生しなくなったことはよいことですが、これまで利用可能であった例外が利用不可になっており、(バンドルにはありませんでしたが)プラグインやカスタマイズされたコードの動作に影響を与えている可能性があります) メール送信専用の共通エラー画面を用意するのであれば、CSRFトークンエラー用のエラー画面と同じように opExecutionFilter のような opMailSend クラスを利用することが想定されるクラス(例えば、アクションクラス)よりも上位のクラスで例外をキャッチし、フォワード(もしくはリダイレクト)の処理を行うべきではないでしょうか。 タスク内からの呼び出し時に関してはエラー画面とは別の考慮が必要ですが、ログにエラーが記録されるのみでタスク内からメール送信処理の失敗が検知できないような変更よりは、仮にうまく共通化ができなかったとしても個別タスク内できちんと例外処理を行うように修正するほうがよいと思います。
Causes¶
#1373 の修正が原因となり、メールの送信に失敗した場合、メール送信のメソッド内で例外を捕捉してしまっているため、上位のクラスで例外を捕捉できない問題が発生している
修正内容¶
#1373 でのコミットを取消、 #1372 の修正と同等の修正内容を取り込む
- sfOpenPNEMailSend::execute() で例外をキャッチしている処理は取消し、適切な場所で例外がキャッチできなくなる問題を改善する。
- このままではアクションの処理の段階で例外が投げられた場合、ユーザ画面に500のレスポンスを返すため、 sfOpenPNEExecutionFilter クラスで例外をキャッチし、適切な画面を表示するように対応した #1372 の修正を取り込み
Related issues
Associated revisions
Squashed commit of the following:
commit 93905c08d7c91ca71152fa0b7e251f5d791cb734
Author: touri <tourimgr@gmail.com>
Date: Wed Aug 17 19:04:42 2011 +0900
fixed to mail error template. (fixes #2360, BP from #1372)
commit 3fc1cdb0aeb224ddf0eaa256ea0a08cc8302037e
Author: touri <tourimgr@gmail.com>
Date: Wed Aug 17 17:21:02 2011 +0900
changed order of process in opMemberAction::executeConfig().
fixed to mail error template. (fixes #2360, BP from #1372)
Conflicts:
lib/action/opMemberAction.class.php
commit 1f56cbf0ca444d19269a9ec9ba5bffa4613b497c
Author: ShinichiU <urabe@nuts-choco.com>
Date: Tue Aug 9 14:19:00 2011 +0900
(refs #1373, BP from #1372) cache Zend_Mail_Protocol_Exception in opExecutionFilter and forwad mailError action.
Conflicts:
lib/filter/opExecutionFilter.class.php
commit c1f6ee4093b994cb483ad17292cc58ced5c243e3
Author: ShinichiU <urabe@tejimaya.com>
Date: Thu Oct 6 20:44:23 2011 +0900
Revert "(fixes #1373) This modification is that modified to catch the exception when sending mail"
This reverts commit c0dd5cb0f8ec1d5c56efb164960a07c228dfeaf5.
History
#1
Updated by Shingo Yamada over 11 years ago
- Priority changed from Normal(通常) to High(高め)
#2
Updated by Shingo Yamada over 11 years ago
- 360対象 set to RC1
#3
Updated by Shingo Yamada over 11 years ago
- 360対象 deleted (
RC1)
#4
Updated by Shinichi Urabe over 11 years ago
- Due date set to 2011-10-06
- Assignee set to Shinichi Urabe
#5
Updated by Shinichi Urabe over 11 years ago
- Status changed from New(新規) to Pending Review(レビュー待ち)
- % Done changed from 0 to 50
更新履歴 3e1322737cb934269ba94fe3d0c3d958d8f5a027 で適用されました。
#6
Updated by Shinichi Urabe over 11 years ago
更新履歴 3e1322737cb934269ba94fe3d0c3d958d8f5a027 で適用されました。
#7
Updated by Shinichi Urabe over 11 years ago
- Description updated (diff)
#8
Updated by Minoru Takai over 11 years ago
- Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
- % Done changed from 50 to 70
本チケットでの修正 3e132273 が http://redmine.openpne.jp/issues/1372#note-21 以降のコミットをマージしたもので、結果的に #1372 のレビュー時点(note-30)と同等の状況になっていることを確認しました。レビューOKです。
#9
Updated by Mutsumi Imamura over 11 years ago
- Status changed from Pending Testing(テスト待ち) to Fixed(完了)
- % Done changed from 70 to 100
動作確認OKです。
#10
Updated by Chiharu Nakajima over 5 years ago
- 3.6 で発生するか set to Unknown (未調査)
3.4系のみで発生(3.6系以降は #1372 で対応済み)