CVS edit link for acetolyne

http://drupal.org/sandbox/acetolyne/1082234

I have developed a new module for Drupal. I wanted to have bot control as do many others and know there are many other modules that do this
through many different methods. My anti-bot module works by utilizing a database of known bots. These bots are caught using a few various
methods. The main way is through the use of honey nets. The bots information is then entereed into a database which get queries through
enumeration of a url that contains the data you want to query from the database. The database then returns to you a response code that is
tested to see if a bot was found. I hope to allow users to use this technique to keep their sites safe from bots and spam. My program simply
provides a way to add botscout functionality to your drupal site and keep users from creating an account based on their ip, name, email, or
any combination of the three.Information on exactly how botscout catches bots please visit www.botscout.com. My module hooks into the registration form and optionally the site-wide contact form to either allow the user or deny the user form submission based on the return we get from the botscout API.

I have done my best to conform to the drupal coding standards to bring you a secure user friendly module that will help protect your site.
I hope that my module can make administration of people's sites much easier. I use this module myself and plan on keeping it up to date and
adding new features in the future to make it even more user friendly and secure. I want to next develop this module for the Drupal 7x core and
hope that people will find my module useful and easy to use. I have worked hard and spent alot of time to learn the drupal coding standards and
work out all bugs and security issues in my module before I released it but will take user feedback into consideration to update the module to be
better for those who use it.

The new 7.x module works much more efficiently than the older module thanks to the new Drupal API calls available for hooking into forms. It is much safer from XSS attacks and the code is cleaner and faster.

NOTE TO PROJECT TESTERS: If you need to check functionality for when a bot is detected go to www.botscout.com and retrieve one of the latest email or names. Make sure you have settings to block by email or name, then it is suggested you use a free public web proxy to connect to the site else your ip address may be added to the database of bots that are blocked by this program.

Download this project with Git:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/acetolyne/1082234.git botscout

Pareview results at:
http://pareview.sh/pareview/httpgitdrupalorgsandboxacetolyne1082234git-7...

Comments

acetolyne’s picture

Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new11.76 KB

Here is the module file in zip format

avpaderno’s picture

Issue tags: +Module review

Hello, and thank you for applying for a CVS account. I am adding the review tags, and some volunteers will report what needs to be changed in the proposed module.

acetolyne’s picture

StatusFileSize
new10.84 KB

Thanks for getting my project started I have made a couple small additions to my file please use the new one attached here.
The new additions take care of setting a variable correctly and adding the name of the site on the email alerts, This helps for multisite configurations so that you can tell which site has blocked the bot. I also have plans on adding system logs that shoe the username, email, and ip address of any user that was blocked by the Botscout module right now it only alerts you that someone was blocked but doesnt provide their information. The live demo site I mentioned before has more information on my module and future additions I am planning.

Ela’s picture

Component: Miscellaneous » miscellaneous

.. subscribing for updates. Would be great if your module worked also for contact forms

acetolyne’s picture

not a bad idea Ill see what I can do about implementing it into contact forms as well thx for the idea.

arianek’s picture

Status: Needs review » Postponed

Hi. Please read all the following and the links provided as this is very important information about your CVS Application:

Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for your application. Please read the following documentation on how this affects and benefits you and the application process:
Migrating from CVS Applications to (Git) Full Project Applications

  • The status of this application will be put to "postponed" and by following the instructions in the above link, you will be able to reopen it.
  • Or if your application has been "needs work" for more than 5 weeks, your application will be marked as "closed (won't fix)". You can still reopen it, by reading the instructions above.
acetolyne’s picture

Project: Drupal.org CVS applications » Drupal.org security advisory coverage applications
Status: Postponed » Needs review

My new git account is set up first time using any version control system but I believe everything is in place now and I am more used to using the version control system my new project page can be found at :

http://drupal.org/sandbox/acetolyne/1082234

thank you for you time I appriciate you looking over this project

acetolyne’s picture

Component: miscellaneous » new project application

forgot to mark the component as new project application sorry my mistake still need someone to review this project when they can thanks for your time

dave reid’s picture

Status: Needs review » Needs work

I'm finding it a little hard to review the code without any indentation at all. Could you please review the Drupal coding standards and also download the Coder module and run it against your module's code to help clean it up? That will help ease the review process.

acetolyne’s picture

Status: Needs work » Needs review

Ok I have updated my coding to be more friendly to the eyes it now returns no warnings at all from the coder module. I have also ported some new features into the module including a counter to tell you how many bots have been blocked, a botscout footer, and the ability to block bots on the site-wide contact form as well, I added hook_help feature for better support, and I also fixed some page display problems as well as a security vunerability in the old code. The new code has been pushed and can be found at the project page. Thank you for taking the time to review my module

sreynen’s picture

Title: acetolyne [acetolyne] » BotScout

Changing title to make this easier to find.

ralt’s picture

Component: new project application » module
Priority: Normal » Critical

Changing priority according to the new priority guidelines.

davidhernandez’s picture

Priority: Critical » Normal

Going back to Dave Reid's comment, the code style still needs work. Please pay particular attention to the doxygen standard for comment blocks before functions. http://drupal.org/node/1354 You are also still missing a lot of indentation in the later part of the .module file.

We aren't trying to be nit-picky, it just makes the code easier to read and understand. We mostly use the repository viewer to look over the code, and it is currently difficult to study.

Remove the license.txt file. It is added automatically.

Regarding code, why do you have these lines at the top? They are outside of the functions and will get executed every time you module file is included.

//Setup some variable to be used.
$filter = '';
$keyfilter = '';

acetolyne’s picture

I have updated my code I believe it is easier to understand for people viewing the code. i have also moved the two variables you had questioned into the appropriate function in the coding. I have also removed the license.txt file. Thank you for the information about that file. If my commenting still is not satisfactory please let me know and I will try to correct any errors however I believe it is good now. thanx for your time

greggles’s picture

Status: Needs review » Needs work
StatusFileSize
new5.53 KB

Suggestions

Please take a moment to make your project page follow tips for a great project page. In particular your page could make use of more distinct areas using headers to split apart the paragraphs.

Please remove $Id$ from the .info file. The first and last lines of the info file are also formatted quite strangely with <<< and >>> characters

The module doesn't actually pass a 6.x-2.x review via coder on "minor" - can you fix that?

There are a few coding standards that coder doesn't catch that your module is missing. Attached is a patch that shows some of them. Please apply similar formatting across your module and see coding standards for more advice.

Required changes

You should remove the GPL from the .module - this will be automatically packaged with the module by drupal.org. I did this in the patch.

Permissions are not translated. So 'access arguments' => array(t('access administration pages')), should not have the translation. Also, I think admin/settings/botscout should use the "administer botscout" permission. I removed translation from your hook_perm as well.

The code under the comment * Changes the form properties based on the page they are on. is not typical of a Drupal module. If you need the code to run on every page try hook_boot.

Rather than doing:


//Changes the form if we are on the registration page.
if ($results == 'user/register') {

/**
 * Implements hook_form_alter().
 *
 * changes the form to redirect the registration form to the botscout page.
 */
  function botscout_form_alter(&$form, $form_state, $form_id) {

    $url = (!empty($_SERVER['HTTPS'])) ? "https://". $_SERVER['SERVER_NAME'] : "http://". $_SERVER['SERVER_NAME'];

You should do:



/**
 * Implements hook_form_alter().
 *
 * changes the form to redirect the registration form to the botscout page.
 */
function botscout_form_alter(&$form, $form_state, $form_id) {
  //Changes the form if we are on the registration page.
  if ($form_id == 'name_of_proper_form_here') {
    $url = (!empty($_SERVER['HTTPS'])) ? "https://". $_SERVER['SERVER_NAME'] : "http://". $_SERVER['SERVER_NAME'];
...

It is common to key your form_alter off the form_id, and basing it on the URL might work, but it's not common, and even if you key off the URL that should be done inside a single form_alter rather than having conditional definitions of the hook_form_alter.

misc’s picture

The applicant has been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256

acetolyne’s picture

Update: I got stuck in the transition from CVS to GIT and this caused great delays it took several months, nearly a year to get anyone to look at my project and after that it took a long time between each review of my project so I got rather discouraged and did think about abondoning this project all together. I have decided to go ahead with this project I dont have onternet access for about a week after that I will review the last suggestions and check the patch that has been submitted to make sure it works how its sopposed to and also finish up the BotScout module for D7. I apollogize for the delays on this project and in the future will stay more on top of the project. Cheers

acetolyne’s picture

Status: Needs work » Needs review

Ok I have updated the project page to be more user friendly.I have also done a rewrite of the coding for better functionality and coding standards. I took out the translation on the access arguments as suggested and fixed the botscout.info file. Thank you for all your suggestions I think the new code should be good it is again ready for someone to review. Ive also took out some unneeded messages to users and added better logging for admins. The code runs much smoother :)

I ran the code in the Coder module and it passed on minor.

Thank you for your time in reviewing my project and for the suggestions you made. Earlier I had tried to use hook_form_alter() instead of the page url to change things but for some reason had trouble with the logic of the program but now with the rewrite I did it is successfully using hook_form_alter()

Please let me know if something still isn't correct I have tried hard to make this module meet all coding standards.

misc’s picture

Status: Needs review » Needs work

If I run the project through PAReview.sh online service http://ventral.org/pareview/httpgitdrupalorgsandboxacetolyne1082234git, I can see errors and warnings you need to take care about. It is mostly about formatting. If you have any questions about this, feel free to ask.

If you handle this quickly I will try to do a manual review as soon as possible.

acetolyne’s picture

Status: Needs work » Needs review

Ive fixed all errors reported by the online service I had alot of trouble making git GUI conform and not alter the line endings but at last I have got it working the report now comes back clean.

Thank you for introducing me to that service it seems like it will help mainstream the project applications and development reviews in general. I wish you could check local files though so you didnt have to commit to re-check the results. Anyhow let me know if there is anything else wrong with the module and thanks for oyur time :)

patrickd’s picture

Status: Needs review » Needs work

you can check localy the same way by installig pareview.sh on you machine

your install.txt looks a little unsystematic, maybe you can make it more structured?

It makes no sense to create a new package called "bot" if there is only your module in.
you should remove package = "Bot" so it'll be added to "other" module group.

function botscout_help($path, $arg) {
  switch ($path) {
    case 'admin/help#botscout':
      return '<p>' . t('Botscout is a module that prevents bots from doing things like registering on your site and sending site-wide contact forms. This module brings the Botscout functionality to Drupal. more info available at www.botscout.com. More info available in the README.TXT file, or installation instructions in the INSTALL.TXT file. Botscout by Acetolyne', array('@blocks' => url('admin/structure/block'))) . '</p>';
  }
}

Is there a reason for providing @blocks but not using it in the t() function its self?

Do it the drupal way ;-)

function botscout_perm() {
  $permsa = t('access botscout content');
  $permsb = t('administer botscout');
  return array($permsa, $permsb);
}

=>

function botscout_perm() {
  return array (
    t('access botscout content'), 
    t('administer botscout'),
  );
}

variable_get('botscout_contact', 0),
A real NULL would make more sense here? (dunno about d6)
variable_get('botscout_contact', NULL),

$returned_data = curl_exec($ch);
is there a reason for not using drupal_http_request() here?

Generally you should use TRUE or FALSE instead of 1 and 0!

$url = (!empty($_SERVER['HTTPS'])) ? "https://" . $_SERVER['SERVER_NAME'] : "http://" . $_SERVER['SERVER_NAME'];
why don't you use url() with 'absolute' option ?

I think that's enough for now

acetolyne’s picture

Status: Needs work » Needs review

I have updated my module to meet the suggestions you have made and it is again ready to be reviewed. Thanks for your time :) Below is a couple notes about my changes.

you can check localy the same way by installig pareview.sh on you machine

Ty Ill look into it

your install.txt looks a little unsystematic, maybe you can make it more structured?

agreed it got messed up due to formatting issues in my Git software it should be fixed now.

It makes no sense to create a new package called "bot" if there is only your module in.
you should remove package = "Bot" so it'll be added to "other" module group.

Ok changed.

Is there a reason for providing @blocks but not using it in the t() function its self?

Opps no it shouldnt be there fixed

function botscout_perm() Has been reformatted

variable_get('botscout_contact', 0),
A real NULL would make more sense here? (dunno about d6)
variable_get('botscout_contact', NULL),

Actually this is a checkbox so NULL doesnt work it needs to be FALSE instead, however I did get the point you were making and have changed the values to TRUE instead of 1 and either FALSE or NULL instead of 0 and changed all if statements throughout my module to use TRUE, FALSE, and NULL values instead of the original 0 and 1 values.

$returned_data = curl_exec($ch);
is there a reason for not using drupal_http_request() here?

Did not know about that function, my module now implements this instead of the Curl or file_get_contents().

$url = (!empty($_SERVER['HTTPS'])) ? "https://" . $_SERVER['SERVER_NAME'] : "http://" . $_SERVER['SERVER_NAME'];
why don't you use url() with 'absolute' option ?

Well I didn't need the absolute option I instead took out the basename part and simply used url() appending the correct page.

Thank you for all the suggestions to make this module even better. I try hard to update this module when something is found to be wrong with it or something could improve it's performance. Thanks for your time in reviewing my module. If here is anything else I can do to improve it please let me know.

sankatha’s picture

Status: Needs review » Needs work
patrickd’s picture

@sankatha woaahhh, please don't paste raw reports into issues look how clumsy that is! ;-)
please edit your comment and provide a link instead!

// edit, Thanks! :)

acetolyne’s picture

Status: Needs work » Closed (won't fix)

After a year and a half of fixing things to others standards and afterwards being reviewed by a different person only to have to change things to different or new specifications I have decided to close my project application.

I have only limited time on the internet and have needed to find more family time as of late.

The project has NOT been abandoned I will soon be getting the D7 module out, however this project at this time will not be a Full application that is supported by Drupal. It will remain atleast for now a sandboxed project and will continue to work and get updates. As always please leave any comments or suggestions for the project and they will be considered. I am hoping to focus more on the D7 version since most users have migrated. Check back soon for updates on the D7 version of the module

Thanks to everyone whom has taken their time to review my project application. I greatly appriciate the time you have taken and the suggestions you all have given, I have simply grown tired of constantly being reviewed by users whom have different standards for what coding should look like. Have a great day please feel free to ask any questions you may have Thx ~Acetolyne~

patrickd’s picture

I'm sorry about that :(

Feel free to re-open whenever you like!

acetolyne’s picture

Issue summary: View changes
Status: Closed (won't fix) » Needs review
acetolyne’s picture

Hi after a long time away I have totally re-written the module. When someone has some time I would appriciate if they could review my project for submission as a full project. I deleted old files and patches since the module has been totally re-written the do not apply anymore. It passes all testing I have done, please let me know if there is anything I can do better in my coding standards and thanks for your time!!

PA robot’s picture

Status: Needs review » Needs work

Git clone command for the sandbox is missing in the issue summary, please add it.

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.

acetolyne’s picture

Issue summary: View changes
acetolyne’s picture

Issue summary: View changes
acetolyne’s picture

Status: Needs work » Needs review
acetolyne’s picture

Title: BotScout » [D7] BotScout

Just added the D7 into title so reviewers know its a D7 module and not a D8 module

acetolyne’s picture

Issue summary: View changes

added Pareview results link showing my module passes all tests
have not yet implemented any automated testing though. In the future i do plan on doing this.

maen’s picture

I will make a review today, but first off: your git link is wrong:

use
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/acetolyne/1082234.git botscout
instead.

Your link is for the maintainer only. (Honestly, I made it also wrong for my first project).

maen’s picture

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
Master Branch
No: Does not follow the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements.
3rd party assets/code
Yes: Follows the guidelines for 3rd party assets/code.
Needs a webservice, so o.k.
README.txt/README.md
No: Does not follow the guidelines for in-project documentation and/or the README Template.
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
Yes: Meets the security requirements.

This review uses the Project Application Review Template.

A word from my PoV:

I can't get it to work to mail from system. But this can be due to my settings at localhost. I debugged the code until line 284 so it seems to be correct! I tested with many different bots token from the list at botscout.com.
So in my mind code is alright.

acetolyne’s picture

Issue summary: View changes
acetolyne’s picture

Thank you for taking the time to review my module. You must have an email setting that is faulty or something, emailing works on my systems. I have fixed the readme file and the git clone link. I however do not understand what is wrong with my master branch, could someone help me understand what needs changed please. Even after reading the guidelines for master branch I don't see what is wrong. i have deleted the master branch and set the head to the 7x branch.

I think I have the repository in a better state now. but If there is something still wrong with it please let me know what needs to be done rather than just saying it doesn't comply with the standards because as far as i can tell it does comply with the repository standards. Thanks for your time in reviewing my module.

greggles’s picture

Status: Needs review » Reviewed & tested by the community

As far as I can tell you are following the guidelines for master branch. Seems like this is RTBC!

avpaderno’s picture

Assigned: Unassigned » avpaderno
Status: Reviewed & tested by the community » Fixed

I updated your account so you can promote new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thank you, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thank you to the dedicated reviewer(s) as well.

acetolyne’s picture

Status: Fixed » Closed (fixed)

Thank you very much the review process was much smoother this time, I had in the past closed this project due to the switch between CVS and GIT here on Drupal and the review process was very frustrating this time though it went smooth and i am very thankful to those that took their time to review my project. I appreciate it very much. The project has finally been moved to a full project and will be maintained and updated as needed. I ill make sure to review some other projects in the future and help with the community here at Drupal as I can. Closing this now as I have moved to a full project. Thanks again!

avpaderno’s picture

Status: Closed (fixed) » Fixed

Fixed is the status we use when the task is completed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.