🐛 bug: reject malformed hostnames in hostauthorization wildcard path#4408
🐛 bug: reject malformed hostnames in hostauthorization wildcard path#4408gaby wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughA new isValidHostSyntax helper validates normalized hosts (rejecting empty, leading/trailing dots, invalid labels; accepting IP literals). parseNormalizedAuthority calls this validator before accepting a host. Tests now assert rejection for hosts containing '/', '?', or whitespace. ChangesHost Syntax Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a host syntax validation helper isValidHostSyntax to the host authorization middleware to prevent invalid hosts (such as those containing path or query delimiters, or spaces) from being accepted, along with corresponding unit tests. A review comment suggests optimizing isValidHostSyntax to avoid heap allocations and iterator overhead on the hot path by validating the host in a single pass.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| func isValidHostSyntax(host string) bool { | ||
| if host == "" { | ||
| return false | ||
| } | ||
|
|
||
| // IPv4 / IPv6 literals. | ||
| if net.ParseIP(host) != nil { | ||
| return true | ||
| } | ||
|
|
||
| if strings.HasPrefix(host, ".") || strings.HasSuffix(host, ".") { | ||
| return false | ||
| } | ||
|
|
||
| for label := range strings.SplitSeq(host, ".") { | ||
| if label == "" || len(label) > maxLabelLength { | ||
| return false | ||
| } | ||
| if label[0] == '-' || label[len(label)-1] == '-' { | ||
| return false | ||
| } | ||
| for i := 0; i < len(label); i++ { | ||
| ch := label[i] | ||
| if (ch >= 'a' && ch <= 'z') || (ch >= '0' && ch <= '9') || ch == '-' { | ||
| continue | ||
| } | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| return true | ||
| } |
There was a problem hiding this comment.
The current implementation of isValidHostSyntax uses strings.SplitSeq to split the host into labels. Since this middleware runs on the hot path of every incoming HTTP request, avoiding heap allocations and iterator overhead is crucial for performance. We can validate the host syntax in a single pass with zero allocations by iterating over the string characters directly.
func isValidHostSyntax(host string) bool {
if host == "" {
return false
}
// IPv4 / IPv6 literals.
if net.ParseIP(host) != nil {
return true
}
if strings.HasPrefix(host, ".") || strings.HasSuffix(host, ".") {
return false
}
labelLen := 0
for i := 0; i < len(host); i++ {
ch := host[i]
if ch == '.' {
if labelLen == 0 || labelLen > maxLabelLength {
return false
}
if host[i-labelLen] == '-' || host[i-1] == '-' {
return false
}
labelLen = 0
continue
}
if (ch >= 'a' && ch <= 'z') || (ch >= '0' && ch <= '9') || ch == '-' {
labelLen++
continue
}
return false
}
if labelLen == 0 || labelLen > maxLabelLength {
return false
}
if host[len(host)-labelLen] == '-' || host[len(host)-1] == '-' {
return false
}
return true
}2 participants
Motivation
attacker.test/.example.com).strings.HasSuffixover insufficiently validated host input.Description
isValidHostSyntax(host string) booltomiddleware/hostauthorization/hostauthorization.goto validate normalized host syntax (accept IP literals or RFC-style DNS labels and reject delimiters/whitespace, empty labels, labels with leading/trailing hyphens, and oversize labels).isValidHostSyntaxfromparseNormalizedAuthorityto reject malformed hosts early beforematchHostruns.Test_ParseNormalizedAuthorityinmiddleware/hostauthorization/hostauthorization_test.gofor inputs containing/,?, and spaces to ensure these are rejected.Testing
make auditwhich executed but failed due togovulncheckreporting multiple Go stdlib vulnerabilities in the pinned toolchain (go1.25.1), causing a non-zero exit (scan output reported numerous standard-library issues). (failed)make generate; the generation step completed successfully. (passed)make betteralign; completed successfully. (passed)make modernize; failed in this environment because themodernizetool requires Go >= 1.26 while the CI/toolchain usesgo1.25.1. (failed)make format; completed successfully. (passed)make lint; linter passed with0 issuesafter a minor gocritic suggestion was addressed. (passed)make test; full test suite executed successfully (3417 tests, 2 skipped). (passed)Codex Task