diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index ac3c877..dd4dc87 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -16,7 +16,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: '8.2' - extensions: json, simplexml + extensions: json, simplexml, curl coverage: none - name: Install dependencies diff --git a/CLAUDE.md b/CLAUDE.md index 8933460..0553a07 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,6 +36,8 @@ Two hooks work in tandem so both the web UI and API sync clients (Fever, GReader - **Embed type normalisation** — the API returns `$type` values like `app.bsky.embed.images#view`; the `#view` suffix is stripped before the switch statement. - **User config** — `depth` (int, 1–1000, default 10) is stored via `setUserConfigurationValue`/`getUserConfigurationValue` and read in `handleConfigureAction()` on POST. - **No auth required** — all requests go to `public.api.bsky.app` and need no credentials. +- **HTTP transport must be cURL** — `apiGet()` uses `curl_*`, *not* `file_get_contents()`. FreshRSS ≥ 1.30.0 calls `unregister_unsafe_protocols()` in `lib/lib_rss.php` at boot, which unregisters every stream wrapper except `file://` and `php://` as an SSRF mitigation. Any remote `file_get_contents()`/`fopen()` in an extension therefore fails with *"Unable to find the wrapper https"*. `testApiGetWorksWithoutHttpsStreamWrapper` guards this by unregistering the wrapper before calling the API. +- **Failures are logged, not swallowed** — `apiGet()` reports non-2xx responses, transport errors and bad JSON via `Minz_Log::warning()` (prefixed `[BlueskyThreads]`). Silent nulls make the extension look like it simply renders posts without threads, which is very hard to notice. ## Bluesky API reference diff --git a/extension.php b/extension.php index 1f9758f..1491358 100644 --- a/extension.php +++ b/extension.php @@ -203,19 +203,62 @@ final class BlueskyThreadsExtension extends Minz_Extension { return isset($data['did']) ? (string) $data['did'] : null; } + /** + * Performs a GET against the Bluesky XRPC API and decodes the JSON body. + * + * Uses cURL rather than file_get_contents(): since FreshRSS 1.30.0 every + * stream wrapper except file:// and php:// is unregistered at boot as an + * SSRF mitigation (lib_rss.php, unregister_unsafe_protocols()), so the + * https:// wrapper is simply not available to extensions any more. + * + * Returns null on any failure, logging why — a silent null here means + * posts quietly render without their thread, which is hard to spot. + */ private function apiGet(string $method, array $params): ?array { $url = self::API_BASE . '/' . $method . '?' . http_build_query($params); - $ctx = stream_context_create(['http' => [ - 'timeout' => 10, - 'ignore_errors' => true, - 'header' => "User-Agent: FreshRSS-BlueskyThreads/0.1\r\nAccept: application/json\r\n", - ]]); - $body = @file_get_contents($url, false, $ctx); - if ($body === false) { + + $ch = curl_init($url); + if ($ch === false) { + $this->logWarning("{$method}: could not initialise cURL"); return null; } + + curl_setopt_array($ch, [ + CURLOPT_RETURNTRANSFER => true, + CURLOPT_FOLLOWLOCATION => false, + CURLOPT_CONNECTTIMEOUT => 5, + CURLOPT_TIMEOUT => 10, + CURLOPT_USERAGENT => 'FreshRSS-BlueskyThreads/0.1', + CURLOPT_HTTPHEADER => ['Accept: application/json'], + ]); + + $body = curl_exec($ch); + $status = (int) curl_getinfo($ch, CURLINFO_RESPONSE_CODE); + $error = curl_error($ch); + curl_close($ch); + + if (!is_string($body)) { + $this->logWarning("{$method}: request failed ({$error})"); + return null; + } + if ($status < 200 || $status >= 300) { + $this->logWarning("{$method}: HTTP {$status}"); + return null; + } + $decoded = json_decode($body, true); - return is_array($decoded) ? $decoded : null; + if (!is_array($decoded)) { + $this->logWarning("{$method}: response was not valid JSON"); + return null; + } + return $decoded; + } + + /** Records a non-fatal API problem in the FreshRSS log, when available. */ + private function logWarning(string $message): void { + if (class_exists('Minz_Log')) { + Minz_Log::warning('[BlueskyThreads] ' . $message); + } } // ------------------------------------------------------------------------- diff --git a/tests/BlueskyThreadsTest.php b/tests/BlueskyThreadsTest.php index 01dff6f..cc56fad 100644 --- a/tests/BlueskyThreadsTest.php +++ b/tests/BlueskyThreadsTest.php @@ -20,6 +20,7 @@ class BlueskyThreadsTest extends TestCase { protected function setUp(): void { Minz_Request::reset(); + Minz_Log::reset(); $this->ext = new BlueskyThreadsExtension(); } @@ -323,4 +324,49 @@ class BlueskyThreadsTest extends TestCase { $this->assertSame(1000, $this->ext->getUserConfigurationValue('depth')); } + + // ------------------------------------------------------------------------- + // API transport — must not depend on PHP stream wrappers + // ------------------------------------------------------------------------- + + /** + * Regression test for the FreshRSS 1.30.0 breakage. + * + * 1.30.0 unregisters every stream wrapper except file:// and php:// at + * boot as an SSRF mitigation, which silently killed the previous + * file_get_contents()-based fetch. We reproduce that exact condition and + * assert apiGet() still reaches the API. + */ + public function testApiGetWorksWithoutHttpsStreamWrapper(): void { + if (!in_array('https', stream_get_wrappers(), true)) { + $this->markTestSkipped('No https stream wrapper registered in this PHP build.'); + } + + stream_wrapper_unregister('https'); + try { + $data = $this->call('apiGet', 'com.atproto.identity.resolveHandle', ['handle' => 'hockeyviz.com']); + } finally { + stream_wrapper_restore('https'); + } + + if ($data === null) { + $this->markTestSkipped('Bluesky API unreachable: ' . implode('; ', Minz_Log::messages())); + } + + $this->assertSame('did:plc:d4324t32vfi5xzydqbh2qdj3', $data['did'] ?? null); + } + + /** + * A failed API call must return null *and* leave a trace in the log. + * The original code suppressed errors with @, so this whole failure mode + * was invisible for a day. Passes online (bogus method → HTTP 4xx) and + * offline (connection error) alike. + */ + public function testApiGetLogsWarningOnFailure(): void { + $data = $this->call('apiGet', 'com.example.doesNotExist', []); + + $this->assertNull($data); + $this->assertNotEmpty(Minz_Log::messages(), 'A failed API call must be logged.'); + $this->assertStringContainsString('[BlueskyThreads]', Minz_Log::messages()[0]); + } } diff --git a/tests/stubs/freshrss_stubs.php b/tests/stubs/freshrss_stubs.php index 4751e72..df573e1 100644 --- a/tests/stubs/freshrss_stubs.php +++ b/tests/stubs/freshrss_stubs.php @@ -78,3 +78,25 @@ class Minz_Request { self::$params = []; } } + +class Minz_Log { + /** @var list */ + private static array $messages = []; + + public static function warning(string $message): void { + self::$messages[] = $message; + } + + /** + * Test helper: all warnings recorded since the last reset(). + * + * @return list + */ + public static function messages(): array { + return self::$messages; + } + + public static function reset(): void { + self::$messages = []; + } +}