fix: guard addrPrefix against panic from cache prefetch#61
Open
alam0rt wants to merge 1 commit into
Open
Conversation
855a59e to
ef8381e
Compare
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>
ef8381e to
ba8de82
Compare
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
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.
I am still in the process of testing, but after updating CoreDNS to 1.14.3, I noticed that our rrl config was panicking
Confirmed that removing
prefetchline 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
newPrefetchResponseWriterto use a static empty&net.TCPAddr{}instead of preserving the original client IP (relevant code). This meansRemoteAddr().String()now returns":0"(2 bytes) for prefetch-initiated requests.When the RRL plugin is configured in the same server block as
cachewithprefetchenabled,addrPrefix()panics:The sequence:
addr = ":0"fromnet.TCPAddr{}.String()i = strings.LastIndex(":0", ":")→i = 0net.ParseIP(addr[:0])→nilnil.To4()→nil, falls through to IPv6 branchaddr[1 : 0-1]→ panic: slice bounds out of range [1:-1]Fix
Add an early return when
LastIndexreturns ≤ 0: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
Test
Added panic-regression tests for
":0","", and":"inputs.