プロジェクト

全般

プロフィール

Bug(バグ) #1687

パスワード再発行処理を行なおうとすると http://opauthmailaddress/ に飛ばされてしまう

pnetan  13年以上前に追加. 8年以上前に更新.

ステータス:
Fixed(完了)
優先度:
High(高め)
担当者:
対象バージョン:
開始日:
2010-10-15
期日:
進捗率:

100%

3.6 で発生するか:
Yes
3.8 で発生するか:
Unknown (未調査)

説明

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 を書き換えることは通常考えられない、この実装でも問題ないと考えます)

generateAppUrlTest.phps (2.84 KB) hiromi hishida, 2010-12-15 11:58


関連するチケット

関連している OpenPNE 3 - Backport(バックポート) #2076: パスワード再発行処理を行なおうとすると http://opauthmailaddress/ に飛ばされてしまう Fixed(完了) 2010-10-15 2011-06-24
関連している OpenPNE 3 - Bug(バグ) #1675: 設定ファイルを記述し忘れた場合に、招待メールに記載されたURLにアクセスできない Fixed(完了) 2010-10-14
関連している OpenPNE 3 - Backport(バックポート) #2681: パスワード再発行処理を行なおうとすると http://opauthmailaddress/ に飛ばされてしまう Invalid(無効) 2010-10-15

関係しているリビジョン

リビジョン 53ddf52e (差分)
Masato Nagasawaほぼ13年前に追加

fixed remove when "/" was included end of the url in the "op_base_url" config(fixes #1687)

リビジョン 6f5cf83d (差分)
Masato Nagasawaほぼ13年前に追加

Revert "fixed remove when "/" was included end of the url in the "op_base_url" config(fixes #1687)"

This reverts commit 53ddf52e4bca623f388c4a9841a3129d45d1664e.

リビジョン eab44bc7 (差分)
Masato Nagasawaほぼ13年前に追加

fixed remove when "/" was included end of the url in the "op_base_url" config(fixes #1687)

履歴

#1 Kousuke Ebihara13年以上前に更新

  • 対象バージョンOpenPNE 3.7.0 にセット
  • 3.6 で発生するかYes にセット

#2 Masato Nagasawa13年以上前に更新

  • ステータスNew(新規) から Accepted(着手) に変更
  • 担当者Masato Nagasawa にセット

#3 Masato Nagasawa13年以上前に更新

  • ステータスAccepted(着手) から New(新規) に変更
  • 担当者 を削除 (Masato Nagasawa)

3.7.0-dev にて確認しましたが、再現できませんでした。
一度手放します。

#4 hiromi hishida13年以上前に更新

元報告者です(http://twitter.com/77web)。
遅ればせながら原因と解決法がわかりました。
メールで届く再発行用URLがhttp://sns.openpne.jp//opAuthMailAddress/~となっているために、form action="//opAuthMailAddress/~"というHTMLが出力されて、ブラウザの仕様により(?)http://opauthmailaddress/~にPOSTしてしまうようです。
修正が必要になるのはopAuthMailAddressで送信するメールの本文を生成している部分(あるいはメールテンプレート)と思われます。

#5 hiromi hishida13年以上前に更新

現象が再現するテストを書きましたので添付します。#1675のチケットと原因が同じだと思います。
手っ取り早い方法としては、メール送信のテンプレート側でapp_url_for()をabsolute=trueで使わずsfConfig::get('op_base_url')とabsolute=falseにしたapp_url_for()の返り値を繋げることで回避できます。

#6 Masato Nagasawa13年以上前に更新

  • プロジェクトOpenPNE 3 から opAuthMailAddressPlugin に変更
  • 対象バージョン を削除 (OpenPNE 3.7.0)
  • [QA]バグ通知済いいえ にセット

#7 Kousuke Ebihara約13年前に更新

  • [QA]バグ通知済いいえ から はい に変更

#8 Shingo Yamadaほぼ13年前に更新

  • 担当者Masato Nagasawa にセット

#9 Masato Nagasawaほぼ13年前に更新

  • ステータスNew(新規) から Accepted(着手) に変更

添付されているテストコードを #1675 の修正適応済みの最新のmasterソースで確認しましたが、
現状でもエラーになっている状態なので、別に修正が必要なようです。

#10 Masato Nagasawaほぼ13年前に更新

この問題は base_url に指定する URL の末尾に"/"を含めた場合に発生します。
2系では末尾に"/"が必須でしたが、3系はこれとは逆の仕様であると思われます。
したがって、この仕様について設定ファイル中に明記されていない事が問題であると思われます。
(2系では設定ファイル中にコメントアウトとして記述されています)

ですがより適切な対応としては、コメントに仕様を記述するよりも、
"/"が末尾に含まれる場合でも正常に動作する方が良いと思いますので、対応可能なように修正を行います。

#11 Masato Nagasawaほぼ13年前に更新

  • プロジェクトopAuthMailAddressPlugin から OpenPNE 3 に変更

#12 Masato Nagasawaほぼ13年前に更新

  • 対象バージョンOpenPNE 3.7.0 にセット

#13 Masato Nagasawaほぼ13年前に更新

  • ステータスAccepted(着手) から Pending Review(レビュー待ち) に変更
  • 進捗率0 から 50 に変更

更新履歴 53ddf52e4bca623f388c4a9841a3129d45d1664e で適用されました。

#14 Naoya Tozukaほぼ13年前に更新

  • ステータスPending Review(レビュー待ち) から Rejected(差し戻し) に変更
  1. base_url が config.yml で定義されていないケースが考慮されていない
  2. キャッシュから読み込んだconfigに対して毎回この処理を行っているが、できればキャッシュには変換済のものを格納したい
  3. 変数 $len が (長さ - 1) を表す数値に用いられているのが若干気持ち悪い(※$lenは変換後の長さとも言えるので見方の問題かも)
  4. '/' はシングルバイト文字なので mb_strlen(), mb_substr() ではなく strlen(), substr() でも可
  5. [mb_]strlen() や [mb_]substr() を使わずに
    if ('/' === substr($url, -1)) 
    {
      $config['base_url'] = substr($url, 0, -1);
    }
    のように書いてもよい
  6. 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 が必要です)

#15 Masato Nagasawaほぼ13年前に更新

  • ステータスRejected(差し戻し) から Pending Review(レビュー待ち) に変更

#16 Masato Nagasawaほぼ13年前に更新

更新履歴 6f5cf83d865ea62d93004552951d4402677dd184 で適用されました。
更新履歴 eab44bc799b72239a0a1d08480067962f64d2864 で適用されました。

#17 Masato Nagasawaほぼ13年前に更新

一度修正を取り消した上で改善案と同様の修正を行いました。

#18 Naoya Tozukaほぼ13年前に更新

  • ステータスPending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
  • 進捗率50 から 70 に変更

OKです。修正どうもありがとうございました。

#19 Fumie Toyooka12年以上前に更新

  • ステータスPending Testing(テスト待ち) から Fixed(完了) に変更
  • 進捗率70 から 100 に変更

テストOKです

#20 kaoru n8年以上前に更新

  • 3.8 で発生するかUnknown (未調査) にセット

他の形式にエクスポート: Atom PDF