フィードバック内容¶
$this->serializedMember の上書きに関して, 84 行目のように if の条件式で false になればよいですが,既存の実装ではキャッシュが空であることを空文字列で表しているようであり文字列のみを扱えば良い状態ですがここで扱われるべきデータに null が追加されるのが気になるため空文字列で初期化してください.
lib/user/opSecurityUser.class.php
84: if ($this->serializedMember)
lib/user/opSecurityUser.class.php
23: $serializedMember = '';
42: $this->serializedMember = '';
99: $this->serializedMember = serialize($member);
レビューメモ¶
opSecurityUser#setMemberId() が呼ばれる段階
- sfContext の初期化時
- executeRegister() および executeRegisterInput() 実行時
opSecurityUser#getMember() で opSecurityUser にメンバ情報がメモリにキャッシュされる. sfContext 初期化時には opSecurityUser がインスタンス化される段階で setMemberId() された上で getMember() を呼んでいるためこの段階でキャッシュされる. setMemberId() でこのキャッシュを削除しない状態だと getMember() は sfContext 初期化時にキャッシュされたメンバが取得されるため(また,このメンバは is_remember_me の Cookie があることから is_active=1 であるはずであり opAuthAdapterMailAddress の) isRegisterBegin() が false となり 404 に遷移する.
期待される内容としては
- opSecurityUser の getMember() が現在のメンバIDに対応するメンバ情報を取得する
- セッションの member_id が現在のメンバIDを表している
という前提だと問題としては opSecurityUser から取得されるメンバ ID がキャッシュの ID とセッションにある ID で一致しないということであることになる.
対応方法としては下記のものが考えられる.
- getMember() を行う際にメンバ情報をキャッシュしない
- sfContext の初期化時に正しく setMemberId() を行うようにする
- executeRegister() および executeRegisterInput() 実行時に呼ばれる setMemberId() でキャッシュを削除する
この内 getMember() でキャッシュしないようにすることは getMember() の呼ばれる頻度から正しい対応とは言えない.また setMemberId() でキャッシュである serialiedMember を上書きすることが 3 番目の対応となり, executeRegister() で logout() に変更することが executeRegisterInput() 時における 2 番目の対応ということになるはず.setMemberId() を行うかは URL やパラメータなどによって決定されるため sfContext の初期化時点ではセットされる member_id が正しく決定できないと思うため修正内容としては妥当ではあるがリクエスト時に一瞬でも以前のメンバでログイン状態となるところは気になる.(ユーザが明示的な操作を行わない限りはメンバのステートは次のリクエストの sfContext 初期化時までを含むという考え方を持ってないと気になる)
sfContext 初期化時における setMemberId までの流れ
getRememberedMemberId() の中で Cookie を見て対応する member_id を DB から取得してくる. 得られた member_id を帰って来た initializeUserStatus() でセッションに保存する.