Skip to content

Commit 62e2956

Browse files
committed
Do not let a blip outrank an account problem where it counts
Second review pass over this release, plus the two things Plugin Check caught on the way in. The one that mattered: the rule that an outage must not bury an account problem guarded the stored verdict but not the cached one - and the cached one is what the gate reads. So a passing network hiccup still replaced "your free tier is used up" for five minutes in the only place that decides whether a form is let through, and with failsafe on those submissions went through unverified. The guard now runs before either is written. Two more in the same file. Skipping the write when nothing changed froze the timestamp at the first sighting, and the outage notice hides itself once that is an hour old - so it would have vanished mid-outage; the record is now refreshed at most every five minutes, which is neither a write per request nor a lie about when we last saw the problem. And the settings screen's connection test classified a response by its body alone, so a 5xx carrying a cached error body could be read as an account problem the test itself could never clear; both paths now share one classifier. Elsewhere: A form that arrives after the page does - a contact form in a popup, a builder rendering on demand - never got a badge. Submission was always safe there, since those listeners are delegated on the document; this is the badge catching up, on first focus rather than through an observer. The credit link announced itself as "Powered by" and nothing else, because the logo beside it is decorative. The translators comment sat above the sprintf rather than above the __() call, which is where the standard looks for it. The widget's missing version is deliberate and now says so at the line rather than being silenced for the whole plugin, where a version we forgot on our own script would matter.
1 parent 38d42b1 commit 62e2956

8 files changed

Lines changed: 104 additions & 37 deletions

‎assets/css/badge.css‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@
2525
box-sizing: border-box;
2626
/* clear only works in normal flow. Block themes and page builders lay forms
2727
out with flex or grid, where an appended element becomes an item on the
28-
last row instead of a row of its own; these two claim the full width
29-
there and do nothing at all outside such a container. */
28+
last row instead of a row of its own. grid-column spans every column;
29+
flex-basis claims the full width, which puts the badge on its own line
30+
wherever the container wraps. Both are inert outside such a container. */
3031
flex-basis: 100%;
3132
grid-column: 1 / -1;
3233
padding: 16px 0 0;

‎assets/js/captchaapi-ajax.js‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,11 @@
9696
function badge() {
9797
var match = selector();
9898

99-
if (!match || !window.captchaapiBadge || typeof window.captchaapiBadge.attachAndWait !== 'function') {
99+
if (!match || !window.captchaapiBadge || typeof window.captchaapiBadge.watch !== 'function') {
100100
return;
101101
}
102102

103-
var forms = document.querySelectorAll(match);
104-
105-
for (var i = 0; i < forms.length; i++) {
106-
window.captchaapiBadge.attachAndWait(forms[i]);
107-
}
103+
window.captchaapiBadge.watch(match);
108104
}
109105

110106
if (document.readyState === 'loading') {

‎assets/js/captchaapi-badge.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@
9898
link.target = '_blank';
9999
link.rel = 'noopener noreferrer';
100100

101+
// The visible text is "Powered by" beside a decorative logo, which on
102+
// its own tells a screen reader nothing about where the link goes.
103+
if (config.aria) {
104+
link.setAttribute('aria-label', config.aria);
105+
}
106+
101107
if (config.logo) {
102108
var logo = document.createElement('img');
103109
logo.className = 'captchaapi-badge__logo';
@@ -129,11 +135,46 @@
129135
}
130136
}
131137

138+
/**
139+
* Attaches to everything matching now, and to anything that shows up later.
140+
*
141+
* A contact form can arrive after the page does - pulled into a popup, or
142+
* rendered on demand by a page builder - and a one-off pass at load would
143+
* miss it. Submission is already safe there, because those listeners are
144+
* delegated on the document; this is the badge catching up. A delegated
145+
* focus listener rather than a MutationObserver: nobody submits a form they
146+
* have not touched, and it costs nothing until they do.
147+
*/
148+
function watch(selector) {
149+
if (!selector) {
150+
return;
151+
}
152+
153+
attachAll(selector);
154+
155+
document.addEventListener('focusin', function (event) {
156+
var form = event.target && event.target.closest ? event.target.closest('form') : null;
157+
158+
if (form && form.matches && form.matches(selector)) {
159+
attachAndWait(form);
160+
}
161+
}, true);
162+
}
163+
164+
function attachAll(selector) {
165+
var forms = document.querySelectorAll(selector);
166+
167+
for (var i = 0; i < forms.length; i++) {
168+
attachAndWait(forms[i]);
169+
}
170+
}
171+
132172
// Published on the config object the plugin already puts here, so the
133173
// integration scripts have one place to go for both.
134174
window.captchaapiBadge = window.captchaapiBadge || {};
135175
window.captchaapiBadge.attach = attach;
136176
window.captchaapiBadge.attachAndWait = attachAndWait;
177+
window.captchaapiBadge.watch = watch;
137178

138179
if (document.readyState === 'loading') {
139180
document.addEventListener('DOMContentLoaded', addBadge);

‎assets/js/captchaapi-cf7.js‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,11 @@
106106
// it on its own - the badge is attached here and put into standby, and
107107
// every state after that arrives through solve({ form }).
108108
function badge() {
109-
if (!window.captchaapiBadge || typeof window.captchaapiBadge.attachAndWait !== 'function') {
109+
if (!window.captchaapiBadge || typeof window.captchaapiBadge.watch !== 'function') {
110110
return;
111111
}
112112

113-
var forms = document.querySelectorAll('form.wpcf7-form');
114-
115-
for (var i = 0; i < forms.length; i++) {
116-
window.captchaapiBadge.attachAndWait(forms[i]);
117-
}
113+
window.captchaapiBadge.watch('form.wpcf7-form');
118114
}
119115

120116
if (document.readyState === 'loading') {

‎assets/js/captchaapi-woocommerce.js‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,11 @@
7373
// it on its own - the badge is attached here and put into standby, and
7474
// every state after that arrives through solve({ form }).
7575
function badge() {
76-
if (!window.captchaapiBadge || typeof window.captchaapiBadge.attachAndWait !== 'function') {
76+
if (!window.captchaapiBadge || typeof window.captchaapiBadge.watch !== 'function') {
7777
return;
7878
}
7979

80-
var forms = document.querySelectorAll(SELECTOR);
81-
82-
for (var i = 0; i < forms.length; i++) {
83-
window.captchaapiBadge.attachAndWait(forms[i]);
84-
}
80+
window.captchaapiBadge.watch(SELECTOR);
8581
}
8682

8783
if (document.readyState === 'loading') {

‎includes/class-captchaapi-assets.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ private function enqueue_widget(
211211
// browser to whatever build shipped alongside this plugin version
212212
// and hide the next widget deploy until the plugin releases again.
213213
// Its own ETag and Last-Modified decide when a browser refetches.
214+
// phpcs:ignore WordPress.WP.EnqueuedResourceParameters.MissingVersion -- Deliberate: the service versions this file, we do not.
214215
wp_register_script('captchaapi', $this->options->widget_url(), $deps, null, $in_footer);
215216
wp_add_inline_script('captchaapi', $this->config_script(), 'before');
216217
}
@@ -308,6 +309,8 @@ private function badge_config(array $selectors): array
308309
'href' => $this->options->site_url(),
309310
'logo' => CAPTCHAAPI_PLUGIN_URL . 'assets/img/captchaapi-logo.svg',
310311
'label' => __('Powered by', 'captchaapi'),
312+
/* translators: %s: captchaapi.eu, the service name. */
313+
'aria' => sprintf(__('Powered by %s', 'captchaapi'), 'captchaapi.eu'),
311314
];
312315
}
313316

‎includes/class-captchaapi-gate.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,8 @@ public static function reason_for(string $code): string
222222
// integrations hand this message to form plugins that print it
223223
// without escaping. It arrives over the network, so it is not
224224
// ours to trust however well we think we know the sender.
225-
/* translators: %s: error code returned by the captchaapi.eu service. */
226225
return sprintf(
226+
/* translators: %s: error code returned by the captchaapi.eu service. */
227227
__('The captchaapi.eu service rejected the request (%s).', 'captchaapi'),
228228
esc_html($code)
229229
);

‎includes/class-captchaapi-service.php‎

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ class Captchaapi_Service
3838

3939
const CACHE_TTL = 5 * MINUTE_IN_SECONDS;
4040

41+
/**
42+
* How stale a recorded problem may get before it is written again. Long
43+
* enough that a busy site is not writing an option per request, short
44+
* enough that "when did we last see this" stays true.
45+
*/
46+
const REFRESH_AFTER = 5 * MINUTE_IN_SECONDS;
47+
4148
/**
4249
* Error codes that mean the site cannot get challenges until the owner does
4350
* something about it. Anything else from the challenge endpoint is treated
@@ -103,15 +110,26 @@ public function blocking_code(): string
103110
*/
104111
public function remember(string $state, string $code = ''): void
105112
{
113+
$stored = get_option(self::STATE_OPTION);
114+
$stored = is_array($stored) ? $stored : null;
115+
$blocked = $stored !== null && ($stored['state'] ?? '') === self::NOT_ENFORCEABLE;
116+
117+
// A passing outage must not bury an account problem. One is a blip that
118+
// hides itself after an hour; the other needs the owner to act, and only
119+
// a submission that verifies says it is over. This governs the cached
120+
// verdict as much as the stored one: the cache is what the gate reads,
121+
// so letting a blip overwrite it there would quietly hand an over-limit
122+
// account the failsafe treatment it must never get.
123+
if ($state === self::UNAVAILABLE && $blocked) {
124+
return;
125+
}
126+
106127
// A real verification just told us more than a probe could, so the
107128
// cached verdict moves with it. Leaving it behind would let a stale
108129
// "everything is fine" answer outlive the account going over its limit,
109130
// and vice versa, for the length of the cache.
110131
set_transient(self::TRANSIENT, ['state' => $state, 'code' => $code], self::CACHE_TTL);
111132

112-
$stored = get_option(self::STATE_OPTION);
113-
$stored = is_array($stored) ? $stored : null;
114-
115133
if ($state === self::ENFORCING) {
116134
// Only on a real transition. Otherwise every verified submission on
117135
// the site would run a delete for a row that is not there.
@@ -122,17 +140,16 @@ public function remember(string $state, string $code = ''): void
122140
return;
123141
}
124142

125-
// A passing outage must not bury an account problem. One is a blip that
126-
// hides itself after an hour; the other needs the owner to act and is
127-
// cleared only by a submission that verifies.
128-
if ($state === self::UNAVAILABLE && $stored !== null && ($stored['state'] ?? '') === self::NOT_ENFORCEABLE) {
129-
return;
130-
}
143+
// An unchanged verdict is rewritten only now and then. Writing every
144+
// time would mean a wp_options write per request during an outage -
145+
// every bot hitting the login form - but never writing would freeze the
146+
// timestamp at the first sighting, and the outage notice hides itself
147+
// once that is an hour old. It would vanish mid-outage.
148+
$unchanged = $stored !== null
149+
&& ($stored['state'] ?? '') === $state
150+
&& ($stored['code'] ?? '') === $code;
131151

132-
// Unchanged verdicts are not rewritten. The timestamp would differ every
133-
// time, so without this an outage means a wp_options write per request -
134-
// including every bot hitting the login form.
135-
if ($stored !== null && ($stored['state'] ?? '') === $state && ($stored['code'] ?? '') === $code) {
152+
if ($unchanged && (time() - (int) ($stored['time'] ?? 0)) < self::REFRESH_AFTER) {
136153
return;
137154
}
138155

@@ -185,17 +202,34 @@ private function probe(): array
185202
'body' => wp_json_encode(['site_key' => $site_key]),
186203
]);
187204

205+
return self::classify($response);
206+
}
207+
208+
/**
209+
* Turns a challenge-endpoint reply into a state. Shared so the settings
210+
* screen's "Test connection" cannot reach a different conclusion from the
211+
* probe about the very same response.
212+
*
213+
* @param array<string, mixed>|WP_Error $response
214+
*
215+
* @return array{0: string, 1: string} state and the error code behind it
216+
*/
217+
public static function classify($response): array
218+
{
188219
if (is_wp_error($response)) {
189220
return [self::UNAVAILABLE, ''];
190221
}
191222

192-
$code = (int) wp_remote_retrieve_response_code($response);
223+
$status = (int) wp_remote_retrieve_response_code($response);
193224

194-
if ($code === 200) {
225+
if ($status === 200) {
195226
return [self::ENFORCING, ''];
196227
}
197228

198-
if ($code >= 500) {
229+
// A 5xx is ours whatever the body claims. An error body cached by a
230+
// proxy during an incident could otherwise be read as an account
231+
// problem and stick around until a submission verifies.
232+
if ($status >= 500) {
199233
return [self::UNAVAILABLE, ''];
200234
}
201235

0 commit comments

Comments
 (0)