Skip to content

fix: guard LRUCache against non-positive capacity - #1749

Open
nikolauspschuetz wants to merge 1 commit into
apache:masterfrom
nikolauspschuetz:fix/lrucache-non-positive-capacity
Open

fix: guard LRUCache against non-positive capacity#1749
nikolauspschuetz wants to merge 1 commit into
apache:masterfrom
nikolauspschuetz:fix/lrucache-non-positive-capacity

Conversation

@nikolauspschuetz

Copy link
Copy Markdown
Contributor

What

NewLRUCache and NewSyncLRUCache are exported public API, but calling Put on a cache created with a non-positive capacity panics with a nil pointer dereference.

Reproduce:

c := util.NewLRUCache(0)
c.Put("a", 1) // panic: runtime error: invalid memory address or nil pointer dereference

On the first insert, the eviction branch is taken (len(cache.m) >= cache.capacity is 0 >= 0), which calls cache.remove(cache.tail.prev, false). On an empty list cache.tail.prev is the head sentinel, whose prev is nil, so remove dereferences nil at util/util.go.

Fix

Return early from Put when capacity <= 0 — such a cache holds nothing, so there is nothing to store and no eviction to perform. This keeps Get returning (nil, false) for every key, consistent with a zero-capacity cache.

Guarding the eviction path instead would let a zero-capacity cache incorrectly hold one entry, so the early return is the more correct minimal fix.

Test

Added TestLRUCacheNonPositiveCapacity in util/util_test.go. It panics on master and passes with this change:

  • before: panic: runtime error: invalid memory address or nil pointer dereference (in Put)
  • after: ok github.com/casbin/casbin/v3/util

for the record: fix found and drafted with AI assistance; reviewed and verified by me.

NewLRUCache and NewSyncLRUCache are exported, but calling Put on a cache
created with a non-positive capacity panicked with a nil pointer
dereference: the eviction path removes cache.tail.prev, which on an empty
list is the head sentinel whose prev pointer is nil.

Return early from Put when capacity is non-positive, since such a cache
holds nothing. Add a regression test that fails (panics) before this
change and passes after.
@nikolauspschuetz
nikolauspschuetz marked this pull request as ready for review August 16, 2026 15:12
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