fix: reject invalid dotted-decimal in hostname_rfc1123 validation#1562
fix: reject invalid dotted-decimal in hostname_rfc1123 validation#1562Herrtian wants to merge 1 commit into
Conversation
RFC 1123 states that a valid hostname can never have the dotted-decimal form #.#.#.# since the highest-level label will be alphabetic. Strings like "277.168.0.1" were incorrectly accepted because the regex allows all-numeric labels. Now hostname_rfc1123 rejects dotted-decimal strings that are not valid IPv4 addresses. Valid IPv4 addresses like "192.168.0.1" continue to pass since they are legitimate host identifiers. Fixes go-playground#1561
There was a problem hiding this comment.
Pull request overview
This PR updates the hostname_rfc1123 validator to reject “dotted-decimal-looking” hostnames that aren’t valid IP addresses, addressing cases like 277.168.0.1 that previously passed the RFC1123 hostname regex.
Changes:
- Added a
looksLikeDottedDecimalhelper to detect digit-and-dot-only inputs. - Updated
isHostnameRFC1123to run an additional dotted-decimal/IP parse check after the existing RFC1123 regex match.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // looksLikeDottedDecimal returns true if s looks like a dotted-decimal address | ||
| // (e.g. "277.168.0.1") — composed entirely of digits and dots, with at least one dot. | ||
| func looksLikeDottedDecimal(s string) bool { | ||
| hasDot := false | ||
| for _, c := range s { | ||
| if c == '.' { | ||
| hasDot = true | ||
| } else if c < '0' || c > '9' { | ||
| return false | ||
| } | ||
| } | ||
| return hasDot |
There was a problem hiding this comment.
looksLikeDottedDecimal returns true for any string containing only digits and dots with at least one dot (e.g. "1.2" or "1.2.3"). That means isHostnameRFC1123 will now reject these values even though hostnameRegexRFC1123 allows them, and the RFC 1123 quote/comment here specifically refers to the IPv4-like form #.#.#.#. Consider tightening the check to only treat the input as “dotted-decimal” when it has exactly 4 numeric labels (3 dots, no empty labels), so you only gate IPv4-shaped inputs behind net.ParseIP.
| // looksLikeDottedDecimal returns true if s looks like a dotted-decimal address | |
| // (e.g. "277.168.0.1") — composed entirely of digits and dots, with at least one dot. | |
| func looksLikeDottedDecimal(s string) bool { | |
| hasDot := false | |
| for _, c := range s { | |
| if c == '.' { | |
| hasDot = true | |
| } else if c < '0' || c > '9' { | |
| return false | |
| } | |
| } | |
| return hasDot | |
| // looksLikeDottedDecimal returns true if s looks like a dotted-decimal IPv4 | |
| // address shape (e.g. "277.168.0.1"): exactly 4 numeric labels separated by | |
| // 3 dots, with no empty labels. | |
| func looksLikeDottedDecimal(s string) bool { | |
| parts := strings.Split(s, ".") | |
| if len(parts) != 4 { | |
| return false | |
| } | |
| for _, part := range parts { | |
| if part == "" { | |
| return false | |
| } | |
| for _, c := range part { | |
| if c < '0' || c > '9' { | |
| return false | |
| } | |
| } | |
| } | |
| return true |
| // form #.#.#.#, since at least the highest-level component label will | ||
| // be alphabetic." Reject strings that look like dotted-decimal but are | ||
| // not valid IPv4 addresses (e.g. 277.168.0.1). | ||
| if net.ParseIP(val) == nil && looksLikeDottedDecimal(val) { |
There was a problem hiding this comment.
In isHostnameRFC1123, net.ParseIP(val) is evaluated for every value that matches the hostname regex, including typical hostnames like "example.com". To avoid unnecessary parsing work, check looksLikeDottedDecimal(val) first and only call net.ParseIP when the string is actually numeric/dotted.
| if net.ParseIP(val) == nil && looksLikeDottedDecimal(val) { | |
| if looksLikeDottedDecimal(val) && net.ParseIP(val) == nil { |
zemzale
left a comment
zemzale
left a comment
There was a problem hiding this comment.
Thanks for the PR.
As Copilot points out:
- the
looksLikeDottedDecimalhas a huge bug net.ParseIPgets called every time which is not needed for a LOT of cases
Also I would require some tests before accepting this change, that would cover all the cases that have been pointed out.
Summary
Fixes #1561
hostname_rfc1123validation accepted strings like277.168.0.1which are neither valid IPv4 addresses nor valid hostnames per RFC 1123.RFC 1123 §2.1 states:
Change
Added a check in
isHostnameRFC1123: after the regex match, if the input looks like a dotted-decimal string (digits and dots only, with at least one dot), it must also be a valid IP address vianet.ParseIP. Otherwise it's rejected.277.168.0.1192.168.0.1123.456.789.0example.com1Test plan
go build ./...passesTestHostnameRFC1123ValidationpassesTestHostnameRFC1123AliasValidationpasses