Bug(バグ) #1687
完了パスワード再発行処理を行なおうとすると http://opauthmailaddress/ に飛ばされてしまう
100%
説明
Overview (現象)¶
1. 「パスワードを忘れた方へ」リンクよりパスワード再発行の申請を行う
2. パスワード再発行メールに
http://sns.example.com//opAuthMailAddress/passwordRecoveryComplete/token/hogehoge/id/XX
というURLが記載されているので、このURLにアクセスする
3. 新しいパスワード入力して、決定ボタンを押下すると、
http://opauthmailaddress/passwordRecoveryComplete/token/hogehoge/id/XX
というURLに飛ばされてしまう
元の報告:
http://twitter.com/77web/statuses/27406372500
確認バージョン¶
- 3.6beta6 再現した
- 3.4 未確認
Causes (原因)¶
この問題は base_url に指定する URL の末尾に"/"を含めた場合に発生します。
2系では末尾に"/"が必須でしたが、3系はこれとは逆の仕様であると思われます。
したがって、この仕様について設定ファイル中に明記されていない事が問題であると思われます。
(2系では設定ファイル中にコメントアウトとして記述されています)
Way to fix (修正内容)¶
適切な対応としては、コメントに仕様を記述するよりも、
"/"が末尾に含まれる場合でも正常に動作する方が良いと思いますので、対応可能なように修正を行います。
OpenPNE.ymlがロードされるタイミング(opProjectConfiguration::setOpenPNEConfiguration())で、"/"が含まれていた場合に除去を行います。
この実装の場合、添付されているテストコードではsfConfigを直接書き換えているためエラーのままとなります。
また、op_base_url を書き換えた場合も"/"の除去は機能しません。
URL生成メソッドの opApplicationConfiguration::getAppRouting() 中に処理を記述することで対応は可能ですが、
この場合URL生成時に毎回処理を実行するため無駄な処理が走る事、他の箇所で参照されていた場合を考慮してこの実装は行いません。
(op_base_url を書き換えることは通常考えられない、この実装でも問題ないと考えます)
ファイル
Masato Nagasawa さんが約14年前に更新
- ステータス を New(新規) から Accepted(着手) に変更
- 担当者 を Masato Nagasawa にセット
Masato Nagasawa さんが約14年前に更新
- ステータス を Accepted(着手) から New(新規) に変更
- 担当者 を削除 (
Masato Nagasawa)
3.7.0-dev にて確認しましたが、再現できませんでした。
一度手放します。
hiromi hishida さんが約14年前に更新
元報告者です(http://twitter.com/77web)。
遅ればせながら原因と解決法がわかりました。
メールで届く再発行用URLがhttp://sns.openpne.jp//opAuthMailAddress/~となっているために、form action="//opAuthMailAddress/~"というHTMLが出力されて、ブラウザの仕様により(?)http://opauthmailaddress/~にPOSTしてしまうようです。
修正が必要になるのはopAuthMailAddressで送信するメールの本文を生成している部分(あるいはメールテンプレート)と思われます。
hiromi hishida さんが約14年前に更新
現象が再現するテストを書きましたので添付します。#1675のチケットと原因が同じだと思います。
手っ取り早い方法としては、メール送信のテンプレート側でapp_url_for()をabsolute=trueで使わずsfConfig::get('op_base_url')とabsolute=falseにしたapp_url_for()の返り値を繋げることで回避できます。
Masato Nagasawa さんが約14年前に更新
- プロジェクト を OpenPNE 3 から opAuthMailAddressPlugin に変更
- 対象バージョン を削除 (
OpenPNE 3.7.0) - [QA]バグ通知済 を いいえ にセット
Masato Nagasawa さんが13年以上前に更新
- ステータス を New(新規) から Accepted(着手) に変更
添付されているテストコードを #1675 の修正適応済みの最新のmasterソースで確認しましたが、
現状でもエラーになっている状態なので、別に修正が必要なようです。
Masato Nagasawa さんが13年以上前に更新
この問題は base_url に指定する URL の末尾に"/"を含めた場合に発生します。
2系では末尾に"/"が必須でしたが、3系はこれとは逆の仕様であると思われます。
したがって、この仕様について設定ファイル中に明記されていない事が問題であると思われます。
(2系では設定ファイル中にコメントアウトとして記述されています)
ですがより適切な対応としては、コメントに仕様を記述するよりも、
"/"が末尾に含まれる場合でも正常に動作する方が良いと思いますので、対応可能なように修正を行います。
Masato Nagasawa さんが13年以上前に更新
- ステータス を Accepted(着手) から Pending Review(レビュー待ち) に変更
- 進捗率 を 0 から 50 に変更
更新履歴 53ddf52e4bca623f388c4a9841a3129d45d1664e で適用されました。
Naoya Tozuka さんが13年以上前に更新
- ステータス を Pending Review(レビュー待ち) から Rejected(差し戻し) に変更
- base_url が config.yml で定義されていないケースが考慮されていない
- キャッシュから読み込んだconfigに対して毎回この処理を行っているが、できればキャッシュには変換済のものを格納したい
- 変数 $len が (長さ - 1) を表す数値に用いられているのが若干気持ち悪い(※$lenは変換後の長さとも言えるので見方の問題かも)
- '/' はシングルバイト文字なので mb_strlen(), mb_substr() ではなく strlen(), substr() でも可
- [mb_]strlen() や [mb_]substr() を使わずに
if ('/' === substr($url, -1)) { $config['base_url'] = substr($url, 0, -1); }
のように書いてもよい - 3.〜5. については、キャッシュされるのであれば正規表現を用いシンプルにしてはどうか
具体的には、
protected function setOpenPNEConfiguration() { $opConfigCachePath = sfConfig::get('sf_cache_dir').DIRECTORY_SEPARATOR.'config'.DIRECTORY_SEPARATOR.'OpenPNE.yml.php'; if (is_readable($opConfigCachePath)) { $config = (array)include($opConfigCachePath); } else { $path = OPENPNE3_CONFIG_DIR.'/OpenPNE.yml'; $config = sfYaml::load($path.'.sample'); if (is_readable($path)) { $config = array_merge($config, sfYaml::load($path)); } + if (isset($config['base_url'])) + { + $config['base_url'] = preg_replace('/\/$/', '', $config['base_url']); + } + opToolkit::writeCacheFile($opConfigCachePath, '<?php return '.var_export($config, true).';'); } $this->configureSessionStorage($config['session_storage']['name'], (array)$config['session_storage']['options']); unset($config['session_storage']); - $url = $config['base_url']; - $len = mb_strlen($url) - 1; - if (0 < $len && '/' == mb_substr($url, $len, 1)) - { - $config['base_url'] = mb_substr($url, 0, $len); - } foreach ($config as $key => $value) { sfConfig::set('op_'.$key, $value); } }のような対応が望ましいと思われます。
(但し、既にキャッシュされているものについては適用されないので本修正の反映後には symfony cc が必要です)
Masato Nagasawa さんが13年以上前に更新
更新履歴 6f5cf83d865ea62d93004552951d4402677dd184 で適用されました。
更新履歴 eab44bc799b72239a0a1d08480067962f64d2864 で適用されました。
Naoya Tozuka さんが13年以上前に更新
- ステータス を Pending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
- 進捗率 を 50 から 70 に変更
OKです。修正どうもありがとうございました。
Fumie Toyooka さんが約13年前に更新
- ステータス を Pending Testing(テスト待ち) から Fixed(完了) に変更
- 進捗率 を 70 から 100 に変更
テストOKです