Use cURL for Bluesky API calls instead of file_get_contents
CI / test (pull_request) Successful in 40s
CI / test (pull_request) Successful in 40s
FreshRSS 1.30.0 added unregister_unsafe_protocols() to lib/lib_rss.php, which unregisters every PHP stream wrapper except file:// and php:// at boot as an SSRF mitigation. That removed the https:// wrapper, so the stream-context-based fetch in apiGet() stopped working entirely — every API call failed with "Unable to find the wrapper https". Because the call site used @file_get_contents(), the failure was silent: apiGet() returned null, loadThread() fell through to its null fallback, and entries were saved with their unenriched RSS content. The visible symptom was Bluesky posts rendering with no thread and no embeds, with nothing in the logs to explain it. Switch apiGet() to cURL, which does not go through stream wrappers (and is how FreshRSS fetches feeds itself, which is why feed retrieval kept working throughout). Connect timeout added and redirect-following explicitly disabled — a public XRPC endpoint has no reason to redirect, and following redirects would reopen an SSRF vector. Also stop swallowing failures: transport errors, non-2xx responses and malformed JSON are now reported via Minz_Log::warning() so this class of breakage is visible next time. Tests: add testApiGetWorksWithoutHttpsStreamWrapper, which unregisters the https wrapper before calling the API and so fails against the old implementation, plus testApiGetLogsWarningOnFailure. Add a Minz_Log stub and curl to the CI extension list. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
@@ -16,7 +16,7 @@ jobs:
|
|||||||
uses: shivammathur/setup-php@v2
|
uses: shivammathur/setup-php@v2
|
||||||
with:
|
with:
|
||||||
php-version: '8.2'
|
php-version: '8.2'
|
||||||
extensions: json, simplexml
|
extensions: json, simplexml, curl
|
||||||
coverage: none
|
coverage: none
|
||||||
|
|
||||||
- name: Install dependencies
|
- name: Install dependencies
|
||||||
|
|||||||
@@ -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.
|
- **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.
|
- **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.
|
- **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
|
## Bluesky API reference
|
||||||
|
|
||||||
|
|||||||
+51
-8
@@ -203,19 +203,62 @@ final class BlueskyThreadsExtension extends Minz_Extension {
|
|||||||
return isset($data['did']) ? (string) $data['did'] : null;
|
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 {
|
private function apiGet(string $method, array $params): ?array {
|
||||||
$url = self::API_BASE . '/' . $method . '?' . http_build_query($params);
|
$url = self::API_BASE . '/' . $method . '?' . http_build_query($params);
|
||||||
$ctx = stream_context_create(['http' => [
|
|
||||||
'timeout' => 10,
|
$ch = curl_init($url);
|
||||||
'ignore_errors' => true,
|
if ($ch === false) {
|
||||||
'header' => "User-Agent: FreshRSS-BlueskyThreads/0.1\r\nAccept: application/json\r\n",
|
$this->logWarning("{$method}: could not initialise cURL");
|
||||||
]]);
|
|
||||||
$body = @file_get_contents($url, false, $ctx);
|
|
||||||
if ($body === false) {
|
|
||||||
return null;
|
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);
|
$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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// -------------------------------------------------------------------------
|
// -------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ class BlueskyThreadsTest extends TestCase {
|
|||||||
|
|
||||||
protected function setUp(): void {
|
protected function setUp(): void {
|
||||||
Minz_Request::reset();
|
Minz_Request::reset();
|
||||||
|
Minz_Log::reset();
|
||||||
$this->ext = new BlueskyThreadsExtension();
|
$this->ext = new BlueskyThreadsExtension();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -323,4 +324,49 @@ class BlueskyThreadsTest extends TestCase {
|
|||||||
|
|
||||||
$this->assertSame(1000, $this->ext->getUserConfigurationValue('depth'));
|
$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]);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -78,3 +78,25 @@ class Minz_Request {
|
|||||||
self::$params = [];
|
self::$params = [];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
class Minz_Log {
|
||||||
|
/** @var list<string> */
|
||||||
|
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<string>
|
||||||
|
*/
|
||||||
|
public static function messages(): array {
|
||||||
|
return self::$messages;
|
||||||
|
}
|
||||||
|
|
||||||
|
public static function reset(): void {
|
||||||
|
self::$messages = [];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user