Skip to content

fix: guard addrPrefix against panic from cache prefetch#61

Open
alam0rt wants to merge 1 commit into
coredns:masterfrom
alam0rt:fix/addrprefix-prefetch-panic
Open

fix: guard addrPrefix against panic from cache prefetch#61
alam0rt wants to merge 1 commit into
coredns:masterfrom
alam0rt:fix/addrprefix-prefetch-panic

Conversation

@alam0rt

@alam0rt alam0rt commented May 19, 2026

Copy link
Copy Markdown

I am still in the process of testing, but after updating CoreDNS to 1.14.3, I noticed that our rrl config was panicking

foo:53 {
    import common
    import rrl
    cache 3600 {
        ...
        prefetch 100 10s # here
    }
}

Confirmed that removing prefetch line prevents panic, but alas, we want prefetch :)

🤖 The PR was made by AI, above are my words & below is Claude Opus 4.7


Summary

CoreDNS v1.14.3 changed newPrefetchResponseWriter to use a static empty &net.TCPAddr{} instead of preserving the original client IP (relevant code). This means RemoteAddr().String() now returns ":0" (2 bytes) for prefetch-initiated requests.

When the RRL plugin is configured in the same server block as cache with prefetch enabled, addrPrefix() panics:

panic: runtime error: slice bounds out of range [1:-1]

goroutine 581 [running]:
github.com/coredns/rrl/plugins/rrl.(*RRL).addrPrefix(0xc0006e2ab0, {0xc001b13cf8, 0x2})
        rrl.go:223 +0x206
github.com/coredns/rrl/plugins/rrl.(*RRL).ServeDNS(...)
        handler.go:29 +0x106

The sequence:

  1. addr = ":0" from net.TCPAddr{}.String()
  2. i = strings.LastIndex(":0", ":")i = 0
  3. net.ParseIP(addr[:0])nil
  4. nil.To4()nil, falls through to IPv6 branch
  5. addr[1 : 0-1]panic: slice bounds out of range [1:-1]

Fix

Add an early return when LastIndex returns ≤ 0:

if i <= 0 {
    return addr
}

All prefetch requests share a single RRL bucket keyed on the raw addr string (":0"). This is correct behavior — internal cache refreshes shouldn't penalize real clients.

Corefile to reproduce

consul:53 {
    rrl . {
        window 1
        ipv4-prefix-length 32
        requests-per-second 100
    }
    forward . 169.254.1.1:8600
    cache 3600 {
        prefetch 100 10s
    }
}

Test

Added panic-regression tests for ":0", "", and ":" inputs.

@alam0rt alam0rt force-pushed the fix/addrprefix-prefetch-panic branch from 855a59e to ef8381e Compare May 19, 2026 04:09
CoreDNS v1.14.3 changed cache prefetch to use an empty net.TCPAddr{}
as the RemoteAddr for internal prefetch requests. RemoteAddr().String()
now returns ":0" (length 2) instead of preserving the original client
IP address.

When the RRL plugin is in the same server block as a cache with
prefetch enabled, addrPrefix() panics on this input:

  panic: runtime error: slice bounds out of range [1:-1]

The fix adds an early return when LastIndex returns 0 or -1, which
covers both ":0" (empty TCPAddr) and "" (no colon at all). In this
case the unmodified addr string is returned as the rate-limit token,
meaning all prefetch-originated requests share a single bucket rather
than penalizing any real client.

Signed-off-by: alam0rt <sam@samlockart.com>
@alam0rt alam0rt force-pushed the fix/addrprefix-prefetch-panic branch from ef8381e to ba8de82 Compare May 19, 2026 04:23
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.

1 participant