Fix and rename the host name validators - #3855
Open
DL6ER wants to merge 2 commits into
Open
Conversation
The regex was unanchored: `/[^<>;"]/` matched on the first character that was not one of those four, so `evil<script>` passed. Output is escaped on render anyway. Its only caller is the client field in group management, which takes a host name or an interface like `:eth0`. FTL matches both verbatim and only records names of `[A-Za-z0-9._-]`, so we check exactly that. Signed-off-by: DL6ER <dl6er@dl6er.de>
There was a problem hiding this comment.
Pull request overview
This PR fixes a misleading client “hostname” validator in the web UI by replacing the ineffective validateHostname() check with a stricter, correctly-anchored validator that matches the actual client identifiers accepted by FTL (hostname-like strings and :interface).
Changes:
- Replace
validateHostname()withvalidateClientPattern()using an anchored regex (^:?[\w.-]+$) to reject inputs that can never match an FTL client identifier. - Update group client entry validation to call the new function and adjust the user-facing warning text accordingly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
scripts/js/utils.js |
Replaces the ineffective validateHostname() helper with validateClientPattern() and exports it via globalThis.utils. |
scripts/js/groups-clients.js |
Switches client input validation to utils.validateClientPattern() and updates the inline comment + warning message. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
validateHostname() -> validateClientPattern()
yubiuser
previously approved these changes
Aug 22, 2026
The name only made sense next to a non-strict counterpart, and that one is gone. Its comment claimed to reject characters invalid in DNS names, which underscores are not - say what the regex actually does instead. Signed-off-by: DL6ER <dl6er@dl6er.de>
DL6ER
force-pushed
the
tweak/client-pattern-validator
branch
from
August 22, 2026 05:49
f2d0687 to
0c3435d
Compare
rdwebdesign
approved these changes
Aug 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you for your contribution to the Pi-hole Community!
Please read the comments below to help us consider your Pull Request.
We are all volunteers and completing the process outlined will help us review your commits quicker.
Please make sure you
What does this PR aim to accomplish?:
validateHostname()validates nothing./[^<>;"]/is unanchored, so it matches on the first character that is not one of those four -evil<script>passes. Only the empty string is ever rejected.Noticed while reviewing #3853.
How does this PR accomplish the above?:
Its only caller is the client field in group management, which after the IP/CIDR and MAC branches takes a host name or an interface like
:eth0. FTL matches both verbatim and only ever records names of[A-Za-z0-9._-](valid_hostname()), so the check is now/^:?[\w.-]+$/uand the function is named after what it does.The reserved characters are not lost: the table escapes on render.
validateHostnameStrict()then drops a suffix that no longer tells it apart from anything.Please merge this after #3853. The two touch neighboring lines, and I would rather resolve that here than push it onto the contributor.
Link documentation PRs if any are needed to support this PR:
None needed.
By submitting this pull request, I confirm the following:
git rebase)