[D7] QapTcha Integration

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxdevelnk2178067git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Hi develnk,

First off, nice module! I took some time to review, and noticed a few things that might be helpful:

• Would be helpful to have installation and configuration instructions in the readme.txt file.
• It looks like you are setting two variables in the variables table. Consider adding hook_unistall and deleting these variables (qaptcha_auto_submit, qaptcha_auto_revert) upon module uninstall. This will reduce database cruft.
• There is something strange going on with default variables on the form page (admin/config/people/captcha/qaptcha). When I first install the module, and before saving the form, your FALSE default value for qaptcha_auto_submit is not working. Strangely, if I change the value to TRUE it works. In testing the same is the case for the qaptcha_auto_revert variable; TRUE works, but FALSE does not. I'm not an expert on these matters, but it would appear to be a bug of some kind. See screenshot.

Hope this helps!

Hi develnk.
This looks good and all pareview issues are fixed.
I got this working by following your README.txt and cloning the forked library from your github account.
One hint:

• Make the module description translatable? Line 26

Master Branch

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

Anyway nice contribution, will think about using it in future projects.
Holger

Moving to needs work for code comment issues and a non-empty master branch.

Master branch:
Moving from a master branch to a version branch.

/**
 * Implements of hook_captcha().
 */
function qaptcha_captcha() {

Format of comment should be "* Implements hook_foo().

function _qaptcha_plugin_version() {
   return TRUE;
}

Me thinks this doesn't really serve the purpose of a version callback function very well.

  function _qaptcha_validation() {
    if (isset($_SESSION['qaptcha_key']) && !empty($_SESSION['qaptcha_key'])) {
      $key = $_SESSION['qaptcha_key'];
      unset($_SESSION['qaptcha_key']);
  
      // Check if the random input created exists and is empty.
      if (isset($_POST['' . $key . '']) && empty($_POST['' . $key . ''])) {
        return TRUE;
      }
      else {
        return FALSE;
      }
    }
  }

Checking for isset and !empty on the same variable isn't necessary. Empty will suffice.
Checking for isset and empty on the same variable creates strange code. Perhaps some code comments would assist.

A hook menu that stores data to SESSION for anonymous users can easily result in a DDoS. My reasoning for DDoS is that SESSION is backed by the database and the table could easily grow very large. I'll leave it to one of the more experienced reviewers to corroborate and add the security tag.

function qaptcha_menu() {
  $items['qaptcha'] = array(
    'page callback' => '_qaptcha_session_save',
    'access callback' => TRUE,
    'type' => MENU_CALLBACK,
  );
}

function _qaptcha_session_save() {
  session_start();
  $response['error'] = FALSE;

  if (isset($_POST['action']) && isset($_POST['qaptcha_key'])) {
    $_SESSION['qaptcha_key'] = FALSE;

    if (htmlentities($_POST['action'], ENT_QUOTES, 'UTF-8') == 'qaptcha') {
      $_SESSION['qaptcha_key'] = $_POST['qaptcha_key'];
      echo json_encode($response);
    }
    else {
      $response['error'] = TRUE;
      echo json_encode($response);
    }
  }
  else {
    $response['error'] = TRUE;
    echo json_encode($response);
  }
}

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.