Skip to content
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

[Validator] Added HostnameValidator #31518

Open
wants to merge 1 commit into
base: 4.4
from

Conversation

Projects
None yet
6 participants
@karser
Copy link
Contributor

commented May 16, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #10088
License MIT
Doc PR symfony/symfony-docs#...

This PR adds HostnameValidator support. I encountered this need in my project and was surprised that this issue has been open for years.

Here is short example:

App\Entity\Acme:
    properties:
        domain:
            - Hostname: ~
        non_tld_domain:
            - Hostname: { requireTld: false }

The option requireTld is true by default and disallows domains like localhost and etc.

@karser karser force-pushed the karser:domain-validator branch from d4b255a to 8e0b725 May 16, 2019

@karser karser changed the title Added DomainValidator [Validator] Added DomainValidator May 16, 2019

@nicolas-grekas nicolas-grekas added this to the next milestone May 20, 2019

@nicolas-grekas nicolas-grekas changed the base branch from master to 4.4 Jun 2, 2019

@javiereguiluz

This comment has been minimized.

Copy link
Member

commented Jul 3, 2019

@karser thanks for this contribution! I like this a lot ... except for the tld option, which is confusing to me.

Here's a proposal:

  • Rename tld to require_tld
  • Set it as true by default
  • If true, domains like localhost are not valid. Set it to false to allow them.

@karser karser force-pushed the karser:domain-validator branch from 8e0b725 to dc1db29 Jul 3, 2019

@karser

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2019

@javiereguiluz thanks for the feedback. I implemented your proposal (see the test cases). wdyt?

App\Entity\Acme:
    properties:
        domain:
            - Domain: ~
        non_tld_domain:
            - Domain: { requireTld: false }

The option requireTld is true by default and disallows domains like localhost and etc.

@karser karser force-pushed the karser:domain-validator branch from dc1db29 to 8101e63 Jul 3, 2019

@karser karser force-pushed the karser:domain-validator branch from 8101e63 to 46da469 Jul 3, 2019

@karser karser force-pushed the karser:domain-validator branch 2 times, most recently from f650936 to 98f6a2e Jul 4, 2019

@karser karser force-pushed the karser:domain-validator branch 2 times, most recently from 69d879a to 5bdd6c3 Jul 4, 2019

@ro0NL
Copy link
Contributor

left a comment

would DomainName be a more inuitive name?

also please update validation.en.xml + any language you know :)

@karser karser force-pushed the karser:domain-validator branch 2 times, most recently from d930263 to 13d3e65 Jul 15, 2019

@karser karser requested review from dunglas, sroze and xabbuh as code owners Jul 15, 2019

@karser karser force-pushed the karser:domain-validator branch from 13d3e65 to d930263 Jul 15, 2019

@karser karser force-pushed the karser:domain-validator branch from d930263 to 365a1f8 Jul 15, 2019

@karser karser changed the title [Validator] Added DomainValidator [Validator] Added HostnameValidator Jul 15, 2019

@karser karser force-pushed the karser:domain-validator branch from 365a1f8 to 3cb731a Jul 15, 2019

@karser

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2019

Regarding having underscore in the hostname - we can add option requireRfc which will be adding FILTER_FLAG_HOSTNAME option in filter_var.

FILTER_FLAG_HOSTNAME adds RFC1035 validation: They must start with a letter, end with a letter or digit, and have as interior characters only letters, digits, and hyphen

filter_var('goog_le.com', FILTER_VALIDATE_DOMAIN); // goog_le.com
filter_var('goog_le.com', FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME); // false
filter_var('qq--.com', FILTER_VALIDATE_DOMAIN); // qq--.com
filter_var('qq--.com', FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME); //false

@karser karser force-pushed the karser:domain-validator branch from 3cb731a to 5976482 Jul 15, 2019

@ro0NL

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2019

im 👍 for a pure Hostname validator, not sure any "domain name" is the actual requirement :/ but @elnur may clarify on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.