A refused licence is retried forever: no negative caching in refreshStatus() or fetchManifest()

[mostafasoufi]

Summary

A site whose licence the server refuses will keep asking forever, at full speed. Two separate pieces of caching are missing, and together they form a loop with no exit.

The two gaps

1. LicenseManager::refreshStatus() never records a refusal.

try {
    $response = $this->client->validate($licenseKey, '');
    $existing = $this->store->get('license');
    $this->store->set('license', $this->mapApiResponse($response, $licenseKey, null, $existing));
} catch (Exception $e) {
    // Keep the cached license; a failed refresh must not lock out a valid site.
}

The intent is right: a site that cannot reach the server must not be locked out. But the catch cannot tell "the network is down" from "the server says this licence is suspended". When the server answers an expired or suspended licence with an HTTP error, the client throws, and the stored status stays at whatever it last was — active. It never moves off that value again.

2. PluginUpdater::fetchManifest() caches success only.

try {
    $manifest = $this->client->fetchManifest($licenseKey, $this->config->currentVersion());
} catch (Exception $e) {
    return null;                                   // nothing written to the transient
}

set_site_transient($cacheKey, $manifest, HOUR_IN_SECONDS * 12);

A successful fetch is quiet for 12 hours. A failed fetch writes nothing, so the next update-check pass repeats the request immediately. The early if (! $this->license->isValid()) return null; above it also returns without writing anything, so that path has the same shape.

Why they compound

PluginUpdater::injectPluginUpdate() guards on $this->license->isValid(), which reads the stored status. Because gap 1 leaves that permanently at active, the guard always passes; because of gap 2, every pass makes a fresh network call that fails. pre_set_site_transient_update_plugins runs on a large share of admin requests, so a busy site can generate thousands of refused requests a day and never back off.

Suggested fix

1. In refreshStatus(), separate an authoritative refusal from a transport failure. When the server actually answers with a licence state, store that state, including a negative one. Keep the current forgiving behaviour only for genuine transport errors (timeout, DNS, connection refused).
2. In fetchManifest(), cache the negative outcome too, with its own shorter TTL, so a refused check costs a handful of requests a day instead of thousands. Both the exception path and the isValid() early return need it.
3. Add a bounded backoff so repeated failures lengthen the interval rather than repeating at a fixed rate.

Point 1 is the one that matters most. Without it a site can never learn that its licence stopped being valid, which is a correctness problem in its own right, separate from the traffic.