Backend LRU cache behavior when MaxEntries == 0 is misleading

[JeremyRand]

The text that appears via ncdns --help includes the following text for the cachemaxentries flag: Maximum name cache entries (default 100). This would lead the user to believe that setting it to 0 will disable the cache. However, in the implementation of Backend, we see this:

b.cache.MaxEntries = cfg.CacheMaxEntries
if b.cache.MaxEntries == 0 {
	b.cache.MaxEntries = defaultMaxEntries
}

defaultMaxEntries is set to... 100.

This behavior is extremely misleading and is likely to cause security problems for users who think they've disabled the LRU cache in order to avoid leaking state between queries (e.g. in the case of Tor stream isolation). Granted, implementing stream-isolated LRU caches is the "right way" to solve the specific use case of Tor stream isolation, but this behavior is still misleading and dangerous.

It's not clear to me why ncdns doesn't just pass through cfg.CacheMaxEntries without the special case for 0. Does the LRU implementation break stuff if you tell it the max size is 0?

[JeremyRand]

The misbehaving code was introduced by 3f88db3 . @hlandau Do you happen to recall why the special-casing of 0 was added by that commit?

[JeremyRand]

Ah, I see. The LRU cache implementation has the following comment:

// MaxEntries is the maximum number of cache entries before
// an item is evicted. Zero means no limit.

So, I think replacing 0 with 100 is a really bad way to deal with this. Users who are familiar with the LRU cache library will think they're getting limitless cache, and users who are going by the ncdns help text will think they're getting a disabled cache. I'd much rather throw an error when the user picks 0, and have extra command-line boolean flags for "disable cache" (which disables the code that calls the LRU cache at all) and "unlimited cache" (which passes 0 to the LRU cache library).