diff --git a/mollom.module b/mollom.module index 7062fb6..634b624 100644 --- a/mollom.module +++ b/mollom.module @@ -1643,11 +1643,6 @@ function mollom_process_mollom($element, &$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( @@ -1883,6 +1878,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( @@ -2022,6 +2018,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'] .= '
captchaId: ' . $element['captchaId']['#value']; + return $element; } @@ -2077,19 +2077,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); @@ -2517,34 +2506,29 @@ function mollom_field_extra_fields() { * 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_DEBUG); - 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 31a4c3e..a313452 100644 --- a/tests/mollom.test +++ b/tests/mollom.test @@ -339,12 +339,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() @@ -365,6 +369,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); + } } /** @@ -3840,13 +3847,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() { @@ -3863,8 +3872,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( @@ -3891,8 +3898,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( @@ -3928,8 +3933,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. @@ -3973,8 +3976,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. @@ -4018,8 +4019,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( @@ -4055,6 +4054,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() { @@ -4276,7 +4424,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 7fd6008..bbd041b 100644 --- a/tests/mollom_test_server.module +++ b/tests/mollom_test_server.module @@ -635,19 +635,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;