Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c822a14af6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let (Some(_fallback), Some(_fallback_domain_filters), Some(policy)) = | ||
| (&self.fallback, &self.fallback_domain_filters, &self.policy) | ||
| && let Some(host) = Self::domain_name_of_message(message) |
There was a problem hiding this comment.
match_policy currently returns a policy hit only when fallback, fallback_domain_filters, and policy are all present, which makes nameserver_policy effectively inert in common configs (for example, when fallback is unset or fallback-filter.domain is empty). In those cases policy rules are silently ignored and domains are resolved through the default path, which is a functional regression for policy-based DNS routing.
Useful? React with 👍 / 👎.
| if let Some(lru) = &self.lru_cache | ||
| && let Some(cached) = lru.read().await.get(query, Instant::now()) | ||
| { | ||
| if !message.recursion_desired() { |
There was a problem hiding this comment.
The cache read path is guarded by if !message.recursion_desired(), but standard client DNS queries set RD=true, so cached entries are skipped for normal traffic even when present. This turns the LRU into write-mostly state, increases upstream query load, and defeats the expected latency/performance benefit of the resolver cache.
Useful? React with 👍 / 👎.
| if server.contains(':') && !server.starts_with('[') { | ||
| server = format!("udp://[{server}]"); | ||
| } else { |
There was a problem hiding this comment.
When a nameserver is provided without scheme but with a colon (e.g. 8.8.8.8:5353), this branch wraps it as udp://[8.8.8.8:5353], which is invalid bracket syntax (brackets are for IPv6 literals). As a result, valid Clash-style IPv4+port entries are rejected during config parsing and DNS startup fails for those configurations.
Useful? React with 👍 / 👎.
MFSGA
force-pushed
the
master
branch
2 times, most recently
from
824da7d to
7bb2f2e
Compare
1 participant
@codex review this.