Skip to main content
IPv6
Source Link
Kate
  • 8.5k
  • 9
  • 21

Avoid unnecessary repetition. RpetitionRepetition is every programmer's enemy. There is also more stuff that you repeat in other pages, for example:

Note that you could use templates for your mails too. All you need is text files with some delimited tags eg %VERIFICATION_CODE%, then you simply replace those tags with the values from your variables.

IPv6

What is your problem with IPv6 ? Are you storing the value in a varchar field ? Then allocate at least 45 characters but there is nothing special about it. Of course IPv6 addresses tend to be longer and have a markedly different pattern. The question is, what is the purpose of storing this information and how do you intend to use it.

Avoid unnecessary repetition. Rpetition is every programmer's enemy. There is also more stuff that you repeat in other pages, for example:

Note that you could use templates for your mails too. All you need is text files with some delimited tags eg %VERIFICATION_CODE%, then you simply replace those tags with the values from your variables.

Avoid unnecessary repetition. Repetition is every programmer's enemy. There is also more stuff that you repeat in other pages, for example:

Note that you could use templates for your mails too. All you need is text files with some delimited tags eg %VERIFICATION_CODE%, then you simply replace those tags with the values from your variables.

IPv6

What is your problem with IPv6 ? Are you storing the value in a varchar field ? Then allocate at least 45 characters but there is nothing special about it. Of course IPv6 addresses tend to be longer and have a markedly different pattern. The question is, what is the purpose of storing this information and how do you intend to use it.

Source Link
Kate
  • 8.5k
  • 9
  • 21

Yes, use an include file for your DB connection. If you change the DB password or the server, you'll have to update several pages and make sure they all match. That is tedious, and even if you have just 4 pages today, that could be 20 or more in the future.

Avoid unnecessary repetition. Rpetition is every programmer's enemy. There is also more stuff that you repeat in other pages, for example:

date_default_timezone_set('America/Los_Angeles'); 

What is the purpose of this ? Anyway, you should have some sort of config file to centralize settings. And then require_once should do.

DNS records

I think the merit of this is debatable:

//Check Email Domain MX Record $email_host = strtolower(substr(strrchr($Temail, "@"), 1)); if (!checkdnsrr($email_host, "MX")) { header("Location: invalid.html"); exit (0); } 

You are checking if the domain has at least one MX record. Presumably you want to thwart random submissions by spammers or whatever. But in itself that proves very little, and it does not demonstrate that the submission was made in good faith. I could use anybody's E-mail address, as long as it passes the test. So I don't see a lot of added value in this personally.

Also, transient DNS errors may occur. This function may fail from time to time, even with legitimate domain names. Normally, the mail is not delivered directly but goes to a local queue and in case of DNS lookup failures or whatever your MTA will try sending the mail at regular intervals.

Also, if I am not wrong, the RFC says that in the absence of a MX record, the MTA can use a A record as a fallback (keep in mind that many DNS zones are poorly configured).

strtolower is not needed, as domain names are not case-sensitive. Although you may want to normalize input and force the whole E-mail address to lowercase in case it contains a mix of lowercase and uppercase characters. Purely for cosmetic reasons.

Includes

Don't do this:

//send verification email using seperate php file include_once 'vEmail.php'; 

That makes the code even more difficult to follow and understand. Remember, code that is hard to read/understand tends to be more buggy and less secure.

Instead, write a dedicated function that returns a boolean value, or anything you want. But you can have an include at the top of your code to import your functions of course. Could be something like this:

if (!send_verification_email($email, $verification_code)) { // an error occured, do something } // proceed normally 

Misc

I think you have too many redirect pages, just consider this:

  • expired1.html
  • expired2.html
  • email.html
  • invalid.html
  • notallowed.html
  • duplicate.html
  • failure.html
  • success.html
  • verified.html

And of course they must all be very similar. I would strongly advise to evolve your coding practices - use a framework or at least a templating solution. One page would suffice, and you pass some parameters like a custom message so that the page can be rendered in many different ways. You don't need a page for every possible scenario. Just think about maintenance and bloat.

Note that you could use templates for your mails too. All you need is text files with some delimited tags eg %VERIFICATION_CODE%, then you simply replace those tags with the values from your variables.