feat: add DNS domain rewrite setting#122
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds DNS hosts rewrite storage, parsing, sing-box config emission, settings UI wiring, localized guidance text, and parser tests. ChangesDNS Hosts Rewrite
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPreferenceFragment
participant DataStore
participant ConfigBuilder
participant SingBoxConfig
User->>SettingsPreferenceFragment: Enter DNS hosts text
SettingsPreferenceFragment->>SettingsPreferenceFragment: Normalize tabs and sanitize input
SettingsPreferenceFragment->>DataStore: Save dnsHosts
ConfigBuilder->>DataStore: Read dnsHosts
ConfigBuilder->>ConfigBuilder: parseDnsHosts(text)
ConfigBuilder->>SingBoxConfig: Add hosts server + DNS rule
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/io/nekohasekai/sagernet/ui/SettingsPreferenceFragment.kt (1)
269-307: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate sanitize/reload logic vs
dnsReloadListener.This inline listener re-implements the same "sanitize → compare → set text/needReload → return true/false" pattern already encapsulated in
dnsReloadListener(lines 49-59), differing only by the tab→space pre-processing. Consider parameterizingdnsReloadListenerwith an optional pre-processing step so both call sites share one implementation.♻️ Proposed refactor
- private fun dnsReloadListener(preference: EditTextPreference, newValue: Any?): Boolean { + private fun dnsReloadListener( + preference: EditTextPreference, + newValue: Any?, + preprocess: (String) -> String = { it }, + ): Boolean { val rawValue = newValue as? String ?: return reloadListener.onPreferenceChange(preference, newValue) - val sanitizedValue = sanitizeDnsPreferenceValue(rawValue) + val sanitizedValue = sanitizeDnsPreferenceValue(preprocess(rawValue)) if (sanitizedValue != rawValue) { preference.text = sanitizedValue needReload() return false } needReload() return true } ... - dnsHosts.onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, newValue -> - val rawValue = newValue as? String - if (rawValue == null) { - reloadListener.onPreferenceChange(dnsHosts, newValue) - } else { - // Tabs are valid separators in pasted hosts entries; convert them to - // spaces first so the control-character sanitization does not merge - // the domain and address tokens together. - val sanitized = sanitizeDnsPreferenceValue(rawValue.replace('\t', ' ')) - if (sanitized != rawValue) { - dnsHosts.text = sanitized - needReload() - false - } else { - needReload() - true - } - } - } + dnsHosts.onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, newValue -> + dnsReloadListener(dnsHosts, newValue) { it.replace('\t', ' ') } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/io/nekohasekai/sagernet/ui/SettingsPreferenceFragment.kt` around lines 269 - 307, The DNS preference change handler is duplicating the same sanitize/reload flow already implemented in dnsReloadListener. Refactor dnsReloadListener to accept an optional pre-processing step for the raw string (so the tab-to-space normalization can be reused), then have dnsHosts.onPreferenceChangeListener delegate to that shared logic instead of reimplementing the sanitize → compare → update text → needReload pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/src/main/java/io/nekohasekai/sagernet/ui/SettingsPreferenceFragment.kt`:
- Around line 269-307: The DNS preference change handler is duplicating the same
sanitize/reload flow already implemented in dnsReloadListener. Refactor
dnsReloadListener to accept an optional pre-processing step for the raw string
(so the tab-to-space normalization can be reused), then have
dnsHosts.onPreferenceChangeListener delegate to that shared logic instead of
reimplementing the sanitize → compare → update text → needReload pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bf1b2888-b313-4d03-bc72-ff26f60d08dc
📒 Files selected for processing (12)
app/src/main/java/io/nekohasekai/sagernet/Constants.ktapp/src/main/java/io/nekohasekai/sagernet/database/DataStore.ktapp/src/main/java/io/nekohasekai/sagernet/fmt/ConfigBuilder.ktapp/src/main/java/io/nekohasekai/sagernet/ui/SettingsPreferenceFragment.ktapp/src/main/res/values-fa/strings.xmlapp/src/main/res/values-ja/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xmlapp/src/main/res/xml/global_preferences.xml
hawkff
force-pushed
the
feat/dns-hosts-rewrite
branch
from
399b3bd to
f9293db
Compare
- Reject IPv6 forms with empty hextet groups that pass the app regex but fail strict address parsing downstream (leading/trailing single colon, repeated or overlapping double-colon runs). - Convert internationalized domains to punycode and allow underscore labels instead of silently dropping such entries. - Treat NBSP as a token separator for pasted content. - Exclude comment lines from the settings line-count summary. - Add unit tests covering accepted and rejected hosts entries.
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
1 participant
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.
Summary
Validation
git diff --checkcoderabbit review --agent --base main(0 findings)Greptile Summary
This PR introduces a global DNS domain-rewrite setting: users enter
domain ip [ip …]entries in a new multiline preference, which are parsed into a sing-boxhosts-type DNS server and a scoped rule that intercepts A/AAAA queries for those exact domains before FakeDNS or remote DNS can handle them.ConfigBuilder.kt):parseDnsHostsvalidates each entry with strict IPv4 (rejects leading-zero octets) and IPv6 (rejects Go-netip-invalid forms) checks, full RFC label validation, NBSP/tab separator handling, IDN→punycode normalisation, and comment stripping — covered by a newDnsHostsParseTestsuite.SettingsPreferenceFragment.kt): a multilineEditTextPreferencewith a plural-aware line-count summary (getQuantityString), tab-to-space preprocessing, andTYPE_TEXT_FLAG_NO_SUGGESTIONS; all six locale files receive thedns_hosts_linesplural resource andnot_setstring.Confidence Score: 5/5
Safe to merge — input validation is thorough, rule ordering is correct, and the change is well-tested.
The parsing layer rejects every class of malformed input (leading-zero IPv4, Go-netip-invalid IPv6, empty labels, IP-as-domain, oversized labels) before anything reaches the sing-box config. DNS rule insertion relies on later index-0 insertions pushing the hosts rule to the right position, which is correctly described in code comments and matches the actual build flow. The UI layer handles tabs before ISO-control sanitization in the right order, and locale files all have the new plural and not_set resources. No gaps found that would cause a config load failure or unexpected DNS behaviour.
No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant App as App (SettingsPreferenceFragment) participant DS as DataStore (dnsHosts) participant CB as ConfigBuilder (buildConfig) participant SB as sing-box DNS engine App->>App: User saves hosts entry (tab→space, sanitize) App->>DS: "dnsHosts = sanitizedValue" CB->>DS: parseDnsHosts(DataStore.dnsHosts) Note over CB: Validate each domain (RFC labels,<br/>IDN→punycode, no IPs)<br/>Validate each address (strict IPv4/IPv6) CB->>SB: "Add DNS server {type:hosts, predefined:{domain→[IPs]}}" CB->>SB: "Insert DNS rule at index 0<br/>{domain:[full:x], qtype:[A,AAAA], server:dns-hosts}" Note over SB: Final rule priority order:<br/>1. per-subscription rules<br/>2. force-bypass rules<br/>3. loopback rule<br/>4. dnsHosts rule (new)<br/>5. user DNS routing rules<br/>6. FakeDNS rule SB-->>App: "A/AAAA for matched domains → static IP(s)<br/>Other qtype/unmatched → normal resolution"%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant App as App (SettingsPreferenceFragment) participant DS as DataStore (dnsHosts) participant CB as ConfigBuilder (buildConfig) participant SB as sing-box DNS engine App->>App: User saves hosts entry (tab→space, sanitize) App->>DS: "dnsHosts = sanitizedValue" CB->>DS: parseDnsHosts(DataStore.dnsHosts) Note over CB: Validate each domain (RFC labels,<br/>IDN→punycode, no IPs)<br/>Validate each address (strict IPv4/IPv6) CB->>SB: "Add DNS server {type:hosts, predefined:{domain→[IPs]}}" CB->>SB: "Insert DNS rule at index 0<br/>{domain:[full:x], qtype:[A,AAAA], server:dns-hosts}" Note over SB: Final rule priority order:<br/>1. per-subscription rules<br/>2. force-bypass rules<br/>3. loopback rule<br/>4. dnsHosts rule (new)<br/>5. user DNS routing rules<br/>6. FakeDNS rule SB-->>App: "A/AAAA for matched domains → static IP(s)<br/>Other qtype/unmatched → normal resolution"Reviews (3): Last reviewed commit: "fix(dns): harden hosts parsing and refin..." | Re-trigger Greptile