From 66a474ef0980b400bbb39948b560cecb01f059aa Mon Sep 17 00:00:00 2001
From: sun <sun@unleashedmind.com>
Date: Wed, 3 Apr 2013 14:18:24 +0200
Subject: [PATCH 1/5] - #1938768 by sun: Fixed CAPTCHA form element is not
 always hidden when it ought to be not visible.

---
 mollom.module                   |  26 +++--
 mollom.pages.inc                |   2 +-
 tests/mollom.test               | 233 ++++++++++++++++++++++++++++++++++++++--
 tests/mollom_test_server.module |  28 ++---
 4 files changed, 253 insertions(+), 36 deletions(-)

diff --git a/mollom.module b/mollom.module
index 30e91b5..a6d74a0 100644
--- a/mollom.module
+++ b/mollom.module
@@ -1772,19 +1772,20 @@ function mollom_process_mollom($element, $input, &$form_state, $complete_form) {
   if (!variable_get('mollom_testing_mode', 0)) {
     $element['captcha']['#attributes']['autocomplete'] = 'off';
   }
-  // For CAPTCHA-only protected forms, retrieve and show an initial CAPTCHA.
+  // For CAPTCHA-only protected forms:
   if (!$form_state['mollom']['require_analysis'] && $form_state['mollom']['require_captcha']) {
-    // Unless the CAPTCHA was solved, display the CAPTCHA.
-    if (!$form_state['mollom']['passed_captcha']) {
+    // Retrieve and show an initial CAPTCHA.
+    if (empty($form_state['process_input'])) {
       // Enable Form API caching, in order to track the state of the CAPTCHA.
       $form_state['cache'] = TRUE;
       // mollom_form_add_captcha() adds the CAPTCHA and disables page caching.
       mollom_form_add_captcha($element, $form_state);
     }
-    // Otherwise, resemble mollom_validate_captcha(). This case is only reached
-    // in case the form 1) is not cached, 2) fully validated, 3) was submitted,
-    // and 4) is getting rebuilt; e.g., "Preview" on comment and node forms.
-    else {
+    // If the CAPTCHA was solved in a previous submission already, resemble
+    // mollom_validate_captcha(). This case is only reached in case the form
+    // 1) is not cached, 2) fully validated, 3) was submitted, and 4) is getting
+    // rebuilt; e.g., "Preview" on comment and node forms.
+    if ($form_state['mollom']['passed_captcha']) {
       $element['captcha']['#solved'] = TRUE;
     }
   }
@@ -1984,6 +1985,7 @@ function mollom_validate_analysis(&$form, &$form_state) {
     switch ($result['spamClassification']) {
       case 'ham':
         $form_state['mollom']['require_captcha'] = FALSE;
+        $form['mollom']['captcha']['#access'] = FALSE;
         mollom_log(array(
           'message' => 'Ham: %teaser',
           'arguments' => array('%teaser' => $teaser),
@@ -1992,6 +1994,7 @@ function mollom_validate_analysis(&$form, &$form_state) {
 
       case 'spam':
         $form_state['mollom']['require_captcha'] = FALSE;
+        $form['mollom']['captcha']['#access'] = FALSE;
         if ($form_state['mollom']['discard']) {
           form_set_error('mollom', t('Your submission has triggered the spam filter and will not be accepted.') . ' ' . _mollom_format_message_falsepositive($form_state, $data));
         }
@@ -2010,6 +2013,7 @@ function mollom_validate_analysis(&$form, &$form_state) {
         }
         else {
           $form_state['mollom']['require_captcha'] = TRUE;
+          $form['mollom']['captcha']['#access'] = TRUE;
         }
         mollom_log(array(
           'message' => 'Unsure: %teaser',
@@ -2026,7 +2030,7 @@ function mollom_validate_analysis(&$form, &$form_state) {
         mollom_log(array(
           'message' => 'Unknown: %teaser',
           'arguments' => array('%teaser' => $teaser),
-        ));
+        ), WATCHDOG_ERROR);
         break;
     }
   }
@@ -2693,8 +2697,8 @@ function mollom_get_captcha(&$form_state) {
       'type' => $form_state['mollom']['captcha_type'],
       'ssl' => (int) (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on'),
     );
-    if (!empty($form_state['mollom']['response']['content']['id'])) {
-      $data['contentId'] = $form_state['mollom']['response']['content']['id'];
+    if (!empty($form_state['values']['mollom']['contentId'])) {
+      $data['contentId'] = $form_state['values']['mollom']['contentId'];
     }
     $result = mollom()->createCaptcha($data);
 
@@ -2707,7 +2711,7 @@ function mollom_get_captcha(&$form_state) {
     if (is_array($result) && isset($result['url'])) {
       $url = $result['url'];
       $form_state['mollom']['response'][$key] = $url;
-      $form_state['mollom']['response']['captcha']['id'] = $result['id'];
+      $form_state['mollom']['response']['captcha'] = $result;
     }
     else {
       return '';
diff --git a/mollom.pages.inc b/mollom.pages.inc
index f733f1f..e0ec300 100644
--- a/mollom.pages.inc
+++ b/mollom.pages.inc
@@ -41,7 +41,7 @@ function mollom_captcha_js($type, $mollom_build_id, $contentId = NULL) {
   else {
     $form_state['mollom'] = array();
     if (isset($contentId)) {
-      $form_state['mollom']['response']['content']['id'] = $contentId;
+      $form_state['values']['mollom']['contentId'] = $contentId;
     }
   }
   $form_state['mollom']['captcha_type'] = $type;
diff --git a/tests/mollom.test b/tests/mollom.test
index 24364b2..0c355bd 100644
--- a/tests/mollom.test
+++ b/tests/mollom.test
@@ -574,8 +574,12 @@ class MollomWebTestCase extends DrupalWebTestCase {
    *   An optional message to test does appear after submission.
    */
   protected function postCorrectCaptcha($url, array $edit = array(), $button, $success_message = '') {
+    if (isset($url)) {
+      $this->drupalGet($url);
+    }
+    $this->assertCaptchaField();
     $edit['mollom[captcha]'] = 'correct';
-    $this->drupalPost($url, $edit, $button);
+    $this->drupalPost(NULL, $edit, $button);
     $this->assertNoCaptchaField();
     $this->assertNoText($this->incorrect_message);
     if ($success_message) {
@@ -596,12 +600,14 @@ class MollomWebTestCase extends DrupalWebTestCase {
    *   An optional message to test does not appear after submission.
    */
   protected function postIncorrectCaptcha($url, array $edit = array(), $button, $success_message = '') {
+    if (isset($url)) {
+      $this->drupalGet($url);
+    }
+    $this->assertCaptchaField();
     $edit['mollom[captcha]'] = 'incorrect';
     $before_url = $this->getUrl();
-    $this->drupalPost($url, $edit, $button);
-    if ($this->getUrl() == $before_url) {
-      $this->assertCaptchaField();
-    }
+    $this->drupalPost(NULL, $edit, $button);
+    $this->assertCaptchaField();
     $this->assertText($this->incorrect_message);
     if ($success_message) {
       $this->assertNoText($success_message);
@@ -625,7 +631,7 @@ class MollomWebTestCase extends DrupalWebTestCase {
   protected function assertSpamSubmit($url, array $spam_fields, array $edit = array(), $button, $success_message = '') {
     $edit += array_fill_keys($spam_fields, 'spam');
     $this->drupalPost($url, $edit, $button);
-    $this->assertNoCaptchaField($url);
+    $this->assertNoCaptchaField();
     $this->assertText($this->spam_message);
     if ($success_message) {
       $this->assertNoText($success_message);
@@ -673,11 +679,8 @@ class MollomWebTestCase extends DrupalWebTestCase {
    */
   protected function assertUnsureSubmit($url, array $unsure_fields, array $edit = array(), $button, $success_message = '') {
     $edit += array_fill_keys($unsure_fields, 'unsure');
-    $before_url = $this->getUrl();
     $this->drupalPost($url, $edit, $button);
-    if ($this->getUrl() == $before_url) {
-      $this->assertCaptchaField();
-    }
+    $this->assertCaptchaField();
     $this->assertText($this->unsure_message);
     if ($success_message) {
       $this->assertNoText($success_message);
@@ -3710,6 +3713,10 @@ class MollomDataCRUDTestCase extends MollomWebTestCase {
 class MollomAnalysisTestCase extends MollomWebTestCase {
   protected $disableDefaultSetup = TRUE;
 
+  // Re-route Mollom communication to this testing site.
+  // @todo Remove this when Testing API is fixed.
+  protected $mollomClass = 'MollomDrupalTestLocal';
+
   public static function getInfo() {
     return array(
       'name' => 'Text analysis',
@@ -3727,7 +3734,211 @@ class MollomAnalysisTestCase extends MollomWebTestCase {
       'access administration pages',
       'administer mollom',
     ));
-    $this->web_user = $this->drupalCreateUser(array('access content'));
+  }
+
+  function testUnsure() {
+    $methods = get_class_methods($this);
+    foreach ($methods as $method) {
+      if (substr($method, 0, 7) === 'subtest') {
+        //debug($method);
+        $this->$method();
+      }
+    }
+  }
+
+  /**
+   * Tests basic unsure submission flow.
+   */
+  function subtestUnsureCorrect() {
+    $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
+
+    $this->drupalGet('mollom-test/form');
+    $this->assertNoCaptchaField();
+    $edit = array(
+      'title' => 'unsure',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId', TRUE);
+    $this->assertResponseIDInForm('captchaId', TRUE);
+    $this->assertCaptchaField();
+
+    $edit = array(
+      'title' => 'unsure unsure',
+      'mollom[captcha]' => 'correct',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertNoCaptchaField();
+    $this->assertTestSubmitData();
+  }
+
+  /**
+   * Tests unsure post with repetitive incorrect CAPTCHA solution.
+   *
+   * @todo CAPTCHA ID should stay the same.
+   * @see http://drupal.org/node/1959904
+   */
+  function subtestUnsureIncorrect() {
+    $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
+
+    $this->drupalGet('mollom-test/form');
+    $this->assertNoCaptchaField();
+    $edit = array(
+      'title' => 'unsure',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId', TRUE);
+    $this->assertResponseIDInForm('captchaId', TRUE);
+    $this->assertCaptchaField();
+
+    $edit = array();
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId');
+//    $this->assertResponseIDInForm('captchaId');
+    $this->assertCaptchaField();
+    $edit = array(
+      'mollom[captcha]' => 'incorrect',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId');
+//    $this->assertResponseIDInForm('captchaId');
+    $this->assertCaptchaField();
+
+    $edit = array(
+      'mollom[captcha]' => 'correct',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertNoCaptchaField();
+    $this->assertTestSubmitData();
+  }
+
+  /**
+   * Tests unsure post with other validation errors.
+   */
+  function subtestUnsureValidation() {
+    $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
+
+    // The 'title' field is required. Omit its value to verify that the CAPTCHA
+    // can be solved, repetitive form validations do not show a CAPTCHA again,
+    // and the post can finally be submitted by providing a title.
+    $this->drupalGet('mollom-test/form');
+    $this->assertNoCaptchaField();
+    $edit = array(
+      'body' => 'unsure',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId', TRUE);
+    $this->assertResponseIDInForm('captchaId', TRUE);
+    $this->assertCaptchaField();
+    $edit = array(
+      'body' => 'unsure unsure',
+      'mollom[captcha]' => 'correct',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId');
+    $this->assertResponseIDInForm('captchaId');
+    $this->assertNoCaptchaField();
+    $edit = array(
+      'body' => 'unsure unsure unsure',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId');
+    $this->assertResponseIDInForm('captchaId');
+    $this->assertNoCaptchaField();
+    $edit = array(
+      'title' => 'unsure',
+      'body' => 'unsure',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertNoCaptchaField();
+    $this->assertTestSubmitData();
+  }
+
+  /**
+   * Tests unsure posts turning into ham.
+   *
+   * @todo CAPTCHA ID should stay the same.
+   * @see http://drupal.org/node/1959904
+   */
+  function subtestUnsureHam() {
+    $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
+
+    $this->drupalGet('mollom-test/form');
+    $this->assertNoCaptchaField();
+    // Posting ham as title would submit the form, so post as body instead.
+    $edit = array(
+      'body' => 'unsure',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId', TRUE);
+    $this->assertResponseIDInForm('captchaId', TRUE);
+    $this->assertCaptchaField();
+    // Turn the post into ham.
+    $edit = array(
+      'body' => 'ham',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId');
+//    $this->assertResponseIDInForm('captchaId');
+    $this->assertNoCaptchaField();
+    // Turn the post back into unsure.
+    $edit = array(
+      'body' => 'unsure',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId');
+//    $this->assertResponseIDInForm('captchaId');
+    $this->assertCaptchaField();
+
+    $edit = array(
+      'title' => 'irrelevant',
+      'mollom[captcha]' => 'correct',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertNoCaptchaField();
+    $this->assertTestSubmitData();
+  }
+
+  /**
+   * Tests unsure posts turning into spam.
+   *
+   * @todo CAPTCHA ID should stay the same.
+   * @see http://drupal.org/node/1959904
+   */
+  function subtestUnsureSpam() {
+    $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
+
+    $this->drupalGet('mollom-test/form');
+    $this->assertNoCaptchaField();
+    $edit = array(
+      'title' => 'unsure',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId', TRUE);
+    $this->assertResponseIDInForm('captchaId', TRUE);
+    $this->assertCaptchaField();
+    // Turn the post into spam.
+    $edit = array(
+      'title' => 'spam',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId');
+//    $this->assertResponseIDInForm('captchaId');
+    $this->assertNoCaptchaField();
+    // Turn the post back into unsure.
+    $edit = array(
+      'title' => 'unsure',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertResponseIDInForm('contentId');
+//    $this->assertResponseIDInForm('captchaId');
+    $this->assertCaptchaField();
+
+    $edit = array(
+      'mollom[captcha]' => 'correct',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertNoCaptchaField();
+    $this->assertTestSubmitData();
   }
 
   /**
diff --git a/tests/mollom_test_server.module b/tests/mollom_test_server.module
index de38363..5af917f 100644
--- a/tests/mollom_test_server.module
+++ b/tests/mollom_test_server.module
@@ -500,14 +500,15 @@ function mollom_test_server_check_content($data) {
     }
     // Lastly, take a forced 'unsure' into account.
     elseif (strpos($post, 'unsure') !== FALSE) {
-      // Differentiate between binary unsure mode.
+      // Enabled unsure mode.
       if (!isset($data['unsure']) || $data['unsure']) {
         $spam = TRUE;
         $ham = TRUE;
       }
+      // Binary mode.
       else {
-        $spam = TRUE;
-        $ham = FALSE;
+        $spam = FALSE;
+        $ham = TRUE;
       }
     }
     // Check blacklist.
@@ -540,9 +541,9 @@ function mollom_test_server_check_content($data) {
     }
     // In case a previous spam check was unsure and a CAPTCHA was solved, the
     // result is supposed to be ham - unless the new content is spam.
-    if ($response['spamClassification'] != 'spam') {
-      $captcha_sessions = variable_get('mollom_test_server_check_captcha_sessions', array());
-      if (!empty($data['captchaId']) && !empty($captcha_sessions[$data['captchaId']])) {
+    if (!empty($data['id']) && $response['spamClassification'] == 'unsure') {
+      $content_captchas = variable_get('mollom_test_server_content_captcha', array());
+      if (!empty($content_captchas[$data['id']])) {
         $response['spamScore'] = 0.0;
         $response['spamClassification'] = 'ham';
       }
@@ -642,7 +643,7 @@ function mollom_test_server_check_content_blacklist($string, $blacklist, $reason
 }
 
 /**
- * API callback for mollom.getImageCaptcha to fetch a CATPCHA image.
+ * API callback for mollom.getImageCaptcha to fetch a CAPTCHA image.
  */
 function mollom_test_server_get_captcha($data) {
   $response = array();
@@ -665,8 +666,6 @@ function mollom_test_server_get_captcha($data) {
 
 /**
  * API callback for mollom.checkCaptcha to validate a CAPTCHA response.
- *
- * @todo Add support for 'redirect' and 'refresh' values.
  */
 function mollom_test_server_check_captcha($data) {
   $response = array();
@@ -684,13 +683,16 @@ function mollom_test_server_check_captcha($data) {
   if (!isset($storage[$captchaId])) {
     return MENU_NOT_FOUND;
   }
-  $storage[$captchaId] = $data;
+  $storage[$captchaId] = array_merge($storage[$captchaId], $data);
   $response['id'] = $captchaId;
   variable_set('mollom_test_server_captcha', $storage);
 
-  $captcha_sessions = variable_get('mollom_test_server_check_captcha_sessions', array());
-  $captcha_sessions[$captchaId] = $response['solved'];
-  variable_set('mollom_test_server_check_captcha_sessions', $captcha_sessions);
+  if (isset($storage[$captchaId]['contentId'])) {
+    $contentId = $storage[$captchaId]['contentId'];
+    $content_captchas = variable_get('mollom_test_server_content_captcha', array());
+    $content_captchas[$contentId] = $response['solved'];
+    variable_set('mollom_test_server_content_captcha', $content_captchas);
+  }
 
   return $response;
 }
-- 
1.7.11.msysgit.1


From 59b23efb996ea1d72f9c497ed9f0bd92dfc8c972 Mon Sep 17 00:00:00 2001
From: sun <sun@unleashedmind.com>
Date: Thu, 4 Apr 2013 20:11:43 +0200
Subject: [PATCH 2/5] - #1938768 by sun: Fixed text analysis validation does
 not always (re-)trigger CAPTCHA when required.

---
 mollom.module                   |  66 ++++++---------
 tests/mollom.test               | 178 ++++++++++++++++++++++++++++++++++++----
 tests/mollom_test_server.module |   9 +-
 3 files changed, 193 insertions(+), 60 deletions(-)

diff --git a/mollom.module b/mollom.module
index a6d74a0..080bc40 100644
--- a/mollom.module
+++ b/mollom.module
@@ -1725,11 +1725,6 @@ function mollom_process_mollom($element, $input, &$form_state, $complete_form) {
   );
   $element['captchaId'] = array(
     '#type' => 'hidden',
-    // For CAPTCHA-only protected forms, the most recent CAPTCHA ID is tracked
-    // in $form_state. A new CAPTCHA ID may be injected by mollom_get_captcha().
-    // Furthermore, the CAPTCHA type can be switched between image and audio,
-    // and for each switch, there is a new CAPTCHA ID.
-    '#default_value' => isset($form_state['mollom']['response']['captcha']['id']) ? $form_state['mollom']['response']['captcha']['id'] : '',
     '#attributes' => array('class' => 'mollom-captcha-id'),
   );
   $element['form_id'] = array(
@@ -2013,6 +2008,7 @@ function mollom_validate_analysis(&$form, &$form_state) {
         }
         else {
           $form_state['mollom']['require_captcha'] = TRUE;
+          $form_state['mollom']['passed_captcha'] = FALSE;
           $form['mollom']['captcha']['#access'] = TRUE;
         }
         mollom_log(array(
@@ -2199,6 +2195,10 @@ function mollom_pre_render_mollom($element) {
   // UX: Empty the CAPTCHA field value, as the user has to re-enter a new one.
   $element['captcha']['#value'] = '';
 
+  // DX: Debugging helpers.
+//  $element['captcha']['#suffix'] = 'contentId: ' . $element['contentId']['#value'];
+//  $element['captcha']['#suffix'] .= '<br />captchaId: ' . $element['captchaId']['#value'];
+
   return $element;
 }
 
@@ -2258,19 +2258,8 @@ function mollom_form_submit($form, &$form_state) {
     // We only consider non-empty and non-zero values as valid entity ids.
     if (!empty($values['postId'])) {
       // Save the Mollom session data.
-      $response = array();
-      if (isset($form_state['mollom']['response']['content'])) {
-        $response += $form_state['mollom']['response']['content'];
-        $response['contentId'] = $form_state['mollom']['response']['content']['id'];
-      }
-      if (isset($form_state['mollom']['response']['captcha'])) {
-        $response += $form_state['mollom']['response']['captcha'];
-        $response['captchaId'] = $form_state['mollom']['response']['captcha']['id'];
-      }
-      $data = (object) $response;
-      $data->entity = $form_state['mollom']['entity'];
+      $data = (object) $form_state['values']['mollom'];
       $data->id = $values['postId'];
-      $data->form_id = $form_state['mollom']['form_id'];
       // Set the moderation flag for forms accepting bad posts.
       $data->moderate = $form_state['mollom']['require_moderation'];
       $form_state['mollom']['data'] = mollom_data_save($data);
@@ -2691,34 +2680,29 @@ function mollom_content_extra_fields($type_name) {
  *   The markup of the CAPTCHA HTML.
  */
 function mollom_get_captcha(&$form_state) {
-  $key = 'captcha_url_' . $form_state['mollom']['captcha_type'];
-  if (empty($form_state['mollom']['response'][$key])) {
-    $data = array(
-      'type' => $form_state['mollom']['captcha_type'],
-      'ssl' => (int) (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on'),
-    );
-    if (!empty($form_state['values']['mollom']['contentId'])) {
-      $data['contentId'] = $form_state['values']['mollom']['contentId'];
-    }
-    $result = mollom()->createCaptcha($data);
+  // @todo Re-use existing CAPTCHA URL when the Mollom server response for
+  //   verifying a CAPTCHA solution returns the existing URL.
+  $data = array(
+    'type' => $form_state['mollom']['captcha_type'],
+    'ssl' => (int) (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on'),
+  );
+  if (!empty($form_state['values']['mollom']['contentId'])) {
+    $data['contentId'] = $form_state['values']['mollom']['contentId'];
+  }
+  $result = mollom()->createCaptcha($data);
 
-    // Add a log message to prevent the request log from appearing without a
-    // message on CAPTCHA-only protected forms.
-    mollom_log(array(
-      'message' => 'Retrieved new CAPTCHA',
-    ), WATCHDOG_DEBUG);
+  // Add a log message to prevent the request log from appearing without a
+  // message on CAPTCHA-only protected forms.
+  mollom_log(array(
+    'message' => 'Retrieved new CAPTCHA',
+  ), WATCHDOG_INFO);
 
-    if (is_array($result) && isset($result['url'])) {
-      $url = $result['url'];
-      $form_state['mollom']['response'][$key] = $url;
-      $form_state['mollom']['response']['captcha'] = $result;
-    }
-    else {
-      return '';
-    }
+  if (is_array($result) && isset($result['url'])) {
+    $url = $result['url'];
+    $form_state['mollom']['response']['captcha'] = $result;
   }
   else {
-    $url = $form_state['mollom']['response'][$key];
+    return '';
   }
 
   // @todo Convert these to actual theme functions?
diff --git a/tests/mollom.test b/tests/mollom.test
index 0c355bd..d03d327 100644
--- a/tests/mollom.test
+++ b/tests/mollom.test
@@ -329,12 +329,16 @@ class MollomWebTestCase extends DrupalWebTestCase {
    *   (optional) Whether to enforce creation of new API keys. New keys are only
    *   created if MollomWebTestCase::$createKeys or respectively the
    *   'mollom_testing_create_keys' variable is set to TRUE. Defaults to FALSE.
+   * @param bool $once
+   *   (optional) Whether to disable the 'mollom_testing_create_keys' variable
+   *   after the first call (and thus omit API key verifications on every page
+   *   request). Defaults to FALSE; i.e., API keys are verified repetitively.
    *
    * @see MollomWebTestCase::$createKeys
    * @see MollomDrupalTest::__construct()
    * @see MollomDrupalTest::createKeys()
    */
-  protected function setKeys($force = FALSE) {
+  protected function setKeys($force = FALSE, $once = FALSE) {
     // Instantiate a Mollom client class.
     // Depending on MollomWebTestCase::$createKeys and ultimately the
     // 'mollom_testing_create_keys' variable, MollomDrupalTest::__construct()
@@ -355,6 +359,9 @@ class MollomWebTestCase extends DrupalWebTestCase {
       // keys still exists in the test, but the configuration is gone.
       $this->mollom->saveKeys();
     }
+    if ($once) {
+      variable_set('mollom_testing_create_keys', FALSE);
+    }
   }
 
   /**
@@ -2911,14 +2918,14 @@ class MollomCommentFormTestCase extends MollomWebTestCase {
     // required field.
     $this->postIncorrectCaptcha(NULL, array(), t('Preview'));
     $this->assertText(t('Comment field is required.'));
-    $this->assertResponseIDInForm('captchaId');
+    $this->assertResponseIDInForm('captchaId', TRUE);
     $this->assertNoPrivacyLink();
 
     // Try to submit a correct answer for the CAPTCHA, still without required
     // field value.
     $this->postCorrectCaptcha(NULL, array(), t('Preview'));
     $this->assertText(t('Comment field is required.'));
-    $captchaId = $this->assertResponseIDInForm('captchaId');
+    $captchaId = $this->assertResponseIDInForm('captchaId', TRUE);
     $this->assertNoPrivacyLink();
 
     // Finally, we should be able to submit a comment.
@@ -3727,13 +3734,15 @@ class MollomAnalysisTestCase extends MollomWebTestCase {
 
   function setUp() {
     parent::setUp(array('mollom', 'mollom_test'));
-    $this->setKeys();
+    $this->setKeys(TRUE, TRUE);
     $this->assertValidKeys();
 
     $this->admin_user = $this->drupalCreateUser(array(
       'access administration pages',
       'administer mollom',
     ));
+
+    $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
   }
 
   function testUnsure() {
@@ -3750,8 +3759,6 @@ class MollomAnalysisTestCase extends MollomWebTestCase {
    * Tests basic unsure submission flow.
    */
   function subtestUnsureCorrect() {
-    $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
-
     $this->drupalGet('mollom-test/form');
     $this->assertNoCaptchaField();
     $edit = array(
@@ -3778,8 +3785,6 @@ class MollomAnalysisTestCase extends MollomWebTestCase {
    * @see http://drupal.org/node/1959904
    */
   function subtestUnsureIncorrect() {
-    $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
-
     $this->drupalGet('mollom-test/form');
     $this->assertNoCaptchaField();
     $edit = array(
@@ -3815,8 +3820,6 @@ class MollomAnalysisTestCase extends MollomWebTestCase {
    * Tests unsure post with other validation errors.
    */
   function subtestUnsureValidation() {
-    $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
-
     // The 'title' field is required. Omit its value to verify that the CAPTCHA
     // can be solved, repetitive form validations do not show a CAPTCHA again,
     // and the post can finally be submitted by providing a title.
@@ -3860,8 +3863,6 @@ class MollomAnalysisTestCase extends MollomWebTestCase {
    * @see http://drupal.org/node/1959904
    */
   function subtestUnsureHam() {
-    $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
-
     $this->drupalGet('mollom-test/form');
     $this->assertNoCaptchaField();
     // Posting ham as title would submit the form, so post as body instead.
@@ -3905,8 +3906,6 @@ class MollomAnalysisTestCase extends MollomWebTestCase {
    * @see http://drupal.org/node/1959904
    */
   function subtestUnsureSpam() {
-    $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
-
     $this->drupalGet('mollom-test/form');
     $this->assertNoCaptchaField();
     $edit = array(
@@ -3942,6 +3941,155 @@ class MollomAnalysisTestCase extends MollomWebTestCase {
   }
 
   /**
+   * Asserts a successful mollom_test_form submission.
+   *
+   * @param $old_mid
+   *   (optional) The existing test record id to assert.
+   */
+  protected function assertTestSubmitData($old_mid = NULL) {
+    $this->assertText('Successful form submission.');
+    $mid = $this->getFieldValueByName('mid');
+    if (isset($old_mid)) {
+      $this->assertSame('Test record id', $mid, $old_mid);
+    }
+    else {
+      $this->assertTrue($mid > 0, t('Test record id @id found.', array('@id' => $mid)));
+    }
+    return $mid;
+  }
+}
+
+/**
+ * Tests text analysis with page cache.
+ */
+class MollomAnalysisPageCacheTestCase extends MollomAnalysisTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'Text analysis (page cache)',
+      'description' => 'Tests text analysis with page cache.',
+      'group' => 'Mollom',
+    );
+  }
+
+  function setUp() {
+    parent::setUp();
+    $this->enablePageCache();
+
+    // Prime the page + form cache.
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+    $this->assertUnsureSubmit(NULL, array('title'), array(), 'Submit');
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+  }
+}
+
+/**
+ * Tests text analysis with page and form cache.
+ */
+class MollomAnalysisPageFormCacheTestCase extends MollomAnalysisTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'Text analysis (page + form cache)',
+      'description' => 'Tests text analysis with page and form cache.',
+      'group' => 'Mollom',
+    );
+  }
+
+  function setUp() {
+    parent::setUp();
+    $this->enablePageCache();
+    variable_set('mollom_test.form.cache', TRUE);
+
+    // Prime the page + form cache.
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+    $this->assertUnsureSubmit(NULL, array('title'), array(), 'Submit');
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+  }
+}
+
+/**
+ * Tests text analysis as authenticated user.
+ */
+class MollomAnalysisAuthenticatedTestCase extends MollomAnalysisTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'Text analysis (authenticated)',
+      'description' => 'Tests text analysis as authenticated user.',
+      'group' => 'Mollom',
+    );
+  }
+
+  function setUp() {
+    parent::setUp();
+    $this->web_user = $this->drupalCreateUser(array());
+    $this->drupalLogin($this->web_user);
+  }
+}
+
+/**
+ * Tests text analysis as authenticated user with enabled page and form cache.
+ */
+class MollomAnalysisAuthenticatedPageFormCacheTestCase extends MollomAnalysisTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'Text analysis (authenticated + page + form cache)',
+      'description' => 'Tests text analysis as authenticated user with enabled page and form cache.',
+      'group' => 'Mollom',
+    );
+  }
+
+  function setUp() {
+    parent::setUp();
+    $this->enablePageCache();
+    variable_set('mollom_test.form.cache', TRUE);
+
+    // Prime the page + form cache.
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+    $this->assertUnsureSubmit(NULL, array('title'), array(), 'Submit');
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+
+    $this->web_user = $this->drupalCreateUser(array());
+    $this->drupalLogin($this->web_user);
+  }
+}
+
+/**
+ * Tests text analysis functionality.
+ */
+class MollomAnalysisOptionsTestCase extends MollomWebTestCase {
+  protected $disableDefaultSetup = TRUE;
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Text analysis options',
+      'description' => 'Tests text analysis binary mode, retaining unsure/spam.',
+      'group' => 'Mollom',
+    );
+  }
+
+  function setUp() {
+    parent::setUp(array('mollom', 'mollom_test'));
+    $this->setKeys();
+    $this->assertValidKeys();
+
+    $this->admin_user = $this->drupalCreateUser(array(
+      'access administration pages',
+      'administer mollom',
+    ));
+  }
+
+  /**
    * Tests binary unsure mode.
    */
   function testUnsureBinary() {
@@ -4163,7 +4311,7 @@ class MollomAnalysisTestCase extends MollomWebTestCase {
 /**
  * Tests basic text analysis functionality with enabled caching.
  */
-class MollomAnalysisPageCacheTestCase extends MollomWebTestCase {
+class MollomAnalysisPageCachingTestCase extends MollomWebTestCase {
 
   protected $disableDefaultSetup = TRUE;
 
diff --git a/tests/mollom_test_server.module b/tests/mollom_test_server.module
index 5af917f..da13df5 100644
--- a/tests/mollom_test_server.module
+++ b/tests/mollom_test_server.module
@@ -646,19 +646,20 @@ function mollom_test_server_check_content_blacklist($string, $blacklist, $reason
  * API callback for mollom.getImageCaptcha to fetch a CAPTCHA image.
  */
 function mollom_test_server_get_captcha($data) {
-  $response = array();
+  $captchaId = (!empty($data['id']) ? $data['id'] : md5(mt_rand()));
+  $response = array(
+    'id' => $captchaId,
+  );
 
   // Return a HTTPS URL if 'ssl' parameter was passed.
   $base_url = $GLOBALS['base_url'];
   if (!empty($data['ssl'])) {
     $base_url = str_replace('http', 'https', $base_url);
   }
-  $response['url'] = $base_url . '/' . drupal_get_path('module', 'mollom') . '/images/powered-by-mollom-2.gif';
+  $response['url'] = $base_url . '/' . drupal_get_path('module', 'mollom') . '/images/powered-by-mollom-2.gif?captchaId=' . $captchaId;
 
   $storage = variable_get('mollom_test_server_captcha', array());
-  $captchaId = (!empty($data['id']) ? $data['id'] : md5(mt_rand()));
   $storage[$captchaId] = $data;
-  $response['id'] = $captchaId;
   variable_set('mollom_test_server_captcha', $storage);
 
   return $response;
-- 
1.7.11.msysgit.1


From 2e63ba784cbd5688afd9d24d093970f171407891 Mon Sep 17 00:00:00 2001
From: sun <sun@unleashedmind.com>
Date: Thu, 4 Apr 2013 22:48:04 +0200
Subject: [PATCH 3/5] - #1938768 by sun: Mimic D7's $form_state['complete
 form'] to allow validation handlers to alter the form
 structure.

---
 mollom.css        |   2 +-
 mollom.module     | 130 ++++++++++++++++--------------------------------------
 tests/mollom.test |   9 +---
 3 files changed, 42 insertions(+), 99 deletions(-)

diff --git a/mollom.css b/mollom.css
index cef4eaf..0b67255 100644
--- a/mollom.css
+++ b/mollom.css
@@ -5,6 +5,6 @@
 
 /* Help themes to properly display Mollom's log messages. */
 .dblog-event pre,
-#simpletest-result-form table td {
+#simpletest-result-form table td pre {
   white-space: pre-wrap;
 }
diff --git a/mollom.module b/mollom.module
index 080bc40..f366fdd 100644
--- a/mollom.module
+++ b/mollom.module
@@ -675,6 +675,9 @@ function mollom_form_alter(&$form, &$form_state, $form_id) {
         '#weight' => $weight,
         '#tree' => TRUE,
       );
+      // D6: Mimic D7's $form_state['complete form'] to allow validation
+      // handlers to access and alter the 'mollom' element.
+      $form_state['complete form'] = &$form;
 
       // Add Mollom form validation handlers.
       // Form-level validation handlers are required, since we need access to
@@ -1534,7 +1537,7 @@ function _mollom_format_message_falsepositive($form_state, $data) {
  *   new CAPTCHA, but without the previously entered value.
  * - In short, roughly:
  *   - Form construction: Nothing.
- *   - Form processing: Nothing.
+ *   - Form processing: Output a CAPTCHA for CAPTCHA-only protected forms.
  *   - Form validation: Perform validation and alterations based on validation.
  *
  * This, however, is not possible due to various bugs in Drupal core.
@@ -1551,18 +1554,9 @@ function _mollom_format_message_falsepositive($form_state, $data) {
  * - We need our own {cache_mollom} table as replacement for native form
  *   caching, as well as our own logic to validate a submitted 'session_id'
  *   ('form_build_id') against forms and users.
- * - We need to perform form alterations during form rendering, where
- *   $form_state is no longer available. To make this possible, we leverage the
- *   fact that an element property that is a reference to a key in $form_state
- *   (which in itself is passed by reference) persists on to the rendering
- *   layer. The essential part is:
- *   @code
- *     $element['#mollom'] = &$form_state['mollom'];
- *   @endcode
- * - Since we cannot alter elements in the form structure during form
- *   validation, this reference already needs to be set up during form
- *   processing (in a #process callback), while everything else lives in form
- *   validation handlers (unless it needs to add or alter the form structure).
+ * - We need to alter the $form structure during form validation. To make this
+ *   possible, we mimic D7 and assign $form to $form_state['complete form'] by
+ *   reference in mollom_form_alter().
  *
  * @see mollom_form_alter()
  */
@@ -1655,19 +1649,6 @@ function mollom_process_mollom($element, $input, &$form_state, $complete_form) {
   // Setup initial Mollom session and form information.
   $form_state += array('mollom' => array());
   $form_state['mollom'] += array(
-    // mollom_build_id works similar to form_build_id in D7; it is an ID that is
-    // unique for each build of a form. However, unlike form_build_id in D6, it
-    // does not change across POSTs, regardless of whether form storage/caching
-    // is enabled or not. Mollom uses the mollom_build_id as cache identifier to
-    // track the state of a form session/submission, which is required to
-    // re-inject prior knowledge about the submission (such as a "unsure" text
-    // analysis result). Although generated whenever this function is called,
-    // $form_state['mollom'] including mollom_build_id will already be
-    // prepopulated by mollom_process_mollom_form_state() and thus not overidden
-    // here. If mollom_build_id is manually removed from the form, the form's
-    // state is merely lost and the form submission needs to go through Mollom
-    // validation from scratch.
-    'mollom_build_id' => md5(uniqid(mt_rand(), TRUE)),
     // Only TRUE if the form is protected by text analysis.
     'require_analysis' => $element['#mollom_form']['mode'] == MOLLOM_MODE_ANALYSIS,
     // Becomes TRUE whenever a CAPTCHA needs to be solved.
@@ -1699,10 +1680,24 @@ function mollom_process_mollom($element, $input, &$form_state, $complete_form) {
   drupal_add_js(drupal_get_path('module', 'mollom') . '/mollom.js');
 
   // D6:
-  $element['mollom_build_id'] = array(
-    '#type' => 'hidden',
-    '#value' => $form_state['mollom']['mollom_build_id'],
-  );
+  // mollom_build_id works similar to form_build_id in D7; it is an ID that is
+  // unique for each build of a form. However, unlike form_build_id in D6, it
+  // does not change across POSTs, regardless of whether form storage/caching
+  // is enabled or not. Mollom uses the mollom_build_id as cache identifier to
+  // track the state of a form session/submission, which is required to
+  // re-inject prior knowledge about the submission (such as a "unsure" text
+  // analysis result). Although generated whenever this function is called,
+  // $form_state['mollom'] including mollom_build_id will already be
+  // prepopulated by mollom_process_mollom_form_state() and thus not overidden
+  // here. If mollom_build_id is manually removed from the form, the form's
+  // state is merely lost and the form submission needs to go through Mollom
+  // validation from scratch.
+  if (!$form_state['mollom']['require_analysis']) {
+    $element['mollom_build_id'] = array(
+      '#type' => 'hidden',
+      '#value' => !empty($form_state['mollom']['mollom_build_id']) ? $form_state['mollom']['mollom_build_id'] : md5(uniqid(mt_rand(), TRUE)),
+    );
+  }
 
   // Add the Mollom session data elements.
   // These elements resemble the {mollom} database schema. The form validation
@@ -1807,12 +1802,6 @@ function mollom_process_mollom($element, $input, &$form_state, $complete_form) {
     ),
   );
 
-  // Make Mollom form and session information available to #pre_render callback.
-  // This must be assigned by reference. It is the essential "communication
-  // layer" between form API and the rendering system. Any modifications to
-  // $form_state['mollom'] will be carried over to the element for rendering.
-  $element['#mollom'] = &$form_state['mollom'];
-
   // Make Mollom form and session information available to entirely different
   // functions.
   // @see mollom_mail_alter()
@@ -1945,7 +1934,7 @@ function mollom_validate_analysis(&$form, &$form_state) {
   // Ensure the latest content ID is output as value.
   // form_set_value() is effectless, as this is not a element-level but a
   // form-level validation handler.
-  $form['mollom']['contentId']['#value'] = $result['contentId'];
+  $form_state['complete form']['mollom']['contentId']['#value'] = $result['contentId'];
 
   // Prepare watchdog message teaser text.
   $teaser = '--';
@@ -1980,7 +1969,7 @@ function mollom_validate_analysis(&$form, &$form_state) {
     switch ($result['spamClassification']) {
       case 'ham':
         $form_state['mollom']['require_captcha'] = FALSE;
-        $form['mollom']['captcha']['#access'] = FALSE;
+        $form_state['complete form']['mollom']['captcha']['#access'] = FALSE;
         mollom_log(array(
           'message' => 'Ham: %teaser',
           'arguments' => array('%teaser' => $teaser),
@@ -1989,7 +1978,7 @@ function mollom_validate_analysis(&$form, &$form_state) {
 
       case 'spam':
         $form_state['mollom']['require_captcha'] = FALSE;
-        $form['mollom']['captcha']['#access'] = FALSE;
+        $form_state['complete form']['mollom']['captcha']['#access'] = FALSE;
         if ($form_state['mollom']['discard']) {
           form_set_error('mollom', t('Your submission has triggered the spam filter and will not be accepted.') . ' ' . _mollom_format_message_falsepositive($form_state, $data));
         }
@@ -2009,7 +1998,7 @@ function mollom_validate_analysis(&$form, &$form_state) {
         else {
           $form_state['mollom']['require_captcha'] = TRUE;
           $form_state['mollom']['passed_captcha'] = FALSE;
-          $form['mollom']['captcha']['#access'] = TRUE;
+          $form_state['complete form']['mollom']['captcha']['#access'] = TRUE;
         }
         mollom_log(array(
           'message' => 'Unsure: %teaser',
@@ -2046,8 +2035,7 @@ function mollom_validate_captcha(&$form, &$form_state) {
 
   // If there is no CAPTCHA ID yet, retrieve one and throw an error.
   if (empty($form_state['values']['mollom']['captchaId'])) {
-    // D6: Not possible here; deferred to mollom_pre_render_mollom().
-    //mollom_form_add_captcha($form['mollom'], $form_state);
+    mollom_form_add_captcha($form_state['complete form']['mollom'], $form_state);
     form_error($form['mollom']['captcha'], t('To complete this form, please complete the word verification below.'));
     return;
   }
@@ -2059,8 +2047,8 @@ function mollom_validate_captcha(&$form, &$form_state) {
   // primary 'require_captcha' condition will not be TRUE unless needed in the
   // first place.
   if ($form_state['mollom']['passed_captcha']) {
-    $form['mollom']['captcha']['#access'] = FALSE;
-    $form['mollom']['captcha']['#solved'] = TRUE;
+    $form_state['complete form']['mollom']['captcha']['#access'] = FALSE;
+    $form_state['complete form']['mollom']['captcha']['#solved'] = TRUE;
     return;
   }
 
@@ -2104,12 +2092,12 @@ function mollom_validate_captcha(&$form, &$form_state) {
   // Ensure the latest CAPTCHA ID is output as value.
   // form_set_value() is effectless, as this is not a element-level but a
   // form-level validation handler.
-  $form['mollom']['captchaId']['#value'] = $result['captchaId'];
+  $form_state['complete form']['mollom']['captchaId']['#value'] = $result['captchaId'];
 
   if (!empty($result['solved'])) {
     $form_state['mollom']['passed_captcha'] = TRUE;
-    $form['mollom']['captcha']['#access'] = FALSE;
-    $form['mollom']['captcha']['#solved'] = TRUE;
+    $form_state['complete form']['mollom']['captcha']['#access'] = FALSE;
+    $form_state['complete form']['mollom']['captcha']['#solved'] = TRUE;
 
     mollom_log(array(
       'message' => 'Correct CAPTCHA',
@@ -2118,8 +2106,7 @@ function mollom_validate_captcha(&$form, &$form_state) {
   else {
     $form_state['mollom']['passed_captcha'] = FALSE;
     form_set_error('mollom][captcha', t('The word verification was not completed correctly. Please complete this new word verification and try again.') . ' ' . _mollom_format_message_falsepositive($form_state, $data));
-    // D6: Not possible here; deferred to mollom_pre_render_mollom().
-    //mollom_form_add_captcha($form['mollom'], $form_state);
+    mollom_form_add_captcha($form_state['complete form']['mollom'], $form_state);
 
     mollom_log(array(
       'message' => 'Incorrect CAPTCHA',
@@ -2132,51 +2119,12 @@ function mollom_validate_captcha(&$form, &$form_state) {
  *
  * - Hides the CAPTCHA if it is not required or the solution was correct.
  * - Marks the CAPTCHA as required.
- *
- * This #pre_render trick is required, because form API validation does not
- * allow form validation handlers to alter the actual form structure. Both the
- * form constructor function and the #process callback for the 'mollom' element
- * are therefore executed too early (before form validation), so the CAPTCHA
- * element still contains not yet validated (default) values.
- * We also cannot invoke a form validation handler during form construction or
- * processing, because mollom_form_get_values() would be invoked too early
- * and therefore $form_state['values'] would not contain any additions from
- * form validation functions like mollom_comment_form_validate().
- * @see http://drupal.org/node/642702
  */
 function mollom_pre_render_mollom($element) {
-  $form_state['mollom'] = &$element['#mollom'];
-  // Request and inject a CAPTCHA after unsuccessful form validation.
-  if ($form_state['mollom']['require_captcha'] && !$form_state['mollom']['passed_captcha']) {
-    // Shortcut to responses, to make the following condition more readable.
-    $r = $form_state['mollom']['response'];
-    // However, do not request a new CAPTCHA a second time. In D7, the form
-    // validation handlers can adjust the form directly when needed. In D6, we
-    // need to check for the exact conditions in which the form validation
-    // handlers would have injected a new CAPTCHA normally. Thus, only if:
-    // 1) text analysis was unsure and there is no CAPTCHA yet.
-    $was_unsure = (empty($r['captcha']['id']) && isset($r['content']['spamClassification']) && $r['content']['spamClassification'] == 'unsure');
-    // 2) there was a CAPTCHA, but it wasn't solved correctly.
-    $was_incorrect = (isset($r['captcha']['solved']) && empty($r['captcha']['solved']));
-    if ($was_unsure || $was_incorrect) {
-      mollom_form_add_captcha($element, $form_state);
-    }
-  }
-  // If no CAPTCHA is required or the response was correct, hide the CAPTCHA.
-  elseif (!$form_state['mollom']['require_captcha'] || $form_state['mollom']['passed_captcha']) {
-    $element['captcha']['#access'] = FALSE;
-  }
-  // Ensure that new/current IDs are contained in the rendered form. Form
-  // validation handlers are not able to alter the $form structure in D6.
-  if (!empty($form_state['mollom']['response']['content']['id'])) {
-    $element['contentId']['#value'] = $form_state['mollom']['response']['content']['id'];
-  }
-  if (!empty($form_state['mollom']['response']['captcha']['id'])) {
-    $element['captchaId']['#value'] = $form_state['mollom']['response']['captcha']['id'];
-  }
-
   // Store the Mollom form state.
-  cache_set($form_state['mollom']['mollom_build_id'], $form_state['mollom'], 'cache_mollom', time() + 21600);
+  if (!empty($form_state['mollom']['mollom_build_id'])) {
+    cache_set($form_state['mollom']['mollom_build_id'], $form_state['mollom'], 'cache_mollom', time() + 21600);
+  }
 
   // If there is no CAPTCHA ID, then there is no CAPTCHA that can be displayed.
   // If a CAPTCHA was solved, then the widget makes no sense either.
diff --git a/tests/mollom.test b/tests/mollom.test
index d03d327..97293b7 100644
--- a/tests/mollom.test
+++ b/tests/mollom.test
@@ -3737,11 +3737,6 @@ class MollomAnalysisTestCase extends MollomWebTestCase {
     $this->setKeys(TRUE, TRUE);
     $this->assertValidKeys();
 
-    $this->admin_user = $this->drupalCreateUser(array(
-      'access administration pages',
-      'administer mollom',
-    ));
-
     $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS);
   }
 
@@ -4028,7 +4023,7 @@ class MollomAnalysisAuthenticatedTestCase extends MollomAnalysisTestCase {
 
   function setUp() {
     parent::setUp();
-    $this->web_user = $this->drupalCreateUser(array());
+    $this->web_user = $this->drupalCreateUser(array('access content'));
     $this->drupalLogin($this->web_user);
   }
 }
@@ -4059,7 +4054,7 @@ class MollomAnalysisAuthenticatedPageFormCacheTestCase extends MollomAnalysisTes
     $this->drupalGet('mollom-test/form');
     $this->assertText('Views: 1');
 
-    $this->web_user = $this->drupalCreateUser(array());
+    $this->web_user = $this->drupalCreateUser(array('access content'));
     $this->drupalLogin($this->web_user);
   }
 }
-- 
1.7.11.msysgit.1


From 2f2dac0052c2da3d3e0625330511e0d5cb288d96 Mon Sep 17 00:00:00 2001
From: sun <sun@unleashedmind.com>
Date: Fri, 5 Apr 2013 03:29:46 +0200
Subject: [PATCH 4/5] - #1938768 by sun: Hardened test coverage for
 CAPTCHA-only protected forms.

---
 tests/mollom.test | 238 ++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 170 insertions(+), 68 deletions(-)

diff --git a/tests/mollom.test b/tests/mollom.test
index 97293b7..0201fe7 100644
--- a/tests/mollom.test
+++ b/tests/mollom.test
@@ -547,6 +547,8 @@ class MollomWebTestCase extends DrupalWebTestCase {
    * Assert that the CAPTCHA field is not found on the current page.
    */
   protected function assertNoCaptchaField() {
+    $this->assertNoText($this->unsure_message);
+    $this->assertNoText($this->incorrect_message);
     $this->assertNoFieldByXPath('//input[@type="text"][@name="mollom[captcha]"]', '', 'CAPTCHA field not found.');
     $image = $this->xpath('//img[@alt="' . t('Type the characters you see in this picture.') . '"]');
     $this->assert(empty($image), 'CAPTCHA image not found.');
@@ -971,6 +973,24 @@ class MollomWebTestCase extends DrupalWebTestCase {
     //variable_set('cache_lifetime', 60);
   }
 
+  /**
+   * Asserts a successful mollom_test_form submission.
+   *
+   * @param $old_mid
+   *   (optional) The existing test record id to assert.
+   */
+  protected function assertTestSubmitData($old_mid = NULL) {
+    $this->assertText('Successful form submission.');
+    $mid = $this->getFieldValueByName('mid');
+    if (isset($old_mid)) {
+      $this->assertSame('Test record id', $mid, $old_mid);
+    }
+    else {
+      $this->assertTrue($mid > 0, t('Test record id @id found.', array('@id' => $mid)));
+    }
+    return $mid;
+  }
+
 }
 
 /**
@@ -3934,24 +3954,6 @@ class MollomAnalysisTestCase extends MollomWebTestCase {
     $this->assertNoCaptchaField();
     $this->assertTestSubmitData();
   }
-
-  /**
-   * Asserts a successful mollom_test_form submission.
-   *
-   * @param $old_mid
-   *   (optional) The existing test record id to assert.
-   */
-  protected function assertTestSubmitData($old_mid = NULL) {
-    $this->assertText('Successful form submission.');
-    $mid = $this->getFieldValueByName('mid');
-    if (isset($old_mid)) {
-      $this->assertSame('Test record id', $mid, $old_mid);
-    }
-    else {
-      $this->assertTrue($mid > 0, t('Test record id @id found.', array('@id' => $mid)));
-    }
-    return $mid;
-  }
 }
 
 /**
@@ -4284,23 +4286,6 @@ class MollomAnalysisOptionsTestCase extends MollomWebTestCase {
     }
   }
 
-  /**
-   * Asserts a successful mollom_test_form submission.
-   *
-   * @param $old_mid
-   *   (optional) The existing test record id to assert.
-   */
-  protected function assertTestSubmitData($old_mid = NULL) {
-    $this->assertText('Successful form submission.');
-    $mid = $this->getFieldValueByName('mid');
-    if (isset($old_mid)) {
-      $this->assertSame('Test record id', $mid, $old_mid);
-    }
-    else {
-      $this->assertTrue($mid > 0, t('Test record id @id found.', array('@id' => $mid)));
-    }
-    return $mid;
-  }
 }
 
 /**
@@ -4364,11 +4349,11 @@ class MollomAnalysisPageCachingTestCase extends MollomWebTestCase {
  * Tests CAPTCHA functionality.
  */
 class MollomCaptchaTestCase extends MollomWebTestCase {
+  protected $disableDefaultSetup = TRUE;
+
   // Re-route Mollom communication to this testing site.
   protected $mollomClass = 'MollomDrupalTestLocal';
 
-  protected $disableDefaultSetup = TRUE;
-
   public static function getInfo() {
     return array(
       'name' => 'CAPTCHA',
@@ -4379,24 +4364,26 @@ class MollomCaptchaTestCase extends MollomWebTestCase {
 
   function setUp() {
     parent::setUp(array('mollom', 'mollom_test'));
-    $this->setKeys();
+    $this->setKeys(TRUE, TRUE);
     $this->assertValidKeys();
 
-    $this->admin_user = $this->drupalCreateUser(array(
-      'access administration pages',
-      'administer mollom',
-    ));
-    $this->web_user = $this->drupalCreateUser(array('access content'));
+    $this->setProtection('mollom_test_form', MOLLOM_MODE_CAPTCHA);
+  }
 
-    $this->drupalLogin($this->admin_user);
-    $this->setProtectionUI('mollom_test_form', MOLLOM_MODE_CAPTCHA);
-    $this->drupalLogout();
+  function testCAPTCHA() {
+    $methods = get_class_methods($this);
+    foreach ($methods as $method) {
+      if (substr($method, 0, 7) === 'subtest') {
+        //debug($method);
+        $this->$method();
+      }
+    }
   }
 
   /**
    * Tests #required validation of CAPTCHA form element.
    */
-  function testRequiredValidation() {
+  function subtestCAPTCHARequired() {
     $this->drupalGet('mollom-test/form');
 
     // Verify that CAPTCHA cannot be left empty.
@@ -4417,18 +4404,28 @@ class MollomCaptchaTestCase extends MollomWebTestCase {
     );
     $this->postIncorrectCaptcha(NULL, $edit, 'Submit', 'Successful form submission.');
 
-    // Lastly, verify correct solution.
+    // Verify correct solution, but trigger other validation errors.
     $edit = array(
+      'title' => '',
       'mollom[captcha]' => 'correct',
     );
     $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertNoCaptchaField();
+    $this->assertNoText('Successful form submission.');
+
+    // Lastly, confirm we're able to submit.
+    $edit = array(
+      'title' => $this->randomString(),
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertNoText($this->incorrect_message);
     $this->assertTestSubmitData();
   }
 
   /**
    * Tests incorrect solution of CAPTCHA form element.
    */
-  function testIncorrect() {
+  function subtestCAPTCHAIncorrect() {
     $this->drupalGet('mollom-test/form');
 
     // Verify that incorrect solution still leaves the field required.
@@ -4446,42 +4443,51 @@ class MollomCaptchaTestCase extends MollomWebTestCase {
   }
 
   /**
-   * Tests correct solution of CAPTCHA form element.
+   * Tests correct solution of CAPTCHA.
    */
-  function testCorrect() {
+  function subtestCAPTCHACorrect() {
     $this->drupalGet('mollom-test/form');
 
-    // Verify that CAPTCHA can be solved in one shot.
     $edit = array(
-      'title' => $this->randomString(),
       'mollom[captcha]' => 'correct',
     );
     $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertNoCaptchaField();
+    $this->assertNoText('Successful form submission.');
+
+    $edit = array(
+      'body' => $this->randomString(),
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertNoCaptchaField();
+    $this->assertNoText('Successful form submission.');
+
+    $edit = array(
+      'title' => $this->randomString(),
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
     $this->assertTestSubmitData();
   }
 
   /**
-   * Asserts a successful mollom_test_form submission.
-   *
-   * @param $old_mid
-   *   (optional) The existing test record id to assert.
+   * Tests correct solution of CAPTCHA in a single pass.
    */
-  protected function assertTestSubmitData($old_mid = NULL) {
-    $this->assertText('Successful form submission.');
-    $mid = $this->getFieldValueByName('mid');
-    if (isset($old_mid)) {
-      $this->assertSame('Test record id', $mid, $old_mid);
-    }
-    else {
-      $this->assertTrue($mid > 0, t('Test record id @id found.', array('@id' => $mid)));
-    }
-    return $mid;
+  function subtestCAPTCHACorrectSinglePass() {
+    $this->drupalGet('mollom-test/form');
+
+    // Verify that CAPTCHA can be solved in one shot.
+    $edit = array(
+      'title' => $this->randomString(),
+      'mollom[captcha]' => 'correct',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertTestSubmitData();
   }
 
   /**
    * Tests the CAPTCHA type switch callback.
    */
-  function testSwitchCallback() {
+  function subtestCAPTCHASwitchCallback() {
     // Verify that the CAPTCHA can be switched on a CAPTCHA-only protected form.
     // (without having a contentId)
     $this->drupalGet('mollom-test/form');
@@ -4507,6 +4513,102 @@ class MollomCaptchaTestCase extends MollomWebTestCase {
 }
 
 /**
+ * Tests CAPTCHA with page cache.
+ */
+class MollomCaptchaPageCacheTestCase extends MollomCaptchaTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'CAPTCHA (page cache)',
+      'description' => 'Tests CAPTCHA with page cache.',
+      'group' => 'Mollom',
+    );
+  }
+
+  function setUp() {
+    parent::setUp();
+    $this->enablePageCache();
+
+    // Prime the page + form cache.
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 2');
+  }
+}
+
+/**
+ * Tests CAPTCHA with page and form cache.
+ */
+class MollomCaptchaPageFormCacheTestCase extends MollomCaptchaTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'CAPTCHA (page + form cache)',
+      'description' => 'Tests CAPTCHA with page and form cache.',
+      'group' => 'Mollom',
+    );
+  }
+
+  function setUp() {
+    parent::setUp();
+    $this->enablePageCache();
+    variable_set('mollom_test.form.cache', TRUE);
+
+    // Prime the page + form cache.
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 2');
+  }
+}
+
+/**
+ * Tests CAPTCHA as authenticated user.
+ */
+class MollomCaptchaAuthenticatedTestCase extends MollomCaptchaTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'CAPTCHA (authenticated)',
+      'description' => 'Tests CAPTCHA as authenticated user.',
+      'group' => 'Mollom',
+    );
+  }
+
+  function setUp() {
+    parent::setUp();
+    $this->web_user = $this->drupalCreateUser(array('access content'));
+    $this->drupalLogin($this->web_user);
+  }
+}
+
+/**
+ * Tests CAPTCHA as authenticated user with enabled page and form cache.
+ */
+class MollomCaptchaAuthenticatedPageFormCacheTestCase extends MollomCaptchaTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'CAPTCHA (authenticated + page + form cache)',
+      'description' => 'Tests CAPTCHA as authenticated user with enabled page and form cache.',
+      'group' => 'Mollom',
+    );
+  }
+
+  function setUp() {
+    parent::setUp();
+    $this->enablePageCache();
+    variable_set('mollom_test.form.cache', TRUE);
+
+    // Prime the page + form cache.
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 1');
+    $this->drupalGet('mollom-test/form');
+    $this->assertText('Views: 2');
+
+    $this->web_user = $this->drupalCreateUser(array('access content'));
+    $this->drupalLogin($this->web_user);
+  }
+}
+
+/**
  * Tests report to Mollom functionality.
  */
 class MollomReportTestCase extends MollomWebTestCase {
-- 
1.7.11.msysgit.1


From d011ff33fb7e9e0937020428bb15ce4a7e6c9c54 Mon Sep 17 00:00:00 2001
From: sun <sun@unleashedmind.com>
Date: Fri, 5 Apr 2013 04:04:52 +0200
Subject: [PATCH 5/5] - #1938768 by sun: Limited mollom_build_id usage to
 CAPTCHA-only protected forms.

---
 mollom.module            | 94 +++++++++++++++++++++++++-----------------------
 tests/mollom_test.module |  2 +-
 2 files changed, 51 insertions(+), 45 deletions(-)

diff --git a/mollom.module b/mollom.module
index f366fdd..14fd85f 100644
--- a/mollom.module
+++ b/mollom.module
@@ -1645,7 +1645,7 @@ function theme_mollom($element) {
  * The session ID is valid for a given $form_id only. We expire it as soon as
  * the form is submitted, to avoid it being replayed.
  */
-function mollom_process_mollom($element, $input, &$form_state, $complete_form) {
+function mollom_process_mollom($element, $input, &$form_state, &$complete_form) {
   // Setup initial Mollom session and form information.
   $form_state += array('mollom' => array());
   $form_state['mollom'] += array(
@@ -1679,26 +1679,6 @@ function mollom_process_mollom($element, $input, &$form_state, $complete_form) {
   // Add the JavaScript.
   drupal_add_js(drupal_get_path('module', 'mollom') . '/mollom.js');
 
-  // D6:
-  // mollom_build_id works similar to form_build_id in D7; it is an ID that is
-  // unique for each build of a form. However, unlike form_build_id in D6, it
-  // does not change across POSTs, regardless of whether form storage/caching
-  // is enabled or not. Mollom uses the mollom_build_id as cache identifier to
-  // track the state of a form session/submission, which is required to
-  // re-inject prior knowledge about the submission (such as a "unsure" text
-  // analysis result). Although generated whenever this function is called,
-  // $form_state['mollom'] including mollom_build_id will already be
-  // prepopulated by mollom_process_mollom_form_state() and thus not overidden
-  // here. If mollom_build_id is manually removed from the form, the form's
-  // state is merely lost and the form submission needs to go through Mollom
-  // validation from scratch.
-  if (!$form_state['mollom']['require_analysis']) {
-    $element['mollom_build_id'] = array(
-      '#type' => 'hidden',
-      '#value' => !empty($form_state['mollom']['mollom_build_id']) ? $form_state['mollom']['mollom_build_id'] : md5(uniqid(mt_rand(), TRUE)),
-    );
-  }
-
   // Add the Mollom session data elements.
   // These elements resemble the {mollom} database schema. The form validation
   // handlers will pollute them with values returned by Mollom. For entity
@@ -1764,10 +1744,23 @@ function mollom_process_mollom($element, $input, &$form_state, $complete_form) {
   }
   // For CAPTCHA-only protected forms:
   if (!$form_state['mollom']['require_analysis'] && $form_state['mollom']['require_captcha']) {
+    // D6: Mimic D7's unique, reliable form build ID.
+    // Tracks the state of a CAPTCHA. Only applies to CAPTCHA-only protected
+    // forms, for which page caching is force-disabled.
+    // @see mollom_process_mollom_form_state()
+    // @see mollom_validate_post()
+    if (!isset($form_state['mollom']['mollom_build_id'])) {
+      $form_state['mollom']['mollom_build_id'] = md5(uniqid(mt_rand(), TRUE));
+    }
+    $element['mollom_build_id'] = array(
+      '#type' => 'hidden',
+      '#value' => $form_state['mollom']['mollom_build_id'],
+    );
+
     // Retrieve and show an initial CAPTCHA.
     if (empty($form_state['process_input'])) {
       // Enable Form API caching, in order to track the state of the CAPTCHA.
-      $form_state['cache'] = TRUE;
+      $element['#cache'] = TRUE;
       // mollom_form_add_captcha() adds the CAPTCHA and disables page caching.
       mollom_form_add_captcha($element, $form_state);
     }
@@ -1802,6 +1795,18 @@ function mollom_process_mollom($element, $input, &$form_state, $complete_form) {
     ),
   );
 
+  // D6: The complete form available to the #pre_render callback.
+  // Only applies to forms with enabled form caching (#cache = TRUE), for which
+  // mollom_form_alter() is not executed. D6 Form API does not pass $form or
+  // $complete_form by reference internally, nor to any kind of callbacks.
+  // The following assignments build a cross-reference for $complete_form
+  // between $form_state and the element property.
+  // @see mollom_pre_render_mollom()
+  if (!isset($form_state['complete form'])) {
+    $form_state['complete form'] = &$complete_form;
+    $element['#complete form'] = &$complete_form;
+  }
+
   // Make Mollom form and session information available to entirely different
   // functions.
   // @see mollom_mail_alter()
@@ -1821,26 +1826,18 @@ function mollom_process_mollom($element, $input, &$form_state, $complete_form) {
  * @see http://drupal.org/node/642702
  */
 function mollom_process_mollom_form_state($element, $input, &$form_state) {
-  // The current state can come either from the $form_state, if the form
-  // was just rebuilt in the same request, or from data posted by the user. In
-  // the latter case the state is fetched from the temporary data store. It is
-  // verified that the session was created for the current form and that it has
-  // not expired or already been used.
   if (empty($form_state['mollom']) && !empty($input['mollom_build_id'])) {
-    if (!$cache = cache_get($input['mollom_build_id'], 'cache_mollom')) {
-      watchdog('mollom', 'Unknown form build ID %form_build_id.', array(
-        '%form_build_id' => $input['mollom_build_id'],
-      ), WATCHDOG_WARNING);
-    }
-    elseif ($cache->data['form_id'] !== $form_state['values']['form_id']) {
-      watchdog('mollom', 'Invalid form ID %form_id for build ID %form_build_id (generated for %cached_form_id).', array(
-        '%form_build_id' => $input['mollom_build_id'],
-        '%cached_form_id' => $cache->data['form_id'],
-        '%form_id' => $form_state['values']['form_id'],
-      ), WATCHDOG_WARNING);
-    }
-    else {
-      $form_state['mollom'] = $cache->data;
+    if ($cache = cache_get($input['mollom_build_id'], 'cache_mollom')) {
+      if ($cache->data['form_id'] !== $form_state['values']['form_id']) {
+        watchdog('mollom', 'Invalid form ID %form_id for build ID %form_build_id (generated for %cached_form_id).', array(
+          '%form_build_id' => $input['mollom_build_id'],
+          '%cached_form_id' => $cache->data['form_id'],
+          '%form_id' => $form_state['values']['form_id'],
+        ), WATCHDOG_WARNING);
+      }
+      else {
+        $form_state['mollom'] = $cache->data;
+      }
     }
   }
   return $element;
@@ -2121,9 +2118,13 @@ function mollom_validate_captcha(&$form, &$form_state) {
  * - Marks the CAPTCHA as required.
  */
 function mollom_pre_render_mollom($element) {
-  // Store the Mollom form state.
-  if (!empty($form_state['mollom']['mollom_build_id'])) {
-    cache_set($form_state['mollom']['mollom_build_id'], $form_state['mollom'], 'cache_mollom', time() + 21600);
+  // D6: Take over any adjustments performed by form validation handlers.
+  // Only applies to forms with enabled form caching (#cache = TRUE).
+  // @see mollom_process_mollom()
+  if (isset($element['#complete form']['mollom'])) {
+    foreach (element_children($element['#complete form']['mollom']) as $key) {
+      $element[$key] = array_merge($element[$key], $element['#complete form']['mollom'][$key]);
+    }
   }
 
   // If there is no CAPTCHA ID, then there is no CAPTCHA that can be displayed.
@@ -2164,6 +2165,11 @@ function mollom_validate_post(&$form, &$form_state) {
     $function = $form_state['mollom']['moderation callback'];
     $function($form, $form_state);
   }
+
+  // Update the Mollom form state.
+  if (!empty($form_state['mollom']['mollom_build_id'])) {
+    cache_set($form_state['mollom']['mollom_build_id'], $form_state['mollom'], 'cache_mollom', time() + (6 * 3600));
+  }
 }
 
 /**
diff --git a/tests/mollom_test.module b/tests/mollom_test.module
index 6838317..3ce35e0 100644
--- a/tests/mollom_test.module
+++ b/tests/mollom_test.module
@@ -142,7 +142,7 @@ function mollom_test_form(&$form_state, $mid = NULL) {
     if (!isset($form_state['storage'])) {
       $form_state['storage'] = $values;
     }
-    $form_state['cache'] = TRUE;
+    $form['#cache'] = TRUE;
   }
 
   $form['#tree'] = TRUE;
-- 
1.7.11.msysgit.1