Skip to content

Commit eaa451e

Browse files
committed
test(tun): add Go tests for mobile/tun FakeDNS codec and DNS mapper
Adds 13 table-driven Go tests across two files in mobile/tun, plus an explicit workflow step in go-test.yml so the subpackage's tests are visible in CI output (the existing exclusion-based step reaches it implicitly, but explicit is better for the next reader). mobile/tun/dns_mapper_test.go (4 tests): - GetFakeIP assigns sequential IPs in 198.18.0.0/16 - GetFakeIP returns stable mapping for the same hostname - GetHostname round-trips with GetFakeIP - GetHostname returns ok=false for unmapped IPs mobile/tun/fakedns_proxy_test.go (9 tests): - parseDNSQuery: valid hostname, single-label, too-short (<12B), label length boundary (63 OK / 64 rejected), compression-pointer rejection (0xC0 caught by the length>63 guard). - buildDNSResponse: valid-IP happy path with full byte-layout assertions (flags 0x8400, ANCOUNT=1, name pointer 0xC00C, type A, rdlen 4, IPv4 trailing), invalid-IP returns nil, short query returns nil. Uses only the Go stdlib 'testing' package — no testify, no external deps — so go.mod/go.sum stay clean (the build_go_mobile.sh immutability trap at lines 10-17 requires this). Imports encoding/binary, net, strings, testing only. Also lands the advisor plans/ index (plans/README.md + plan files 001-019) generated by the /improve skill against commit 29a1de2. These are documentation artifacts (no source code touched); they record the audit findings, their priority/effort/risk, and the recommended execution order, so future executors and humans have one place to see what's planned. Verification (run with Go 1.26.1 on Windows): go test -count=1 ./mobile/tun/... -v -> 13 PASS go test -count=1 \ -> exit 0 git diff --quiet -- go.mod go.sum -> exit 0 (immutability invariant preserved)
1 parent 1723605 commit eaa451e

23 files changed

Lines changed: 6291 additions & 0 deletions

.github/workflows/go-test.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,6 @@ jobs:
1919

2020
- name: Run Go tests (exclude root and gomobile bridge)
2121
run: go test $(go list ./... | grep -v -e '^masterdnsvpn-go$' -e '^masterdnsvpn-go/mobile$')
22+
23+
- name: Run mobile/tun tests (explicit, incl. new characterization tests)
24+
run: go test ./mobile/tun/... -v

mobile/tun/dns_mapper_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package tun
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestDNSMapper_GetFakeIP_AssignsSequentialIPsIn198_18Range(t *testing.T) {
8+
d := NewDNSMapper()
9+
10+
first := d.GetFakeIP("a.example")
11+
if first != "198.18.0.2" {
12+
// counter starts at 1; first AddUint32 -> 2 in NewDNSMapper's setup
13+
t.Fatalf("first fake IP = %q, want 198.18.0.2", first)
14+
}
15+
16+
second := d.GetFakeIP("b.example")
17+
if second != "198.18.0.3" {
18+
t.Fatalf("second fake IP = %q, want 198.18.0.3", second)
19+
}
20+
}
21+
22+
func TestDNSMapper_GetFakeIP_StableForSameHostname(t *testing.T) {
23+
d := NewDNSMapper()
24+
25+
first := d.GetFakeIP("dup.example")
26+
second := d.GetFakeIP("dup.example")
27+
if first != second {
28+
t.Fatalf("duplicate hostname mapped to %q then %q", first, second)
29+
}
30+
}
31+
32+
func TestDNSMapper_GetHostname_RoundTrips(t *testing.T) {
33+
d := NewDNSMapper()
34+
35+
const host = "roundtrip.example"
36+
ip := d.GetFakeIP(host)
37+
38+
got, ok := d.GetHostname(ip)
39+
if !ok {
40+
t.Fatalf("GetHostname(%q) returned ok=false", ip)
41+
}
42+
if got != host {
43+
t.Fatalf("GetHostname(%q) = %q, want %q", ip, got, host)
44+
}
45+
}
46+
47+
func TestDNSMapper_GetHostname_UnknownIPOKFalse(t *testing.T) {
48+
d := NewDNSMapper()
49+
if _, ok := d.GetHostname("198.18.99.99"); ok {
50+
t.Fatal("GetHostname returned ok=true for unmapped IP")
51+
}
52+
}

mobile/tun/fakedns_proxy_test.go

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package tun
2+
3+
import (
4+
"encoding/binary"
5+
"net"
6+
"strings"
7+
"testing"
8+
)
9+
10+
// helper: build a DNS query for a hostname like "example.com"
11+
func buildQuery(t *testing.T, hostname string) []byte {
12+
t.Helper()
13+
// Header: 12 bytes (id, flags, qdcount=1, ancount=0, nscount=0, arcount=0)
14+
q := make([]byte, 12)
15+
binary.BigEndian.PutUint16(q[0:2], 0x1234) // id
16+
binary.BigEndian.PutUint16(q[4:6], 1) // qdcount = 1
17+
18+
// Question section
19+
parts := strings.Split(hostname, ".")
20+
for _, label := range parts {
21+
if len(label) > 63 {
22+
t.Fatalf("label %q too long for test builder", label)
23+
}
24+
q = append(q, byte(len(label)))
25+
q = append(q, []byte(label)...)
26+
}
27+
q = append(q, 0) // terminator
28+
q = append(q, 0, 1) // QTYPE = A
29+
q = append(q, 0, 1) // QCLASS = IN
30+
return q
31+
}
32+
33+
func TestParseDNSQuery_ValidHostname(t *testing.T) {
34+
q := buildQuery(t, "example.com")
35+
got := parseDNSQuery(q)
36+
if got != "example.com" {
37+
t.Fatalf("parseDNSQuery = %q, want %q", got, "example.com")
38+
}
39+
}
40+
41+
func TestParseDNSQuery_SingleLabelHostname(t *testing.T) {
42+
q := buildQuery(t, "localhost")
43+
got := parseDNSQuery(q)
44+
if got != "localhost" {
45+
t.Fatalf("parseDNSQuery single label = %q, want %q", got, "localhost")
46+
}
47+
}
48+
49+
func TestParseDNSQuery_TooShortReturnsEmpty(t *testing.T) {
50+
got := parseDNSQuery([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}) // 11 bytes < 12
51+
if got != "" {
52+
t.Fatalf("parseDNSQuery short query = %q, want empty", got)
53+
}
54+
}
55+
56+
func TestParseDNSQuery_LabelLen63OK(t *testing.T) {
57+
long := strings.Repeat("a", 63)
58+
q := buildQuery(t, long+".example")
59+
got := parseDNSQuery(q)
60+
if got != long+".example" {
61+
t.Fatalf("parseDNSQuery 63-char label = %q", got)
62+
}
63+
}
64+
65+
func TestParseDNSQuery_LabelLen64Rejected(t *testing.T) {
66+
long := strings.Repeat("a", 64)
67+
// build query manually since buildQuery rejects >63
68+
q := make([]byte, 12)
69+
binary.BigEndian.PutUint16(q[4:6], 1)
70+
q = append(q, byte(64))
71+
q = append(q, []byte(long)...)
72+
q = append(q, 0, 0, 1, 0, 1)
73+
got := parseDNSQuery(q)
74+
if got != "" {
75+
t.Fatalf("parseDNSQuery 64-char label = %q, want empty (rejected)", got)
76+
}
77+
}
78+
79+
func TestParseDNSQuery_CompressionPointerRejected(t *testing.T) {
80+
// A compression pointer has top two bits set (0xC0). The parser rejects via length > 63.
81+
q := buildQuery(t, "example.com")
82+
// Replace the first label-length byte with 0xC0 0x0C (pointer to offset 12)
83+
q[12] = 0xC0
84+
q[13] = 0x0C
85+
got := parseDNSQuery(q)
86+
if got != "" {
87+
t.Fatalf("parseDNSQuery compression pointer = %q, want empty (rejected)", got)
88+
}
89+
}
90+
91+
func TestBuildDNSResponse_ValidIPReturnsAResponse(t *testing.T) {
92+
q := buildQuery(t, "example.com")
93+
resp := buildDNSResponse(q, "198.18.0.5")
94+
if resp == nil {
95+
t.Fatal("buildDNSResponse returned nil for valid input")
96+
}
97+
98+
// Response should be the query, with flags updated to 0x8400, ANCOUNT=1.
99+
flags := binary.BigEndian.Uint16(resp[2:4])
100+
if flags&0x8400 != 0x8400 {
101+
t.Fatalf("response flags = 0x%04X, want 0x8400 bit set", flags)
102+
}
103+
ancount := binary.BigEndian.Uint16(resp[6:8])
104+
if ancount != 1 {
105+
t.Fatalf("ancount = %d, want 1", ancount)
106+
}
107+
108+
// The answer section is at the end. Check that it specifies type/class A=1 IN=1
109+
// and rdlength=4 and that the final 4 bytes are the IPv4.
110+
// Layout: query | 0xC0 0x0C | type(2) | class(2) | ttl(4) | rdlen(2) | rdata(4)
111+
answerStart := len(q)
112+
if int(resp[answerStart]) != 0xC0 || int(resp[answerStart+1]) != 0x0C {
113+
t.Fatalf("answer name pointer = 0x%02X%02X, want 0xC00C", resp[answerStart], resp[answerStart+1])
114+
}
115+
typeA := binary.BigEndian.Uint16(resp[answerStart+2 : answerStart+4])
116+
if typeA != 1 {
117+
t.Fatalf("answer type = %d, want 1 (A)", typeA)
118+
}
119+
rdlen := binary.BigEndian.Uint16(resp[answerStart+10 : answerStart+12])
120+
if rdlen != 4 {
121+
t.Fatalf("rdlength = %d, want 4", rdlen)
122+
}
123+
ip := net.IP(resp[answerStart+12 : answerStart+16]).String()
124+
if ip != "198.18.0.5" {
125+
t.Fatalf("answer IP = %s, want 198.18.0.5", ip)
126+
}
127+
}
128+
129+
func TestBuildDNSResponse_InvalidIPReturnsNil(t *testing.T) {
130+
q := buildQuery(t, "example.com")
131+
// An IP with no IPv4 representation: a hostname string
132+
if resp := buildDNSResponse(q, "not-an-ip"); resp != nil {
133+
t.Fatalf("buildDNSResponse with non-IP returned %v, want nil", resp)
134+
}
135+
}
136+
137+
func TestBuildDNSResponse_QueryTooShortReturnsNil(t *testing.T) {
138+
if resp := buildDNSResponse([]byte{0, 1, 2}, "1.2.3.4"); resp != nil {
139+
t.Fatalf("buildDNSResponse short query returned %v, want nil", resp)
140+
}
141+
}
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
# Plan 001: Gitignore Android-specific entries (AAR, keystore, caches)
2+
3+
> **Executor instructions**: Follow this plan step by step. Run every
4+
> verification command and confirm the expected result before moving to the
5+
> next step. If anything in the "STOP conditions" section occurs, stop and
6+
> report — do not improvise. When done, update the status row for this plan
7+
> in `plans/README.md` — unless a reviewer dispatched you and told you they
8+
> maintain the index.
9+
>
10+
> **Drift check (run first)**: `git diff --stat 29a1de2..HEAD -- .gitignore`
11+
> If `.gitignore` changed since this plan was written, compare the
12+
> "Current state" excerpt against the live code before proceeding; on a
13+
> mismatch, treat it as a STOP condition.
14+
15+
## Status
16+
17+
- **Priority**: P1
18+
- **Effort**: S
19+
- **Risk**: LOW
20+
- **Depends on**: none
21+
- **Category**: dx
22+
- **Planned at**: commit `29a1de2`, 2026-07-21
23+
24+
## Why this matters
25+
26+
The root `.gitignore` is Python-only. Running `bash ./android/build_go_mobile.sh` produces `android/app/libs/masterdnsvpn.aar` (the Go bridge AAR). The Android `build.gradle.kts:113` packages `*.aar` from that dir via `fileTree("libs", include listOf("*.aar", "*.jar"))`. The release workflow writes `android/release.jks` (the signing keystore). If a contributor `git add`s these by accident, the AAR bloats the repo and the keystore leaks the **private signing key** — a credential. Adding the entries is a one-line, zero-risk fix that prevents a high-impact mistake.
27+
28+
## Current state
29+
30+
`.gitignore` at repo root (25 lines):
31+
```
32+
__pycache__
33+
server_config.py
34+
client_config.py
35+
server_config.toml
36+
client_config.toml
37+
client_resolvers.txt
38+
encrypt_key.txt
39+
local_dns_cache.json
40+
local_dns_cache.bin
41+
*.key
42+
*.pem
43+
*.crt
44+
.vscode/
45+
.env
46+
.DS_Store
47+
logs/
48+
*.log
49+
*.zip
50+
*.tar.gz
51+
*.bak
52+
*.tmp
53+
*.exe
54+
build/
55+
.gocache
56+
.bench/
57+
```
58+
59+
Notable absences: `*.aar`, `*.jks`, `*.keystore`, `android/.gradle/`, `android/local.properties`, `android/app/libs/*.aar`.
60+
61+
The AAR target path is hardcoded in `android/build_go_mobile.sh:8`:
62+
```
63+
OUTPUT_AAR="$ROOT_DIR/android/app/libs/masterdnsvpn.aar"
64+
```
65+
66+
The keystore target path is hardcoded in `.github/workflows/release-manual.yml:56`:
67+
```
68+
echo "$ANDROID_KEYSTORE_BASE64" | base64 -d > android/release.jks
69+
```
70+
71+
## Commands you will need
72+
73+
| Purpose | Command | Expected on success |
74+
|--------|---------|---------------------|
75+
| Verify entries added | `git check-ignore -v android/app/libs/masterdnsvpn.aar android/release.jks android/.gradle/anything android/local.properties` | each path printed with `.gitignore:N` line |
76+
| Confirm git status clean of ignored paths | `git status --porcelain` | no new untracked files listed (改动 only `.gitignore`) |
77+
78+
## Scope
79+
80+
**In scope** (the only files you should modify):
81+
- `.gitignore` (append entries)
82+
83+
**Out of scope** (do NOT touch):
84+
- `android/app/build.gradle.kts` (the `fileTree libs` directive stays — AARs must still load at build time)
85+
- Any workflow file (the keystore path stays)
86+
- `android/build_go_mobile.sh` (the AAR output path stays)
87+
- Other gitignore entries (don't reorder or merge — append only)
88+
89+
## Git workflow
90+
91+
- Branch: `advisor/001-gitignore-android` (matches the repo's `advisor/NNN-<slug>` convention)
92+
- Commit message (match repo's conventional-commits style seen in `git log`):
93+
`chore: gitignore Android AAR, keystore, and Gradle caches`
94+
- Do NOT push or open a PR unless instructed.
95+
96+
## Steps
97+
98+
### Step 1: Append Android entries to `.gitignore`
99+
100+
Append the following block to the end of `.gitignore`. Use a single edit that adds exactly these lines after the existing `.bench/` line:
101+
102+
```
103+
# Android build artifacts and signing keys
104+
*.aar
105+
*.jks
106+
*.keystore
107+
android/.gradle/
108+
android/local.properties
109+
android/release.jks
110+
android/app/libs/*.aar
111+
```
112+
113+
Note the loose `*.aar` and `*.jks` entries give belt-and-suspenders defense (catching AARs/keystores written anywhere outside the android tree); the `android/...` path-specific entries document the canonical locations.
114+
115+
**Verify**: `git check-ignore -v android/app/libs/masterdnsvpn.aar android/release.jks android/.gradle/foo android/local.properties`
116+
→ Each line prints the matching `.gitignore:N` rule.
117+
118+
### Step 2: Confirm no other files were touched
119+
120+
**Verify**: `git status --porcelain`
121+
→ Exactly one modified line: ` M .gitignore` (or `??` for untracked entries you didn't create — none expected).
122+
123+
## Test plan
124+
125+
No tests apply. This is a config-only change verified by `git check-ignore`.
126+
127+
## Done criteria
128+
129+
ALL must hold:
130+
131+
- [ ] `git check-ignore -v android/app/libs/masterdnsvpn.aar` returns a `.gitignore:N` rule
132+
- [ ] `git check-ignore -v android/release.jks` returns a `.gitignore:N` rule
133+
- [ ] `git check-ignore -v android/.gradle/test` returns a `.gitignore:N` rule
134+
- [ ] `git check-ignore -v android/local.properties` returns a `.gitignore:N` rule
135+
- [ ] `git status --porcelain` shows only `.gitignore` modified
136+
- [ ] `plans/README.md` status row updated
137+
138+
## STOP conditions
139+
140+
Stop and report back (do not improvise) if:
141+
142+
- `.gitignore` at HEAD already contains `*.aar` or `*.jks` (likely another contributor fixed this; reconcile instead of duplicating).
143+
- `git check-ignore` returns no match for any of the four paths in Step 1's verify — the entry was added incorrectly (whitespace, trailing `\r`, or wrong line).
144+
145+
## Maintenance notes
146+
147+
- A future move of the AAR output path (e.g. modularizing `mobile/` into its own gradle module) must update the `android/app/libs/*.aar` entry to the new path.
148+
- A future switch to environment-derived keystores (no on-disk JKS) can remove `*.jks` and `android/release.jks`, but should leave them until that work lands.
149+
- Reviewer: scan for `git add android/app/libs/*.aar` or `git add android/release.jks` in future PRs — the ignore alone doesn't prevent `git add -f`.

0 commit comments

Comments
 (0)