luci-mod-network: fix DNS forward validation rejecting interface-only spec#8746
luci-mod-network: fix DNS forward validation rejecting interface-only spec#8746maxskokov wants to merge 1 commit into
Conversation
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit. The regex change correctly fixes the primary reported case (server=1.2.3.4@eth0), and the commit message accurately describes the diff. One edge-case observation left inline regarding interface names that collide with the source-IP character class.
Generated by Claude Code
| // ipaddr%scopeid#srvport[@source[@interface]]#srcport or ipaddr%scopeid#srvport@interface | ||
|
|
||
| m = m[2].match(/^([0-9a-f:.]+)(?:%[^#@]+)?(?:#(\d+))?(?:@([0-9a-f:.]+)(?:@[^#]+)?(?:#(\d+))?)?$/); | ||
| m = m[2].match(/^([0-9a-f:.]+)(?:%[^#@]+)?(?:#(\d+))?(?:@([0-9a-f:.]+)(?:@[^#]+)?(?:#(\d+))?|@[^#@0-9][^#@]*)?$/); |
There was a problem hiding this comment.
The new interface-only alternative is only reached when the first alternative @([0-9a-f:.]+)... fails to match. Because that source-IP branch accepts any string of [0-9a-f:.], an interface name composed solely of hex characters is still captured into the source-IP group (m[3]) rather than falling through to the new @[^#@0-9][^#@]* branch. For example server=1.2.3.4@ace parses with m[3]="ace", then the source-address check at dns.js:74-84`` runs parseIPv4("ace"), which fails and still rejects the spec with "valid IPv4 address". Common names (`wan`, `lan`, `eth0`, `br-lan`) contain non-hex letters and are unaffected, so this is an edge case, but it's the same class of false rejection this PR sets out to fix. Is leaving pure-hex interface names unsupported intentional?
nit: an interface name starting with a digit (e.g. 1.2.3.4@2eth) also fails to match, since the new branch's first char class [^#@0-9] excludes digits.
Generated by Claude Code
There was a problem hiding this comment.
Good catch. Fixed by also allowing m[3] to be a non-IP string (interface name): if it parses as neither IPv4 nor IPv6, the source-address check is skipped and the value is treated as an interface name. Pure-hex interface names like ace are now handled correctly.
… spec dnsmasq supports server=1.2.3.4@eth0 (server IP bound to interface without source IP), but the regex in validateServerSpec() required a source IP address before the interface name, causing valid entries to fail with 'Expecting: valid IP address'. Allow @interface without preceding @source in the server spec. Fixes openwrt#7888. Signed-off-by: Maxim Skokov <skokov.m020709@gmail.com>
maxskokov
force-pushed
the
fix-dns-forward-interface-validation
branch
from
f6ada0e to
87c4efd
Compare
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit; no new issues found.
The follow-up change (adding !validation.parseIPv6(m[3]) to both source-address checks) correctly resolves the pure-hex interface-name case raised previously, since parseIPv6 accepts hex-only strings and so the source check is now skipped for non-IP m[3] values. The previously noted nit about interface names beginning with a digit (e.g. 1.2.3.4@2eth, excluded by the new branch's leading [^#@0-9] class) still applies but was already raised and is left to your discretion.
Generated by Claude Code
2 participants
Pull request details
Description
dnsmasq supports server=1.2.3.4@eth0 (server IP bound to interface without source IP), but the regex in validateServerSpec() required a source IP address before the interface name, causing valid entries to fail with 'Expecting: valid IP address'.
Allow @interface without preceding @source in the server spec.
Fixes #7888.
Screenshot or video of changes (if applicable)
Maintainer (preferred)
@jow-
Tested on
Verified by code inspection against dnsmasq documentation.
Checklist
Signed-off-by: <my@email.address>row (viagit commit --signoff).<package name>: titlefirst line subject for packages.