About:

The Check DNS module simply prevents user registration with an invalid email domain on the user registration form.

It validates the email domain before registration and checks if the domain exists. This prevents false registration.

Project:

https://www.drupal.org/project/check_dns

Git instructions:

git clone --branch 8.x-1.x https://git.drupalcode.org/project/check_dns.git

PAReview checklist :

https://pareview.sh/pareview/https-git.drupal.org-project-check_dns.git

Comments

Gaurav_drupal created an issue. See original summary.

baskaran’s picture

Status: Needs review » Needs work

Hi Gaurav_drupal,

Please fix the below change.

FILE: ...00000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/README.txt
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
14 | WARNING | Line exceeds 80 characters; contains 86 characters
--------------------------------------------------------------------------

Gaurav_drupal’s picture

Status: Needs work » Needs review

Thanks @baskaran for the review, fixed and pushed to 8.x-1.x

ashutosh.mishra’s picture

Issue summary: View changes
ashutosh.mishra’s picture

Issue summary: View changes

Thank you for applying! I modified the Git instructions for non-maintainer users and added PAReview checklist link.

shaktik’s picture

Thank you for your contribution!, Gaurav_drupal
bit optimization here
from

$mail = Html::escape($mail);
      $mail = explode('@', $mail);

to
$mail = explode('@', Html::escape($mail));

the drupal check looks good.

check_dns drupal-check -ad .

2/2 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

[OK] No errors

Gaurav_drupal’s picture

Thank you @shaktik for review. I have refactored and pushed the changes to 8.x-1.x

apaderno’s picture

klausi’s picture

Status: Needs review » Needs work
  1. CheckDnsService: no need to run Html::escape() here since you are not printing anything directly to HTML.
  2. check_dns.services.yml: always use 2 space indentation in YML files.

Looks good to me otherwise. Setting this to "needs work" because you need to know when to use Html::escape() and when not, but after fixing that I think we can approve this.

apaderno’s picture

Just for helping to find the code that should not sanitize the method parameter, this is the code.

  public function validateEmail(string $mail) {

    if (filter_var($mail, FILTER_VALIDATE_EMAIL)) {
      $mail = explode('@', Html::escape($mail));

      return $this->validateHost(end($mail));
    }
    else {
      return FALSE;
    }
  }

As side note, there isn't any need to put an empty line before the first code line of a function/method and after the last code line of a function/method.

Gaurav_drupal’s picture

Status: Needs work » Needs review

Thank you @kiamlaluno @klausi for review and your invaluable suggestions. I have refactored and pushed the changes to the 8.x-1.x branch.

klausi’s picture

Status: Needs review » Fixed

Thanks for your contribution, Gaurav!

I updated your account so you can opt into security advisory coverage now.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on Slack or IRC in #drupal-contribute. So, come hang out and stay 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.

Status: Fixed » Closed (fixed)

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