@messageResult:
@result', array('@message' => print_r($data, TRUE), '@result' => print_r($result, TRUE))); } + // Handle the spam check result. + // The Mollom backend is remembering results of previous mollom.checkContent + // invocations for a single user/post session. When content is re-checked + // during form validation, the result may change according to the values that + // have been submitted (which e.g. can change during previews). Only in case + // the spam check led to a MOLLOM_ANALYSIS_UNSURE result, and the user solved + // the CAPTCHA correctly, subsequent spam check results will likely be + // MOLLOM_ANALYSIS_HAM (though not guaranteed). if (isset($result['spam'])) { switch ($result['spam']) { case MOLLOM_ANALYSIS_HAM: @@ -1288,27 +1300,34 @@ function mollom_validate_analysis(&$form break; case MOLLOM_ANALYSIS_UNSURE: - $form_state['mollom']['require_captcha'] = TRUE; - form_set_error('mollom][captcha', t('To complete this form, please complete the word verification below.')); watchdog('mollom', 'Unsure:
@messageResult:
@result', array('@message' => print_r($data, TRUE), '@result' => print_r($result, TRUE))); - $form['mollom']['captcha']['#access'] = TRUE; - $form['mollom']['captcha']['#required'] = TRUE; + // Only throw a validation error and retrieve a CAPTCHA, if we check + // this post for the first time. Otherwise, mollom_validate_captcha() + // issued the CAPTCHA and needs to validate it prior to throwing any + // errors. + if (!$form_state['mollom']['require_captcha']) { + $form_state['mollom']['require_captcha'] = TRUE; + form_set_error('mollom][captcha', t('To complete this form, please complete the word verification below.')); - $captcha_data = array( - 'author_ip' => $data['author_ip'], - 'session_id' => $result['session_id'], - ); - $captcha = mollom_get_captcha('image', $captcha_data); - - // If we get a response, add the image CAPTCHA to the form element. - if (isset($captcha['response']['session_id']) && !empty($captcha['markup'])) { - $form_state['mollom']['response']['session_id'] = $captcha['response']['session_id']; - $form['mollom']['session_id']['#value'] = $captcha['response']['session_id']; - $form['mollom']['captcha']['#field_prefix'] = $captcha['markup']; + $form['mollom']['captcha']['#access'] = TRUE; + $form['mollom']['captcha']['#required'] = TRUE; + + $captcha_data = array( + 'session_id' => $result['session_id'], + ); + $captcha = mollom_get_captcha('image', $captcha_data); + + // If we get a response, add the image CAPTCHA to the form element. + if (isset($captcha['response']['session_id']) && !empty($captcha['markup'])) { + $form_state['mollom']['response']['session_id'] = $captcha['response']['session_id']; + $form['mollom']['session_id']['#value'] = $captcha['response']['session_id']; + $form['mollom']['captcha']['#field_prefix'] = $captcha['markup']; + } } break; + case MOLLOM_ANALYSIS_UNKNOWN: default: // If we end up here, something went totally wrong. _mollom_fallback(); @@ -1318,22 +1337,28 @@ function mollom_validate_analysis(&$form } /** - * Form validation handler for CAPTCHA form element. + * Form validation handler for Mollom's CAPTCHA form element. + * + * Validates whether a CAPTCHA was solved correctly. A form may contain a + * CAPTCHA, if it was configured to be protected by a CAPTCHA only, or when the + * text analysis result is "unsure". */ function mollom_validate_captcha(&$form, &$form_state) { - if (!$form_state['mollom']['require_captcha']) { - $form['mollom']['captcha']['#access'] = FALSE; - return; - } - - // When re-validating a form that already passed a CAPTCHA in a previous - // request, we need to re-populate our global variable for mollom_data_save(). - if ($form_state['mollom']['passed_captcha']) { + // CAPTCHA validation may only be skipped, if we do not require it in the + // first place, or if the user already solved a CAPTCHA correctly. We need to + // validate, if $form_state['mollom']['require_captcha'] is TRUE, which is + // either set during initialization of $form_state['mollom'] in + // mollom_process_form(), or after performing text analysis. The second + // return condition, $form_state['mollom']['passed_captcha'], may only ever be + // set by this validation handler and must not be changed elsewhere. + if (!$form_state['mollom']['require_captcha'] || $form_state['mollom']['passed_captcha']) { $form['mollom']['captcha']['#access'] = FALSE; return; } // Nothing to validate if there is no value. + // @todo The field is #required, so Form API should already handle this. Add a + // test to be sure and remove this code. if (empty($form_state['values']['mollom']['captcha'])) { return; } @@ -1351,7 +1376,8 @@ function mollom_validate_captcha(&$form, )); // Invoke fallback behavior upon a server error; communication errors are - // handled by mollom() already. + // handled by mollom() already. A server error may happen in case of an + // expired or invalid session_id. if ($result === MOLLOM_ERROR) { return _mollom_fallback(); } @@ -1360,8 +1386,6 @@ function mollom_validate_captcha(&$form, $form_state['mollom']['response']['captcha'] = $result; $form['mollom']['session_id']['#value'] = $form_state['mollom']['response']['session_id']; - // Explictly check for TRUE, since mollom.checkCaptcha() can also return an - // error message (e.g. expired or invalid session_id). if ($result === TRUE) { $form_state['mollom']['passed_captcha'] = TRUE; $form['mollom']['captcha']['#access'] = FALSE; @@ -1369,7 +1393,7 @@ function mollom_validate_captcha(&$form, watchdog('mollom', 'Correct CAPTCHA:
@data', array('@data' => print_r($form_state['values'], TRUE)));
}
else {
- // Empty the CAPTCHA field value, since the user has to re-enter a new one.
+ // UX: Empty the CAPTCHA field value, as the user has to re-enter a new one.
$form['mollom']['captcha']['#value'] = '';
form_set_error('mollom][captcha', t('The word verification was not completed correctly. Please complete this new word verification and try again.'));
@@ -1392,8 +1416,9 @@ function mollom_form_submit($form, &$for
if (!empty($form_state['mollom']['entity']) && isset($form_state['mollom']['mapping']['post_id'])) {
// For new entities, the entity's form submit handler will have added the
// new entity id value into $form_state['values'], so we need to rebuild the
- // data mapping.
- $data = mollom_form_get_values($form_state['values'], $form_state['mollom']['enabled_fields'], $form_state['mollom']['mapping']);
+ // data mapping. We do not care for the actual fields, only for the value of
+ // the mapped post_id.
+ $data = mollom_form_get_values($form_state['values'], array(), $form_state['mollom']['mapping']);
// We only consider non-empty and non-zero values as valid entity ids.
if (!empty($data['post_id'])) {
mollom_data_save($form_state['mollom']['entity'], $data['post_id']);
Index: tests/mollom.test
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/mollom/tests/mollom.test,v
retrieving revision 1.55
diff -u -p -r1.55 mollom.test
--- tests/mollom.test 11 Sep 2010 02:22:27 -0000 1.55
+++ tests/mollom.test 11 Sep 2010 20:58:37 -0000
@@ -558,6 +558,7 @@ class MollomWebTestCase extends DrupalWe
* (optional) The XML-RPC method name to retrieve submitted values from.
* Defaults to 'mollom.checkContent'.
*
+ * @see MollomWebTestCase::resetServerRecords()
* @see mollom_test_xmlrpc()
*/
protected function getServerRecord($method = 'mollom.checkContent') {
@@ -575,6 +576,26 @@ class MollomWebTestCase extends DrupalWe
}
/**
+ * Resets recorded XML-RPC values.
+ *
+ * @param $method
+ * (optional) The XML-RPC method name to reset records of. Defaults to
+ * 'mollom.checkContent'.
+ *
+ * @see MollomWebTestCase::getServerRecord()
+ * @see mollom_test_xmlrpc()
+ */
+ protected function resetServerRecords($method = 'mollom.checkContent') {
+ // Map the XML-RPC method name to the corresponding function callback name.
+ drupal_load('module', 'mollom_test');
+ $method_function_map = mollom_test_xmlrpc();
+ $function = $method_function_map[$method];
+
+ // Delete the variable.
+ variable_del($function);
+ }
+
+ /**
* Wraps drupalGet() for additional watchdog message assertion.
*
* @param $options
@@ -872,7 +893,19 @@ class MollomResponseTestCase extends Mol
$this->assertSame('profanity', $result['profanity'], 1);
$session_id = $this->assertSessionID($result['session_id']);
+ // Change the string to contain profanity only.
+ $data['post_body'] = 'profanity';
+ $data['checks'] = 'spam,quality,profanity';
+ $data['session_id'] = $session_id;
+ $result = mollom('mollom.checkContent', $data);
+ $this->assertMollomWatchdogMessages();
+ $this->assertSame('spam', $result['spam'], MOLLOM_ANALYSIS_UNSURE);
+ $this->assertSame('quality', $result['quality'], 0);
+ $this->assertSame('profanity', $result['profanity'], 1);
+ $session_id = $this->assertSessionID($result['session_id']);
+
// Disable spam checking, only do profanity checking.
+ $data['post_body'] = 'spam profanity';
$data['checks'] = 'profanity';
$data['session_id'] = $session_id;
$result = mollom('mollom.checkContent', $data);
@@ -894,6 +927,50 @@ class MollomResponseTestCase extends Mol
}
/**
+ * Tests results of mollom.checkContent() across requests for a single session.
+ */
+ function testCheckContentSession() {
+ $data = array(
+ 'author_name' => $this->admin_user->name,
+ 'author_mail' => $this->admin_user->mail,
+ 'author_id' => $this->admin_user->uid,
+ 'author_ip' => ip_address(),
+ );
+
+ // Sequence: Post unsure spam, correct CAPTCHA, change post into spam,
+ // expect it to be ham (due to correct CAPTCHA).
+ $data['post_body'] = 'unsure';
+ $result = mollom('mollom.checkContent', $data);
+ $this->assertMollomWatchdogMessages();
+ $this->assertSame('spam', $result['spam'], MOLLOM_ANALYSIS_UNSURE);
+ $data['session_id'] = $this->assertSessionID($result['session_id']);
+
+ $captcha_data = array(
+ 'session_id' => $data['session_id'],
+ 'author_ip' => $data['author_ip'],
+ );
+ $result = mollom('mollom.getImageCaptcha', $captcha_data);
+ $this->assertMollomWatchdogMessages();
+ $data['session_id'] = $this->assertSessionID($result['session_id']);
+
+ $captcha_data = array(
+ 'session_id' => $data['session_id'],
+ 'author_ip' => $data['author_ip'],
+ 'author_id' => $data['author_id'],
+ 'captcha_result' => 'correct',
+ );
+ $result = mollom('mollom.checkCaptcha', $captcha_data);
+ $this->assertMollomWatchdogMessages();
+ $this->assertIdentical($result, TRUE, t('CAPTCHA response was correct.'));
+
+ $data['post_body'] = 'spam';
+ $result = mollom('mollom.checkContent', $data);
+ $this->assertMollomWatchdogMessages();
+ $this->assertSame('spam', $result['spam'], MOLLOM_ANALYSIS_HAM);
+ $data['session_id'] = $this->assertSessionID($result['session_id']);
+ }
+
+ /**
* Tests mollom.getImageCaptcha().
*/
function testGetImageCaptcha() {
@@ -1379,27 +1456,26 @@ class MollomBlacklistTestCase extends Mo
class MollomProfanityTestCase extends MollomWebTestCase {
public static function getInfo() {
return array(
- 'name' => 'Profanity filtering',
- 'description' => 'Verify that forms can be properly protected and unprotected.',
+ 'name' => 'Profanity checking',
+ 'description' => 'Tests form protection with text analysis checking for profanity.',
'group' => 'Mollom',
);
}
function setUp() {
parent::setUp('mollom_test');
- // Re-route Mollom communication to this testing site.
- variable_set('mollom_servers', array($GLOBALS['base_url'] . '/xmlrpc.php?version='));
- $this->drupalLogin($this->admin_user);
+ user_role_grant_permissions(DRUPAL_ANONYMOUS_RID, array('access comments', 'post comments', 'post comments without approval'));
}
/**
- * Test the different levels of profanity filtering. With our test Mollom
- * server, the profanity keyword is 'Joomla'.
+ * Tests text analysis profanity checking.
*/
- function testProfanityFiltering() {
- // Protect Mollom test form but do not enable the profanity filter.
+ function testProfanity() {
+ // Protect Mollom test form but do not enable profanity checking.
+ $this->drupalLogin($this->admin_user);
$edit_config = array(
+ 'mollom[checks][spam]' => TRUE,
'mollom[checks][profanity]' => FALSE,
);
$this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS, NULL, $edit_config);
@@ -1414,11 +1490,11 @@ class MollomProfanityTestCase extends Mo
$this->postCorrectCaptcha(NULL, array(), 'Submit', 'Successful form submission.');
$this->assertNoText($this->profanity_message);
- // Enable the profanity filter setting for this form.
+ // Enable profanity checking, disable spam checking.
$this->drupalLogin($this->admin_user);
$edit_config = array(
'mollom[checks][spam]' => FALSE,
- 'mollom[checks][profanity]' => 1,
+ 'mollom[checks][profanity]' => TRUE,
);
$this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS, NULL, $edit_config);
$this->drupalLogout();
@@ -1430,13 +1506,76 @@ class MollomProfanityTestCase extends Mo
// Verify that we are able to post after removing profanity, as the error
// message suggests.
- // @todo Doesn't really make sense in testing mode, and even more so, when
- // posting to the local fake server.
$edit['body'] = 'This is a post just for unsure Joomla lovers.';
$this->drupalPost('mollom-test/form', $edit, 'Submit');
$this->assertText('Successful form submission.');
$this->assertNoText($this->profanity_message);
}
+
+ /**
+ * Tests text analysis with both profanity and spam checking.
+ */
+ function testProfanitySpam() {
+ variable_set('comment_preview_article', DRUPAL_OPTIONAL);
+ $node = $this->drupalCreateNode(array('type' => 'article'));
+ $langcode = LANGUAGE_NONE;
+
+ // Enable spam and profanity checking for the article node comment form.
+ $this->drupalLogin($this->admin_user);
+ $edit_config = array(
+ 'mollom[checks][profanity]' => TRUE,
+ 'mollom[checks][spam]' => TRUE,
+ );
+ $this->setProtection('comment_node_article_form', MOLLOM_MODE_ANALYSIS, NULL, $edit_config);
+ $this->drupalLogout();
+
+ // Sequence: Post profanity (ham), remove profanity (still ham), and expect
+ // that to be accepted.
+ $edit = array(
+ 'subject' => $this->randomName(),
+ );
+ $this->drupalGet("node/{$node->nid}");
+ $this->assertNoCaptchaField();
+ $this->assertPrivacyLink();
+
+ $edit["comment_body[$langcode][0][value]"] = 'profanity ham';
+ $this->drupalPost(NULL, $edit, t('Save'));
+ $this->assertText($this->profanity_message);
+ $this->assertNoText(t('Your comment has been posted.'));
+ $session_id = $this->assertSessionIDInForm();
+
+ $edit["comment_body[$langcode][0][value]"] = 'not profane ham';
+ $this->drupalPost(NULL, $edit, t('Save'));
+ $this->assertNoText($this->profanity_message);
+ $this->assertText(t('Your comment has been posted.'));
+ $this->assertRaw('' . $edit["comment_body[$langcode][0][value]"] . '
', t('Comment previously containing profanity was found.'));
+ $cid = db_query('SELECT cid FROM {comment} WHERE subject = :subject ORDER BY created DESC', array(':subject' => $edit['subject']))->fetchField();
+ $this->assertMollomData('comment', $cid, $session_id);
+
+ // Sequence: Post unsure spam (not profanity), post profanity along with
+ // correct CAPTCHA, and expect that to be rejected.
+ $this->web_user = $this->drupalCreateUser();
+ $this->drupalLogin($this->web_user);
+ $edit = array(
+ 'subject' => $this->randomName(),
+ );
+ $this->drupalGet("node/{$node->nid}");
+ $this->assertNoCaptchaField();
+ $this->assertPrivacyLink();
+
+ $edit["comment_body[$langcode][0][value]"] = 'unsure';
+ $this->drupalPost(NULL, $edit, t('Save'));
+ $this->assertCaptchaField();
+ $this->assertNoText($this->profanity_message);
+ $this->assertNoText(t('Your comment has been posted.'));
+ $session_id = $this->assertSessionIDInForm();
+
+ $edit["comment_body[$langcode][0][value]"] = 'unsure profanity';
+ $this->postCorrectCaptcha(NULL, $edit, t('Save'));
+ $this->assertNoCaptchaField();
+ $this->assertText($this->profanity_message);
+ $this->assertNoText(t('Your comment has been posted.'));
+ }
}
/**
@@ -1844,18 +1983,18 @@ class MollomCommentFormTestCase extends
$session_id = $this->assertSessionIDInForm();
$this->assertPrivacyLink();
- // Try to submit the form by using an invalid CAPTCHA. At this point,
- // the submission should be rejected and a new CAPTCHA generated (even
- // if the text of the comment is changed to ham).
- $this->postIncorrectCaptcha(NULL, array('comment_body[und][0][value]' => 'ham'), t('Save'));
+ // Try to submit the form by solving the CAPTCHA incorrectly. At this point,
+ // the submission should be blocked and a new CAPTCHA generated, but only if
+ // the comment is still neither ham or spam.
+ $this->postIncorrectCaptcha(NULL, array(), t('Save'));
+ $this->assertCaptchaField();
$session_id = $this->assertSessionIDInForm();
$this->assertPrivacyLink();
- // Now try using a valid CAPTCHA. The CAPTCHA form should no longer
- // be present.
+ // Correctly solving the CAPTCHA should accept the form submission.
$this->postCorrectCaptcha(NULL, array(), t('Save'));
- $this->assertRaw('ham
', t('A comment that is known to be ham appears on the screen after it is submitted.'));
- $cid = db_query('SELECT cid FROM {comment} WHERE subject = :subject ORDER BY created DESC', array(':subject' => 'ham'))->fetchField();
+ $this->assertRaw('unsure
', t('A comment that may contain spam was found.'));
+ $cid = db_query('SELECT cid FROM {comment} WHERE subject = :subject ORDER BY created DESC', array(':subject' => 'unsure'))->fetchField();
$this->assertMollomData('comment', $cid, $session_id);
// Try to save a new 'spam' comment; it should be rejected, with no CAPTCHA
@@ -2263,6 +2402,7 @@ class MollomDataTestCase extends MollomW
$this->drupalPost('admin/structure/types/manage/article', $edit, t('Save content type'));
// Log out and post a comment as anonymous user.
+ $this->resetServerRecords();
$this->drupalLogout();
$this->drupalGet('node/' . $node->nid);
$this->clickLink(t('Add new comment'));
@@ -2295,6 +2435,7 @@ class MollomDataTestCase extends MollomW
$this->assertSame('author_id', $data['author_id'], NULL);
// Log in admin user and edit comment containing spam.
+ $this->resetServerRecords();
$this->drupalLogin($this->admin_user);
$this->drupalGet('comment/' . $comment->cid . '/edit');
// Post without modification.