Skip to content

feat(config): support multiple static IP addresses - #36

Merged
olicesx merged 4 commits into
olicesx:mainfrom
JohnsonRan:feat/static-ip-multiple-addresses
Aug 11, 2026
Merged

feat(config): support multiple static IP addresses#36
olicesx merged 4 commits into
olicesx:mainfrom
JohnsonRan:feat/static-ip-multiple-addresses

Conversation

@JohnsonRan

@JohnsonRan JohnsonRan commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • allow static_ip_response.ip to contain comma-separated IPv4 and IPv6 addresses
  • trim each address and validate the entire list atomically
  • return only A records for A queries, only AAAA records for AAAA queries, and both for ANY queries
  • return NOERROR/NODATA for other query types, including HTTPS and SVCB; do not synthesize ipv4hint or ipv6hint
  • share static IP response construction across normal, response-action, and precomputed paths
  • document the syntax and update the config editor placeholder

Example

{ "type": "static_ip_response", "ip": "192.0.2.1,2001:db8::1" }

Behavior

  • A query: returns configured IPv4 addresses
  • AAAA query: returns configured IPv6 addresses
  • ANY query: returns both address families
  • HTTPS/SVCB and other query types: returns NODATA so clients can fall back to A/AAAA resolution
  • any invalid or empty list entry: returns SERVFAIL without partial answers

Scope boundary

This PR controls how a selected static_ip_response action constructs its DNS answer. It does not decide whether an upstream HTTPS/SVCB response belongs to a GeoIP tag and does not inspect ipv4hint/ipv6hint for matching. That response-classification behavior is handled separately by PR #37.

When both PRs are used together, PR #37 can match an upstream HTTPS/SVCB address hint, and this PR then ensures the selected static IP action returns NOERROR/NODATA for the HTTPS/SVCB query instead of synthesizing an invalid service-binding record.

Validation

  • cargo fmt --all -- --check
  • cargo test (110 unit tests, 6 listener tests, 9 DoH integration tests, 10 doctests)
  • cargo clippy --all-targets --all-features -- -D warnings
  • focused HTTPS NODATA regression test
  • git diff --check

Allow static_ip_response to accept comma-separated IPv4 and IPv6 addresses. Validate the list atomically and share response construction across normal, response-action, and precomputed paths.
@olicesx

olicesx commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Review Conclusion: Ready to Merge (No Blockers)

Core mechanisms verified through dual-channel validation (direct code reading + independent agent cross-check).

✅ Verified

  • Cache isolation, no pollution: rule cache (calculate_rule_hash includes qtype + RuleCacheEntry::matches checks qtype) and DNS response cache (dedupe_hash includes qtype + CacheEntry stores qtype + read-side double check) are triple-isolated; HTTPS→NODATA won't pollute A/AAAA caches
  • qtype semantics correct at all three call sites: pipeline request.qtype / rules ctx.qtype / advanced_rule parameter are all the request's query type
  • Inline logic unified in pipeline.rs: the inline A/AAAA construction in apply_rules has been replaced with the shared make_static_ip_answer; all three paths now behave consistently
  • Atomic validation / empty entry handling: any invalid entry (including ones of an irrelevant address family) → SERVFAIL without partial answers; empty string / trailing comma / consecutive commas all covered by tests
  • Syntax consistent with existing convention: comma-separated matches the existing multi-value convention of Forward.upstream (address-type uses comma-separated, text-type uses array)

🟢 Notes (non-blocking)

  1. Consider adding a response-action path test: the StaticIpResponse branch in apply_response_actions has no dedicated test (neither on-match nor on-miss covered). Low risk since it shares the same function, but ctx.qtype is a boundary unique to this path.
  2. Optional alignment: Forward.upstream accepts an array at deserialization (deserialize_upstream untagged). ip could reuse that pattern to also accept ["1.2.3.4","2001:db8::1"]. Backward compatible, not required.
  3. UDP size boundary: with many configured addresses the response may exceed 512/1232 bytes with no TC truncation. Won't trigger with normal configs.

❓ Question (needs project decision, non-blocking)

Should NODATA include an SOA authority record? HTTPS/SVCB → NOERROR + empty answer, no SOA (RFC 2308 §2.2 SHOULD). However, all static responses in the project (StaticResponse/TXT/IP) currently omit SOA by design; adding SOA would change existing behavior and needs a project decision.

📌 Pre-existing issue (unrelated to this PR, suggest filing separately)

dns_cache's dedupe_hash does not include client_ip: when a rule uses a client_ip matcher, static decision cache entries can be shared across clients (rule_cache has uses_client_ip isolation, dns_cache does not).

@olicesx
olicesx merged commit d4ce74f into olicesx:main Aug 11, 2026
10 checks passed
@JohnsonRan
JohnsonRan deleted the feat/static-ip-multiple-addresses branch August 11, 2026 17:08
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.

2 participants