Domain Ip

Hello,

Here is the review

.info file
files[] = domain_ip.module
This should not be posted here, check this - http://drupal.org/node/542202
Only extra files should be added to info file.

.module file
@file declaration is missing
Missing comments for - function domain_ip_whitelist_own_address, domain_ip_switch_list

.admin.inc file
domain_ip_form - Not sure on this, but I believe you need not to mention

$form['#validate'][] = 'domain_ip_form_validate';
$form['#submit'][] = 'domain_ip_form_submit';

Drupal will by default hit these functions, if names are not mentioned. Please check, on this, if this works, you save on extra code.

elseif ($ip_start == ip_address() || ($ip_end && domain_ip_check_range($ip_start . '-' . $ip_end, ip_address()))) {
  // This fires if users own IP is in the selected range. Currently we wont do anything in that case.
 }

What are we doing in this function, this can work, but hopefully you will add some activity. :)

Line 147 - return;, why do we do that ?

Apart from all this, there are some minor formatting issues, check here - http://ventral.org/pareview/httpgitdrupalorgsandboxsmiletrl1795244git

Also, to fasten the Review process, please read - https://drupal.org/node/1410826 - and gain a review bonus :)

Welcome Smiletrl and thank you for helping on the review process !

Automatic review

All seems ok ok.

Manual review

1. There is an XSS vulnerability on the display of the banned message.
If you set this as message an alert is displayed when banned. <script>alert('hello');</script>
Please check that all user entered content is sanitized on output.

function domain_ip_deny_access($domain_name) {
  header($_SERVER['SERVER_PROTOCOL'] . ' 403 Forbidden');
  $message = variable_get('Banned_IP_error_message', 'Sorry, you are banned for this site.');
  print $message;
  exit();
}

I've found another issue but it's related to the domain module not the domain_ip module...

** At the page admin/structure/domain when clicking on edit domain i'm redirected to another domain name... I've made a local installation with three domain names at /etc/hosts pointing to the same folder files to test this module. Then I blacklisted my self from one of the domain names. When I enter to edit the setup using a domain name where I'm not blacklisted and click on domain name I'm redirected to localhost (without the global $base_url so to a 404 page).

Except issue 1 the code is clean from what I saw.
Just correct this XSS problem and I guess this module is ready to RTBC.

manual review:

1. domain_ip_deny_access(): is that function really necessary? You could simply use drupal_access_denied().
2. domain_ip_deny_access(): also that does not even return valid HTML? And you should use drupal_exit() instead of exit.
3. "'#title' => 'Your name',": all user facing text must run through t() for translation.
4. domain_ip_restriction_page(): $ip->author is user provided text and should be sanitized before printing. I could not exploit that because the textfield is limited to 15 characters, but you should take care of it anyway. See also http://drupal.org/node/28984
5. you could add a check for the user's IP adress in a validation function and display a confirmation form if ip_address() is in the blacklist range.
6. "array('@list' => check_plain($list))": that check_plain() is not necessary since the "@" placeholder will do the sanitization fro you.
7. quite a few spelling and grammar errors here and there, I would recommend proof reading from a native english speaker. Also sentences in ALL CAPS are hard to read and annoying.

But that are not hard blockers, otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Remove some text.

no objections for more than a week, so ...

Thanks for your contribution, smiletrl!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

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

Thanks, 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.

Thanks to the dedicated reviewer(s) as well.