プロジェクト

全般

プロフィール

Bug(バグ) #2408

サイドバナーのRSSリーダーでURLに特殊文字が含まれる場合に正しいRSSフィードが得られない

Yuya Watanabeほぼ7年前に追加. ほぼ3年前に更新.

ステータス:
Fixed(完了)
優先度:
Normal(通常)
担当者:
対象バージョン:
開始日:
2011-09-17
期日:
進捗率:

100%

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

説明

概要

サイドバナーのRSSリーダーを利用する際,URLに特殊文字が含まれる場合に正しいRSSフィードが得られない.

確認手順

  1. pc_backend.php/design/gadget/type/sideBannerで「RSSリーダー」のガジェットを追加する
  2. 「RSSリーダー」をクリックして下記URLを登録する
    http://video.baidu.jp/api/search?word=バイク&output=rss&start=0&count=12&sort=dt_recent&adult_filter=1
    
  3. ログイン画面等のサイドバナーが表示されるページで表示される結果を確認する
    • Firefox等の別のRSSリーダーで表示される結果と違う

原因

下記コード部において$this->gadgetがGadgetクラスをsfOutputEscaperIteratorDecoratorクラスでラップされているため,特殊文字がエスケープされた文字列が取得されてしまう.

apps/pc_frontend/modules/default/actions/components.class.php

 98     try
 99     {
100       $fetcher = new opRssFetcher('UTF-8');
101       $this->result = @$fetcher->fetch($this->gadget->getConfig('url'), true);
102       if ($this->result)
103       {
104         $this->result[1] = array_slice($this->result[1], 0, 5);
105       }
106     }

修正案

  1. $this->gadgetがsfOutputEscaperIteratorDecoratorクラスでラップせずにGadgetクラスでexecuteRssBox()に渡すようにする
  2. executeRssBox()内で$this->gadgetのsfOutputEscaperIteratorDecoratorのラップを剥がして値を取得する

実装案

修正案1の実装方法が確認できなかったため修正案2について実装案を提示.おそらく以下の修正で修正案2に相当すると思われる.

diff --git a/apps/pc_frontend/modules/default/actions/components.class.php b/apps/pc_frontend/modules/default/actions/components.class.php
index ad79046..06b97c4 100644
--- a/apps/pc_frontend/modules/default/actions/components.class.php
+++ b/apps/pc_frontend/modules/default/actions/components.class.php
@@ -98,7 +98,7 @@ class defaultComponents extends sfComponents
     try
     {
       $fetcher = new opRssFetcher('UTF-8');
-      $this->result = @$fetcher->fetch($this->gadget->getConfig('url'), true);
+      $this->result = @$fetcher->fetch(sfOutputEscaper::unescape($this->gadget)->getConfig('url'), true);
       if ($this->result)
       {
         $this->result[1] = array_slice($this->result[1], 0, 5);

確認バージョン

OpenPNE 3.7.0-dev, 3.6beta13

実際の修正

各アプリケーションのconfig/settings.ymlのescaping_strategy部分を'on'からtrue,'off'からfalseに置換する.


関連するチケット

関連している OpenPNE 3 - Bug(バグ) #1684: サイドバナーのRSSリーダーで読み込めない時がある Fixed(完了) 2010-10-15
関連している OpenPNE 3 - Bug(バグ) #2407: 管理画面ガジェット設定で&などのエスケープ文字を入力すると元の文字で表示される New(新規) 2011-09-17
関連している OpenPNE 3 - Backport(バックポート) #2731: サイドバナーのRSSリーダーでURLに特殊文字が含まれる場合に正しいRSSフィードが得られない Fixed(完了) 2011-09-17
関連している OpenPNE 3 - Backport(バックポート) #2732: サイドバナーのRSSリーダーでURLに特殊文字が含まれる場合に正しいRSSフィードが得られない Fixed(完了) 2011-09-17

関係しているリビジョン

リビジョン e934a366 (差分)
Yuya Watanabeほぼ7年前に追加

(fixes #2408) replace setting value to unescape component

リビジョン 58120919 (差分)
Yuya Watanabe6年以上前に追加

(fixes #2408) add patch to symfony-1.4.13 and patch not to unescape using get_partial/include_partial

リビジョン 543738d5 (差分)
Yuya Watanabe6年以上前に追加

(fixes #2408) add patch to symfony-1.4.13 and patch not to unescape using get_partial/include_partial

履歴

#1 Kousuke Ebiharaほぼ7年前に更新

  • 360対象RC1 にセット

#2 Kousuke Ebiharaほぼ7年前に更新

ガジェットのレンダリングに使用されている get_component() ヘルパー関数に、以下のようなコードが存在しています。

$view->setPartialVars(true === sfConfig::get('sf_escaping_strategy') ? sfOutputEscaper::unescape($vars) : $vars);

ビューから渡ってきた HTML エスケープ済みの値をロジックに渡す前にアンエスケープすることを目的としたコードですが、実は、 OpenPNE は sf_escaping_strategy の値が true ではなく "on" となっているために、データが アンエスケープされないまま ロジックに渡されてしまいます。これがこのバグの原因となっています。

YAML ファイル内の bool 値の表現は symfony 1.4 へのアップデート時に変更がおこなわれました( https://groups.google.com/group/openpne-dev/browse_thread/thread/a2f2e2798fd308f3/15214bdaa90644d2?hl=ja&lnk=gst&q=yaml#15214bdaa90644d2 )。いままで bool 値として扱われていた on や off などの表記がただの文字列として扱われるように変更されています。

symfony 1.4 への対応をおこなう過程でこのような代替表現は置き換えられたものとばかり思っていましたが、 on が文字列として扱われたところで、 PHP の世界で bool に変換すれば true になるのだから同じことだとして on 等の表現についてはあまり真面目に置換がおこなわれなかったのかもしれません。

さて、symfony が意図するように、ロジック側にデータを引き渡す前に一度アンエスケープされるよう修正する、つまり、 sf_escaping_strategy が確実に true として扱われるように設定ファイルを変更するのが理想的ですが、 OpenPNE で使用しているすべてのコンポーネントに影響がありうる変更なため、本当にこの策を採っていいかどうかについてはよく考えるべきです。

#3 Kousuke Ebiharaほぼ7年前に更新

コアとすべてのバンドルプラグインに存在する、すべてのコンポーネントの実装を確認しましたが、コンポーネントのロジック側では sfOutputEscaper にラップされていることを想定したコードは存在しないであろうことがわかりました。

現時点で sf_escaping_strategy が "on" となってしまうように設定されている設定ファイルをすべて true に変更するように対応をおこなってください。

#4 Yuya Watanabeほぼ7年前に更新

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

#5 wa taほぼ7年前に更新

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

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

#6 Kousuke Ebiharaほぼ7年前に更新

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

#7 Yuya Watanabeほぼ7年前に更新

  • 説明 を更新 (diff)

#8 Kousuke Ebihara6年以上前に更新

  • ステータスPending Testing(テスト待ち) から Rejected(差し戻し) に変更
  • 進捗率70 から 50 に変更
  • 3.6 で発生するかUnknown (未調査) にセット
  • 3.4 で発生するかUnknown (未調査) にセット

この修正により、コンポーネントだけでなくパーシャルでもアンエスケープされないままデータが渡されるようになってしまいました。パーシャルではこれ以降エスケープ処理がおこなわれないため、この時点でアンエスケープをするのは不適切です。

#9 Yuya Watanabe6年以上前に更新

  • ステータスRejected(差し戻し) から Accepted(着手) に変更
  • 進捗率50 から 0 に変更

#10 Yuya Watanabe6年以上前に更新

escaping_strategy によって get_component および get_partial に影響をあたえるようになったのは以下のチケットおよびチェンジセットです.

http://trac.symfony-project.org/ticket/7825
http://trac.symfony-project.org/changeset/27755

コンポーネント
lib/vendor/symfony/lib/helper/PartialHelper.php 141行目

134 function get_component($moduleName, $componentName, $vars = array())
135 { 
136   $context = sfContext::getInstance();
137   $actionName = '_'.$componentName;
138     
139   $class = sfConfig::get('mod_'.strtolower($moduleName).'_partial_view_class', 'sf').'PartialView';
140   $view = new $class($context, $moduleName, $actionName, '');
141   $view->setPartialVars(true === sfConfig::get('sf_escaping_strategy') ? sfOutputEscaper::unescape($vars) : $vars);
142   
143   if ($retval = $view->getCache())
144   {
145     return $retval;
146   }
147 
148   $allVars = _call_component($moduleName, $componentName, $vars);
149 
150   if (null !== $allVars)
151   {
152     // render
153     $view->getAttributeHolder()->add($allVars);
154 
155     return $view->render();
156   }
157 }

パーシャル
lib/vendor/symfony/lib/helper/PartialHelper.php 216行目

198 function get_partial($templateName, $vars = array())
199 {
200   $context = sfContext::getInstance();
201 
202   // partial is in another module?
203   if (false !== $sep = strpos($templateName, '/'))
204   {
205     $moduleName   = substr($templateName, 0, $sep);
206     $templateName = substr($templateName, $sep + 1);
207   }
208   else
209   {
210     $moduleName = $context->getActionStack()->getLastEntry()->getModuleName();
211   }
212   $actionName = '_'.$templateName;
213 
214   $class = sfConfig::get('mod_'.strtolower($moduleName).'_partial_view_class', 'sf').'PartialView';
215   $view = new $class($context, $moduleName, $actionName, '');
216   $view->setPartialVars(true === sfConfig::get('sf_escaping_strategy') ? sfOutputEscaper::unescape($vars) : $vars);
217 
218   return $view->render();
219 } 

この問題を OpenPNE 単体で対処するには以下の2つの方法があると思います.

  1. escaping_strategy を false にして get_component() を用いる際の引数を,与える時点でアンエスケープする.
  2. escaping_strategy を true にして get_partial()/include_partial() を用いる際の引数をエスケープする.

Symfony のコードを変更することが許容されるならば escaping_strategy を true にして get_partial() のアンエスケープ部分を削除することが最も簡単だと思います.

#11 Yuya Watanabe6年以上前に更新

OpenPNE 単体での解決が難しいため, Symfony に対してパッチを当てる方針とします.

#12 Yuya Watanabe6年以上前に更新

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

更新履歴 5812091917cf75337151aa0ba8ebc770359fe4a1 で適用されました。

#13 Kousuke Ebihara6年以上前に更新

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

#14 Shouta Kashiwagi6年以上前に更新

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

テストOKです。

#15 kaoru nishizoeほぼ3年前に更新

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

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