OpenPNE 3

CAPTCHA はコア側で用意している機能です

調査途中¶

下記部分ではバリデートエラーとして例外を投げているが，この例外がうまく機能していない？

lib/form/opCaptchaForm.class.php

 39   public function validateCaptchaString($validator, $value, $arguments)
 40   {
 41     $answer = '';
 42     if (isset($_SESSION['captcha_keystring']))
 43     {
 44       $answer = $_SESSION['captcha_keystring'];
 45       unset($_SESSION['captcha_keystring']);
 46     }
 47 
 48     if ($value['captcha'] !== $answer)
 49     {
 50       $error = new sfValidatorError($validator, 'invalid');
 51 
 52       throw new sfValidatorErrorSchema($validator, array('captcha' => $error));
 53     }
 54 
 55     return $value;
 56   }

例えばopAuthMailAddressPluginでは下記のようにフォームを用いている．

plugins/opAuthMailAddressPlugin/lib/form/opRequestRegisterURLForm.class.php

 37     if (sfConfig::get('op_is_use_captcha', false))
 38     {
 39       $this->embedForm('captcha', new opCaptchaForm()); 
 40     }

まず、 note-6 で示されている独自のコールバックのバリデータ部分を通っているか検証しました。 throw の直前で適当に echo などをして exit してみましたが、ここは通っていました。

独自のコールバックでのバリデータの記述が悪いのか、あるいは captcha ウィジェットに対する全てのエラーメッセージが渡っていないのか、あるいはエラーメッセージが渡っているがテンプレート側で出力されていないだけなのかを区別するため、とりあえず次のような変更を加えて動作を確認してみました。

diff --git a/lib/form/opCaptchaForm.class.php b/lib/form/opCaptchaForm.class.php
index c9f8ae2..ad33c0c 100644
--- a/lib/form/opCaptchaForm.class.php
+++ b/lib/form/opCaptchaForm.class.php
@@ -20,7 +20,7 @@ class opCaptchaForm extends BaseForm
   public function configure()
   {
     $this->setWidget('captcha', new opWidgetFormCaptcha());
-    $this->setValidator('captcha', new sfValidatorPass());
+    $this->setValidator('captcha', new sfValidatorString(array('max_length' => 4)));

     $formatter = new sfWidgetFormSchemaFormatterList($this->widgetSchema);
     $formatter->setRowFormat("<li>%field%%help%\n%hidden_fields%</li>\n");

これは captcha に入力した文字列の長さが 4 を超える場合はエラーとなるようなバリデータを使った状態です。これで正しい captcha を入力しても、長さに関してエラーとなるようになりますが、これを実際に試したところ、エラーメッセージは表示されませんでした。

これで、コールバックの独自バリデータが悪いというわけではなさそうであることが確認できました。

次に、エラーメッセージが渡っているかどうかを確認するため、次の変更を加えました。

diff --git a/apps/pc_frontend/modules/opAuthMailAddress/templates/requestRegisterURLInput.php b/apps/pc_frontend/modules/opAuthMailAddress/templates/requestRegi
index 4af40e8..12f91ee 100644
--- a/apps/pc_frontend/modules/opAuthMailAddress/templates/requestRegisterURLInput.php
+++ b/apps/pc_frontend/modules/opAuthMailAddress/templates/requestRegisterURLInput.php
@@ -1,7 +1,12 @@
 <?php slot('_request_register_url_body') ?>
 <?php echo __('Please input your e-mail address. Invitation for %1% will be sent to your e-mail address.', array('%1%' => $op_config['sns_name'])) ?>
 <?php end_slot(); ?>
-
+<?php
+  //var_dump($form['captcha']->hasError());
+var_dump($form['captcha']->renderError());
+//var_dump($form['captcha']);
+exit;
+?>
 <?php echo op_include_form('requestRegisterURL', $form, array(
   'title' => __('Register'),
   'body' => get_slot('_request_register_url_body'),

これを実行したところ、

string '  <div class="error"><ul class="error_list">
    <li>4文字以内で入力してください。</li>
    <li>正しくありません。</li>
  </ul></div>
' (length=157)

のような結果が得られました。つまりテンプレート側にエラーメッセージが渡っており、取得できることが確認できました。

embedForm を使っているせいか分かりませんが、フォームの入力内容に不備があって再表示される際に captcha ウィジェットに関してはエラーメッセージが自動的に表示されていない状態であったということです。

補足¶

http://www.symfony-project.org/more-with-symfony/1_4/ja/06-Advanced-Forms に「カスタムバリデータを作る」という節がありますが、ここで次のように示されています。

      // この埋め込みフォームでのエラー
      if (count($errorSchemaLocal))
      {
        $errorSchema->addError($errorSchemaLocal, (string) $key);
      }
    }

    // エラーをメインフォームへスロー
    if (count($errorSchema))
    {
      throw new sfValidatorErrorSchema($this, $errorSchema);
    }

embedForm を用いる場合はこのようにメインとなるフォームにエラーを渡す必要があるのかもしれません。

note-8 から更に調べて修正しました。 note-9 で修正していますが、修正差分は次のものです。

commit 872e491b1ecf5c9606201c91cdc724008a4dcdb6
Author: mtakai <m.takai@tejimaya.com>
Date:   Mon Feb 6 23:42:50 2012 +0900

    (fixes #1193) use renderError() and render() instead of renderRow() to output error messages in global/partsForm

diff --git a/apps/pc_frontend/templates/_partsForm.php b/apps/pc_frontend/templates/_partsForm.php
index dbac137..5b826c3 100644
--- a/apps/pc_frontend/templates/_partsForm.php
+++ b/apps/pc_frontend/templates/_partsForm.php
@@ -94,7 +94,10 @@ if ($options['mark_required_field']
   $hasRequiredField = true;
 }
 ?>
-<?php echo $field->renderRow($attributes, $field->renderLabelName().$labelSuffix) ?>
+<tr>
+<th><?php echo $field->renderLabel($field->renderLabelName().$labelSuffix); ?></th>
+<td><?php echo $field->renderError(), $field->render($attributes); ?></td>
+</tr>
 <?php endforeach; ?>
 <?php endforeach; ?>
 <?php echo $options->getRaw('lastRow') ?>

生じていた問題¶

新規登録用URL送信フォーム（招待なしの新規登録フォーム）、および招待フォームで op_include_form() が使われており、それにより global/_partsForm.php が呼ばれていましたが、 partsForm では少なくとも CAPTCHA に関してウィジェットに対するエラーが出力されていませんでした。

この問題は CAPTCHA に限らず partsForm によって表示される全てのフォーム項目に共通する内容ではないかと思いましたが、このコメントでの修正を適用する前でも、設定変更ページのフォームなどでは、 CAPTCHA 以外の項目についてはエラーメッセージが出力されていました。

原因¶

symfony の form クラスが用意しているメソッドや使い方をあまり把握していませんが、 renderRow() を用いており、 CAPTCHA の場合に内部で renderError() が行なわれていませんでした。 CAPTCHA に限って出力されていなかった原因は調査できていません。

そのため、 renderRow() メソッドと同等の HTML を直に書き、 render() 相当の出力の直前に renderError() を出力するように書き換えました。

たまたま見つけましたが、 #911 の a79b33cc の editSuccess.php でも、目的が違うかもしれませんが同様の修正を行なっているようです。

関連する問題¶

このチケットの問題とは異なりますが、関連する問題を見つけたのでコメントしておきます。

• opAuthMailAddressPlugin の新規登録用URL送信フォーム（招待なしの新規登録フォーム）、および招待フォームにおいて、メールアドレスのバリデートにおけるエラーメッセージがウィジェットに対するものではなく globalError に出力されています
• 今回の修正にもありますが、フォームコントロールのラベル名には、必須項目である場合に <strong>*</strong> のような文字列が末尾に付きますが、バリデータが sfValidatorPass か sfValidatorSchema の場合にはこれが出力されないようになっています（新規登録用URL送信フォームや招待フォームでは、メールアドレスも CAPTCHA も必須であり $validator->getOption('required') が true であるのに、必須マークが出力されていません）
  

  apps/pc_frontend/templates/_partsForm.php
  88-if ($options['mark_required_field'] 
  89-  && !($validator instanceof sfValidatorPass)
  90-  && !($validator instanceof sfValidatorSchema)
  91-  && $validator->getOption('required'))
  92-{
  93-  $labelSuffix = ' <strong>*</strong>';
  94-  $hasRequiredField = true;
  95-}
  96-?>
  97-<tr>
  98-<th><?php echo $field->renderLabel($field->renderLabelName().$labelSuffix); ?></th>
  99-<td><?php echo $field->renderError(), $field->render($attributes); ?></td>
  100-</tr>
  

レビュアーおよびテスターの方へ¶

• 修正内容が妥当かどうかは、レビュアーの方のチェックに委ねます（実装者としても symfony のフォーム周りや、 partsForm パーツのもともとの実装の意図を知らないので、これが適切でない修正の可能性がある）。
  • http://www.symfony-project.org/gentle-introduction/1_4/ja/10-Forms でメソッドを確認して、 renderRow() がもともと何を出力していたかをデバッグ出力等で確認し、それと同等の HTML を記述したつもりです。
• 今回、 partsForm という共通のテンプレートファイル（ op_include_form() を使うテンプレート全てから参照されるファイル）を書き換えています。そのため、動作テストに関しては、このチケットで言及されているページだけでなく、他のフォームページ（例えば、設定変更ページ）の正常系動作も確認したほうが良いかもしれません。
  • 必須マークやHTMLの文法など、細かい点については実装者である私自身が注意していますが、テスターテストの目的と、その動作テストによって保証されるべき内容について意識されるべきであるということです。

このチケットでの修正についてコメントします。

symfony の form クラスが用意しているメソッドや使い方をあまり把握していませんが、 renderRow() を用いており、 CAPTCHA の場合に内部で renderError() が行なわれていませんでした。 CAPTCHA に限って出力されていなかった原因は調査できていません。

と note-10 で言っていますが、これは CAPTCHA フォームが embedForm で扱われていることが原因かもしれません（が、この辺りに詳しくないので断言できません）。 render() メソッドは以下のように分岐があります。

lib/vendor/symfony/lib/form/sfFormField.class.php
115-  function render($attributes = array())
116-  {
117-    if ($this->parent)
118-    {
119-      return $this->parent->getWidget()->renderField($this->name, $this->value, $attributes, $this->error);
120-    }
121-    else
122-    {
123-      return $this->widget->render($this->name, $this->value, $attributes, $this->error);
124-    }
125-  }

ところで、 note-10 で行なった修正では、

-<?php echo $field->renderRow($attributes, $field->renderLabelName().$labelSuffix) ?>
+<tr>
+<th><?php echo $field->renderLabel($field->renderLabelName().$labelSuffix); ?></th>
+<td><?php echo $field->renderError(), $field->render($attributes); ?></td>
+</tr>

のように renderRow() の代わりに「renderError(), render()」を書いていますが、 renderHelp() の内容が出力されていません。 renderHelp() が出力されない問題は別チケット #2847 で扱います。