diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProvider.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProvider.kt index af62b37767..4ce346f6c9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProvider.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProvider.kt @@ -139,6 +139,15 @@ class BlossomReadAuthTokenProvider( if (header != null) { cache[host] = CachedToken(header, clock() + CACHE_TTL_MS) } + + // Retire the entry *before* completing it. `invokeOnCompletion` fires when the + // job ends, which is after `complete()` resumes the awaiting caller — so a + // caller that returned from `header()` could come straight back, find this + // finished deferred still in the map, and be handed its already-signed token + // instead of signing a new one. A caller whose token has just expired does + // exactly that, and got the expired token back for as long as the window + // lasted — [refreshesAfterExpiry] closes it immediately and so hit it every run. + inFlight.remove(host, fresh) fresh.complete(header) }.invokeOnCompletion { inFlight.remove(host, fresh) diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProviderTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProviderTest.kt index b209522a56..de8de985e2 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProviderTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProviderTest.kt @@ -35,6 +35,7 @@ import kotlinx.coroutines.withTimeout import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test @@ -114,14 +115,28 @@ class BlossomReadAuthTokenProviderTest { fun refreshesAfterExpiry() = runBlocking { var now = 0L - val provider = BlossomReadAuthTokenProvider({ signer }, scope, clock = { now }) + var lookups = 0 + val provider = + BlossomReadAuthTokenProvider({ + lookups++ + signer + }, scope, clock = { now }) + + assertNotNull(provider.header(host)) + assertEquals("the first call must sign", 1, lookups) - val first = provider.header(host) now += 56L * 60L * 1000L assertNull("token must be gone from the pure read once expired", provider.cachedHeader(host)) - val second = provider.header(host) - assertNotEquals("an expired token must be re-signed", first, second) + // Counts signatures rather than comparing the two headers: the injected [clock] only + // drives the cache TTL, while the signed BlossomAuthorizationEvent takes its + // `created_at` from the real wall clock (TimeUtils.now()). Both signings land in the + // same second on any quick machine, so the two events — and therefore their ids, sigs + // and headers — are byte-identical, and an assertNotEquals on them fails even though + // the token was correctly re-signed. The signer is only ever consulted on a real + // signing pass (a cache hit returns before it), so this counter says exactly that. + assertNotNull(provider.header(host)) + assertEquals("an expired token must be re-signed", 2, lookups) } @Test