Skip to content

Make isFQDN more strict #1180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ Validator | Description
**isEmail(str [, options])** | check if the string is an email.<br/><br/>`options` is an object which defaults to `{ allow_display_name: false, require_display_name: false, allow_utf8_local_part: true, require_tld: true, allow_ip_domain: false, domain_specific_validation: false }`. If `allow_display_name` is set to true, the validator will also match `Display Name <email-address>`. If `require_display_name` is set to true, the validator will reject strings without the format `Display Name <email-address>`. If `allow_utf8_local_part` is set to false, the validator will not allow any non-English UTF8 character in email address' local part. If `require_tld` is set to false, e-mail addresses without having TLD in their domain will also be matched. If `ignore_max_length` is set to true, the validator will not check for the standard max length of an email. If `allow_ip_domain` is set to true, the validator will allow IP addresses in the host part. If `domain_specific_validation` is true, some additional validation will be enabled, e.g. disallowing certain syntactically valid email addresses that are rejected by GMail.
**isEmpty(str [, options])** | check if the string has a length of zero.<br/><br/>`options` is an object which defaults to `{ ignore_whitespace:false }`.
**isFloat(str [, options])** | check if the string is a float.<br/><br/>`options` is an object which can contain the keys `min`, `max`, `gt`, and/or `lt` to validate the float is within boundaries (e.g. `{ min: 7.22, max: 9.55 }`) it also has `locale` as an option.<br/><br/>`min` and `max` are equivalent to 'greater or equal' and 'less or equal', respectively while `gt` and `lt` are their strict counterparts.<br/><br/>`locale` determine the decimal separator and is one of `['ar', 'ar-AE', 'ar-BH', 'ar-DZ', 'ar-EG', 'ar-IQ', 'ar-JO', 'ar-KW', 'ar-LB', 'ar-LY', 'ar-MA', 'ar-QA', 'ar-QM', 'ar-SA', 'ar-SD', 'ar-SY', 'ar-TN', 'ar-YE', 'bg-BG', 'cs-CZ', 'da-DK', 'de-DE', 'en-AU', 'en-GB', 'en-HK', 'en-IN', 'en-NZ', 'en-US', 'en-ZA', 'en-ZM', 'es-ES', 'fr-FR', 'hu-HU', 'it-IT', 'nb-NO', 'nl-NL', 'nn-NO', 'pl-PL', 'pt-BR', 'pt-PT', 'ru-RU', 'sl-SI', 'sr-RS', 'sr-RS@latin', 'sv-SE', 'tr-TR', 'uk-UA']`. Locale list is `validator.isFloatLocales`.
**isFQDN(str [, options])** | check if the string is a fully qualified domain name (e.g. domain.com).<br/><br/>`options` is an object which defaults to `{ require_tld: true, allow_underscores: false, allow_trailing_dot: false }`.
**isFQDN(str [, options])** | check if the string is a fully qualified domain name (e.g. domain.com).<br/><br/>`options` is an object which defaults to `{ require_tld: true, allow_underscores: false, allow_trailing_dot: false , allow_numeric_tld: false }`.
**isFullWidth(str)** | check if the string contains any full-width chars.
**isHalfWidth(str)** | check if the string contains any half-width chars.
**isHash(str, algorithm)** | check if the string is a hash of type algorithm.<br/><br/>Algorithm is one of `['md4', 'md5', 'sha1', 'sha256', 'sha384', 'sha512', 'ripemd128', 'ripemd160', 'tiger128', 'tiger160', 'tiger192', 'crc32', 'crc32b']`
Expand Down
4 changes: 4 additions & 0 deletions src/lib/isFQDN.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const default_fqdn_options = {
require_tld: true,
allow_underscores: false,
allow_trailing_dot: false,
allow_numeric_tld: false,
};

export default function isFQDN(str, options) {
Expand Down Expand Up @@ -33,6 +34,9 @@ export default function isFQDN(str, options) {
}
for (let part, i = 0; i < parts.length; i++) {
part = parts[i];
if (!options.allow_numeric_tld && i === parts.length - 1 && /^\d+$/.test(part)) {
return false; // reject numeric TLDs
}
if (options.allow_underscores) {
part = part.replace(/_/g, '');
}
Expand Down
29 changes: 29 additions & 0 deletions test/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,9 @@ describe('Validators', () => {
's!ome.com',
'domain.com/',
'/more.com',
'example.0',
'192.168.0.9999',
'192.168.0',
],
});
});
Expand All @@ -828,6 +831,32 @@ describe('Validators', () => {
],
});
});
it('should invalidate FQDN when not require_tld', () => {
test({
validator: 'isFQDN',
args: [
{ require_tld: false },
],
invalid: [
'example.0',
'192.168.0',
'192.168.0.9999',
],
});
});
it('should validate FQDN when not require_tld but allow_numeric_tld', () => {
test({
validator: 'isFQDN',
args: [
{ allow_numeric_tld: true, require_tld: false },
],
valid: [
'example.0',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would example.0 be a valid FQDN? 🤔 And even .9999, is there anything I'm not understanding...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @profnandaa ! Thanks for the quick response.

Why would example.0 be a valid FQDN? And even .9999, is there anything I'm not understanding...

In this test case, allow_numeric_tld is true and require_tld is false, this mimics the current behavior of isFQDN when require_tld is false. In that way, isFQDN will allow numeric TLDs.

I think this solution make sense and doesn't complicate the implementation of isFQDN.

Other options?

The RFC 3696 mentions two ways to validate the TLD:

One is by polling the list published by IANA DomainList and validate against it. However, based on the comment in #704:

Ideally the validator would check TLDs against a list of known values, however the addition of gTLDs mean that the list would be long and ever-growing. I'm not interested in bloating the library or keeping the list up to date.

Since we don't want to keep that list up today, the second option is to validate that the TLD name is not all-numeric.

Does it make sense?

Note:

I think we could avoid the parameter allow_numeric_tld and always require that the TLD is not all-numeric. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Really sorry for the long delay on this one, can't believe it has almost been a year!

/cc. @tux-tn @ezkemboi -- could you please have a look at this and advise.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to me @profnandaa.
After checking on comments and some research online, I agree with the changes.

'192.168.0',
'192.168.0.9999',
],
});
});

it('should validate alpha strings', () => {
test({
Expand Down