Skip to content

Fixes #39287 - validate domain name format#10977

Open
Alleny244 wants to merge 1 commit intotheforeman:developfrom
Alleny244:feature/domain-name-host-regexp-validation
Open

Fixes #39287 - validate domain name format#10977
Alleny244 wants to merge 1 commit intotheforeman:developfrom
Alleny244:feature/domain-name-host-regexp-validation

Conversation

@Alleny244
Copy link
Copy Markdown

Why

Domain names were validated only for presence/uniqueness in the Domain model.
This allowed invalid characters and malformed values to be accepted.

What changed

  • Added format validation for Domain#name using Net::Validations::HOST_REGEXP
  • Added domain-specific validation message constant:
    • Net::Validations::DOMAIN_NAME_FORMAT_ERR_MSG
  • Extended domain_test with:
    • accepted example (example.com)
    • rejected examples (Example.com, example_domain.com)
    • additional invalid cases (*, *.*, *.example.com, example.*.com, !!!, example..com, etc.)
  • Normalized test fixtures/inputs to lowercase where needed

Test plan

Steps

  • bundle exec rails test test/models/domain_test.rb

Expected result

  • Domain format validation rejects invalid names
  • Existing domain behavior remains unchanged for valid lowercase dotted names

@Alleny244 Alleny244 force-pushed the feature/domain-name-host-regexp-validation branch from f0c6993 to 933b749 Compare May 6, 2026 08:16
Copy link
Copy Markdown
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

One small comment

Comment thread app/models/domain.rb
include ParameterSearch
validates :name, :presence => true, :uniqueness => true
validates :name, :presence => true, :uniqueness => true,
:format => {:with => Net::Validations::HOST_REGEXP, :message => N_(Net::Validations::DOMAIN_NAME_FORMAT_ERR_MSG)}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

DOMAIN_NAME_FORMAT_ERR_MSG is already defined as N_(...) in validations.rb:13. Wrapping it again with N_() is redundant.

Suggested change
:format => {:with => Net::Validations::HOST_REGEXP, :message => N_(Net::Validations::DOMAIN_NAME_FORMAT_ERR_MSG)}
:format => {:with => Net::Validations::HOST_REGEXP, :message => Net::Validations::DOMAIN_NAME_FORMAT_ERR_MSG}

Copy link
Copy Markdown
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

What about data in the database that is already invalid? Have you considered how users deal with that?

MASK_REGEXP ||= /\A((255.){3}(0|128|192|224|240|248|252|254))|((255.){2}(0|128|192|224|240|248|252|254).0)|(255.(0|128|192|224|240|248|252|254)(.0){2})|((128|192|224|240|248|252|254)(.0){3})\z/

HOST_REGEXP_ERR_MSG = N_("hostname can contain only lowercase letters, numbers, dashes and dots according to RFC921, RFC952 and RFC1123")
DOMAIN_NAME_FORMAT_ERR_MSG = N_("domain name can contain only lowercase letters, numbers, dashes and dots according to RFC921, RFC952 and RFC1123")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Domain names are case insensitive. It confuses me a bit why HOST_REGEXP does enforce lower case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants