OpenPNE 3

更新履歴 371b27cb02df1a251dda0fcbf1646a388cdd0af3 で適用されました。

以下の疑問について確認するため、一旦差し戻します。

疑問¶

今回の修正では parse_url() 直後に

      $parts['path'] = isset($parts['path']) ? $parts['path'] : '/';
      $parts['host'] = isset($parts['host']) ? $parts['host'] : '';

を追記していますが、この変更内容は問題ないものなのでしょうか。

• base_url のURLにパスが含まれない場合に、 $parts['path'] が '/' であるものとしてしまってよいのでしょうか。
  • $parts['path'] を '' （空文字列）としておくよりも '/' である方がよいのでしょうか。
• $parts['host'] に対しても同様の記述がありますが、この host に対する記述は必要なものなのでしょうか。もしこれが必要だとしたら、他にも併記すべき記述はないでしょうか。
• この修正後に、 管理画面のヘッダ部にある（フロントエンドへの）リンクのURLが http://sns.example.com// のように末尾にスラッシュが連続(*1)してしまっていますが、これはこの修正に際して認識している挙動でしょうか（ #1632 で関連問題が指摘されていますが、これ(*1)と同一の内容なのかは判断できていません）。

備考¶

この問題は、チケットで示されている通り opApplicationConfiguration::getAppRouting() メソッド内で parse_url() で得た連想配列に対して、定義されていない可能性のある 'path' インデックスの値を参照しているという問題です。

• 修正前の lib/config/opApplicationConfiguration.class.php
  

      $url = sfConfig::get('op_base_url');
      if ('http://example.com' !== $url)
      {
        $parts = parse_url($url);
  
        $options['context'] = array(
          'prefix' => $this->getAppScriptName($application, sfConfig::get('sf_environment'), $parts['path'], $isNoScriptName),
          'host'   => $parts['host'],
        );
      }
  

• A. URLがホスト名（直後にスラッシュなし）で終わる場合（パスなし）
  

  $ php -r 'var_dump(parse_url("http://example.com"));'
  
  array(2) {
    ["scheme"]=>  string(4) "http" 
    ["host"]=>  string(11) "example.com" 
  }
  

• B. URLがホスト名（直後にスラッシュあり）で終わる場合（パスあり）
  

  $ php -r 'var_dump(parse_url("http://example.com/"));'
  
  array(3) {
    ["scheme"]=>  string(4) "http" 
    ["host"]=>  string(11) "example.com" 
    ["path"]=>  string(1) "/" 
  }
  

• C. URLの末尾にスラッシュがないが、パスを含む場合（パスあり）
  

  $ php -r 'var_dump(parse_url("http://example.com/path/to"));'
  
  array(3) {
    ["scheme"]=>  string(4) "http" 
    ["host"]=>  string(11) "example.com" 
    ["path"]=>  string(8) "/path/to" 
  }
  

この問題が生じるのは「 base_url の末尾に '/' がない場合（AまたはC）」ではなく、「 base_url にパスが含まれない場合（A）」です。チケットタイトルは少し語弊があるかもしれません。

To: Takai, Ebihara

指摘把握です。
Takai さんが、 Ebihara さんの まとめ に納得したのであれば、それで修正させていただきます。

更新履歴 7a7e20ce4b78c4331ffa70bf337f09ae0f484645 で適用されました。

note-15 をレビューしました。一点疑問があります。

note-15 の修正前と修正後の違い¶

• $options['context'] の値について： note-15 修正前
  

  -      $options['context'] = array(
  -        'prefix' => $this->getAppScriptName($application, sfConfig::get('sf_environment'), $parts['path'], $isNoScriptName),
  -        'host'   => $parts['host'],
  -      );
  

  • 修正前は、このブロック内で $options['context'] = array() という形で代入されているので $options['context'] に元々あった値は上書きされており、（このコードから明らかですが） $options['context'] には prefix と host というキーしかありません。
    • 手元の環境で確認した、このブロック直後での $options['context'] の値（ sns.example.com の部分は書き換えました）
      

      array
        'prefix' => string '/' (length=1)
        'host' => string 'sns.example.com' (length=15)
      

• $options['context'] の値について： note-15 修正後
  

  +      $options['context']['prefix'] =
  +        $this->getAppScriptName($application, sfConfig::get('sf_environment'), $parts['path'], $isNoScriptName);
  
  +      if (isset($parts['host']))
  +      {
  +        $options['context']['host'] = $parts['host'];
  +      }
  

  • 修正後は $options['context']['prefix'] を追加する記述から始まっており、$options['context'] に元々あった値が残っています。
    • 手元の環境で確認した、このブロック直後での $options['context'] の値（ sns.example.com の部分は書き換えました。また、この確認は管理画面 pc_backend.php で行いました）
      

      array
        'path_info' => string '/' (length=1)
        'prefix' => string '' (length=0)
        'method' => string 'GET' (length=3)
        'format' => null
        'host' => string 'sns.example.com' (length=15)
        'is_secure' => boolean false
        'request_uri' => string 'http://sns.example.com/pc_backend.php' (length=37)
      

疑問¶

• $options['context'] に含まれている値を残してしまっていることは問題ないでしょうか。
• あるいは逆に、修正前の記述で（もともとの値を）上書きしてしまっていることは問題であると考えられるでしょうか。

この疑問は、補足で示している #1155 のコミット 59527e56 に対して言えるものかもしれません。

補足¶

以下は getAppRouting() が追記された #1155 のコミット 59527e56 でのコードですが、この時点で if 文を通らない場合は $options['context'] に含まれている値が残っていたということになります。つまり、このチケットでの修正前のコードの時点で以下 532 行目の if 文を通るか否かで $options['context'] の値を上書きするか否かが異なっていたということです。

530-    $options = $context->getRouting()->getOptions();
531-    $url = sfConfig::get('op_base_url');
532-    if ('http://example.com' !== $url)
533-    {
534-      $parts = parse_url($url);
535:      $options['context'] = array(
536-        'prefix' => $this->getAppScriptName($application, sfConfig::get('sf_environment'), $parts['path'], $isNoScriptName),
537-        'host'   => $parts['host'],
538-      );
539-    }
540-    else
541-    {
542:      $path = dirname($options['context']['prefix']);
543:      $options['context']['prefix'] = $this->getAppScriptName($application, sfConfig::get('sf_environment'), $path, $isNoScriptName);
544-    }

$options['context'] の元々の値を上書きしても残していてもどちらでもよいというのであれば note-15 の修正は問題ないと思います。

(note-15) これは見落としでした。

修正コード中での $options['context'] を利用する sfPatternRouting での挙動も考えつつ修正案出します。

(note-17 - note-19) 確認ありがとうございます。 note-18 の考察内容を以て note-16 の疑問が晴れるため、 note-15 の修正はOKとします。（万一、 note-18 の内容が誤っていたとしても、その誤りが発覚した時点で新たに問題提起をして解決することを予定します。）

ここまでの修正を以て、このチケットの内容には対応できたと判断します（コーディング規約の観点からも、特に問題はないと思うのでOKとします）。

動作テスト¶

この問題は、ソースコードの修正さえ見れば改善されているか否かは明らかであり、動作テストの重点はコードチェックに置かれます。ソースコードの修正は妥当だと判断できており、このチケットへの対応はOKと判断できます。

ついでにですが、 http://sns.example.com/pc_backend_dev.php のようなURLとなるSNS（テスト環境）において、修正前のコードでは「未定義の $parts['path'] を参照している旨の warning が表示されていること」を確認し、修正後のコードではこの警告が表示されなくなったことを確認しました。

完了とします。