fix(redis): Don't SETEX with a non-positive TTL

[DarkaMaul]

RedisCache.set forwarded its expiry straight to SETEX, which Redis rejects for seconds <=0 with an "invalid expire time" ResponseError.

(Verified against a local redis container)

[woodruffw]

Hmm, I might be missing something but shouldn't we be catching this at a higher layer? I believe the catching RFCs say that invalid ages/TTLs should be treated as 0, which we should probably check and handle well before we get to the actual underlying caching implementation.

Ref: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Cache-Control#:~:text=If%20the%20max%2Dage%20value%20is%20negative%20(for%20example%2C%20%2D1)%20or%20isn't%20an%20integer%20(for%20example%2C%203599.99)%2C%20then%20the%20caching%20behavior%20is%20unspecified.%20Caches%20are%20encouraged%20to%20treat%20the%20value%20as%20if%20it%20were%200%20(this%20is%20noted%20in%20the%20Calculating%20Freshness%20Lifetime%20section%20of%20the%20HTTP%20specification).

[DarkaMaul]

Ooo, I was not familiar with this part of the spec... Indeed, the much cleaner fix is to reject negative values much earlier.
Note that the internal docs says :

The google homepage specifically uses a negative expires header and private cache control header to avoid caches. We've managed to work around that aspect and cache the response using our heuristic.

I can refine the PR to just handle the 0 case that also fails with setex. And then, normalize/reject negative values in the controller, WDYT?

[woodruffw]

Yeah, that sounds good to me. Specifically we should normalize k valid TTLs to zero (and emit a warning log in that case, maybe?), which the controller logic should then treat as "don't cache this."

(That'll change the Google behavior but IMO that's good -- we should follow the specs precisely here.)