Harden rl.php endpoint check and surface actionable diagnostics

[jjroelofs]

Summary

Implements all hardening items from #50 in a single change.

The rl.php accessibility check (EndpointChecker) produced false negatives behind reverse proxies that terminate TLS — a common Drupal multisite setup — and false positives when its probe followed a redirect chain that ended at an unrelated 200. The hook_requirements() warning collapsed every failure into a generic "rl.php is not accessible," leaving operators with no idea what to fix.

This PR turns the warning into a precise, actionable diagnostic and removes the most common false-negative path.

What changed

EndpointChecker

• Validate the response body is pong, not just status 200. Catches redirect chains that land on a different host returning HTML.
• Trust X-Forwarded-Proto for the probe URL even when $settings['reverse_proxy'] isn't set. Safe for a same-site self-probe.
• After a public-URL failure, retry against http://127.0.0.1[:port] with the original Host header. A loopback success tells you the file is being served correctly and the failure is in the proxy / scheme / DNS chain — not in Apache or PHP.
• Return a structured result (status, detail, code, public_url, loopback_url, redirects) so the requirements report can describe the actual failure mode.
• Keep follow-redirects on (legit setups may HTTP→HTTPS upgrade or canonical-host redirect) but enable track_redirects so the body-mismatch diagnostic can name the redirect target. Use strict mode so POST survives 301/302 hops.
• Asymmetric cache TTLs: 1 h on success, 5 min on failure so a fixed misconfig clears without manual cache rebuild.
• Narrow the swallow-all \Exception fallback to cURL errno 6/7 (COULDNT_RESOLVE_HOST / COULDNT_CONNECT). SSL handshake errors and timeouts now surface instead of being silently treated as success.

rl_requirements()
One branch per status with a description that names the specific failure (redirect target, HTTP code, body sample, exception detail) and points at the right config knob.

Public API

Preserved. The existing EndpointChecker::isAccessible(): bool stays as a thin wrapper over the new EndpointChecker::getResult(): array. Existing callers (e.g. dxpr_builder_page_attachments()) keep working unchanged.

State-cache compatibility

The cached state shape changed (accessible → result). Old entries are detected and refreshed gracefully — no surprise TRUE or FALSE from a stale cache. Worst case: one extra check on the first request after upgrade.

A note on disabling redirects

I considered hard-disabling redirect-following so the probe could fail loudly on any 3xx, but that would false-fail legit setups (canonical-host redirects, http→https upgrade). Body-validation alone catches the bad cases (today's failure: redirect to wrong host returning HTML 200) while still tolerating sites whose canonical URL is reached via a 301. The track_redirects option gives the diagnostic enough info ("body wasn't pong, followed redirect to X") without rejecting valid redirect chains.

Test plan

• php -l clean on changed files
• phpcs --standard=Drupal clean
• phpcs --standard=DrupalPractice clean
• CI: drupal-lint, drupal-check, drush-e2e
• Manual: install module on a Drupal 11 site behind nginx → Apache; visit /admin/reports/status. Confirm:
  • With $settings['reverse_proxy'] set: rl.php is accessible ✓
  • With $settings['reverse_proxy'] unset and X-Forwarded-Proto present: still rl.php is accessible ✓ (new XFP trust)
  • Force a misconfig (point nginx redirect to wrong host): warning reads "rl.php probe was redirected to an unexpected target …" with the redirect URL, not the generic message.
  • Force a non-200 (rename rl.php temporarily): warning reads "rl.php returned HTTP 404" with the probed URL.
  • Force a body change (have the file echo something else): warning reads "rl.php returned unexpected content" with the body sample.

Possible follow-ups (not in this PR)

• Unit tests for probe() classification (no tests/ directory exists yet — adding one is its own change).
• Optional strict mode with redirects fully disabled, exposed as a config flag for operators who want the strict probe behavior.

Closes #50

[jjroelofs]

Thanks for the thorough work here. The core fix (body validation, structured results, asymmetric cache TTLs) is solid. One thing to fix before merge:

STATUS_REDIRECTED is overloaded -- When the loopback probe succeeds but the public probe fails, check() always returns STATUS_REDIRECTED regardless of the actual public failure. If the public probe failed with a TLS handshake error or timeout, the status report says "rl.php probe was redirected to an unexpected target" and advises setting $settings['reverse_proxy'], which is wrong advice for those cases. Either add a dedicated status (e.g. STATUS_PROXY_MISMATCH) for the "loopback OK, public failed" scenario, or pass through the original public probe status and attach the loopback success as supplementary context.

Two smaller items that can be follow-ups:

1. cURL errno 6/7 returns STATUS_OK before the loopback probe runs -- this early return means the loopback fallback never fires for DNS/connect failures, which undercuts its diagnostic value. Consider letting those fall through to the loopback probe.
2. Trusting X-Forwarded-Proto without reverse_proxy -- the justification holds for a self-probe, but worth an inline comment making clear this is a deliberate exception, not a general pattern.

[jjroelofs]

Addressed all three points in 60d9e38:

1. STATUS_REDIRECTED overload — went with your option (b): check() now preserves the actual public probe status (REDIRECTED / HTTP_ERROR / BODY_MISMATCH / CONNECTION_ERROR) and attaches loopback_ok=TRUE + loopback_url as supplementary context when loopback works. Each hook_requirements() branch surfaces that hint as "the local server is fine, fix the proxy / scheme / redirect rule" alongside the precise diagnosis. The STATUS_REDIRECTED description now also names the actual redirect target instead of giving generic reverse-proxy advice.

2. errno 6/7 short-circuit — removed. probe() now reports STATUS_CONNECTION_ERROR for any transport-layer failure and lets check() decide. When public is CONNECTION_ERROR AND loopback is OK, check() upgrades to STATUS_OK — that's the Docker / split-horizon-DNS case where browsers can reach the public URL but the server can't reach itself. Stronger signal than the previous "file exists on disk, assume good" heuristic.

3. XFP-trust docblock — added a short note that this is a deliberate self-probe-only exception, not a pattern to copy elsewhere.

Validated end-to-end on the real multisite where this all started: production-realistic request (XFP=https) → STATUS_OK; broken-redirect simulation → no longer mis-routes to the reverse_proxy advice. Local lint clean (phpcs --standard=Drupal and DrupalPractice); CI workflow re-running on the new commit.