fix: return user-friendly error messages instead of raw exceptions #35
Conversation
- Handle HTTP errors (403, 404, timeouts, etc.) gracefully - Return translated 'site_cannot_be_reached' message instead of exposing raw Guzzle exception messages with HTML content - Fixes issue where Cloudflare-protected sites showed ugly error messages
There was a problem hiding this comment.
Pull request overview
This pull request improves error handling in the link preview service by returning user-friendly translated error messages instead of exposing raw Guzzle exception messages that could contain HTML content. Additionally, it enhances HTTP client configuration to better handle various websites.
Key changes:
- Enhanced HTTP client with increased timeouts, browser-like headers, redirect handling, and cookie support
- Added specific exception handling for RequestException with translated error messages
- Standardized error responses across all exception handlers to use the translated message
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Services/LinkPreviewService.php | Enhanced HTTP client configuration with longer timeouts, comprehensive browser headers, redirect support, and cookie handling to improve compatibility with various websites |
| src/Api/Controllers/SingleLinkPreviewController.php | Added RequestException catch block and updated generic exception handler to return translated user-friendly error message |
| src/Api/Controllers/BatchLinkPreviewController.php | Updated all exception handlers to return translated user-friendly error messages instead of exposing raw exception details |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'track_redirects' => true, | ||
| ], | ||
| 'headers' => [ | ||
| 'User-Agent' => 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36', |
There was a problem hiding this comment.
The User-Agent string contains a hardcoded Chrome version (120.0.0.0) that will become outdated over time. Consider using a more generic or configurable User-Agent string to avoid maintenance issues and potential detection as an outdated client.
| namespace Datlechin\LinkPreview\Api\Controllers; | ||
|
|
||
| use Datlechin\LinkPreview\Services\LinkPreviewService; | ||
| use GuzzleHttp\Exception\RequestException; |
There was a problem hiding this comment.
The RequestException class is imported but not used in this file. The generic Throwable catch block at line 25 handles all exceptions including RequestException. Consider removing this unused import or using it to provide more specific error handling if different error messages are needed for HTTP request failures versus other types of exceptions.
| use GuzzleHttp\Exception\RequestException; |
| 'connect_timeout' => 5, | ||
| 'timeout' => 15, | ||
| 'connect_timeout' => 10, | ||
| 'verify' => false, |
There was a problem hiding this comment.
The Guzzle Client is created with verify => false, which completely disables TLS certificate validation for all HTTPS link preview requests. An attacker on the network path can impersonate any HTTPS site being previewed, inject arbitrary HTML/metadata into the response, or snoop on which URLs are being fetched despite HTTPS. Re-enable certificate verification (or make it configurable and only disabled for trusted development environments) so remote hosts are properly authenticated when fetching previews.
|
|
||
| foreach ($urlsToFetch as $originalUrl => $normalizedUrl) { | ||
| try { | ||
| $promises[$originalUrl] = $this->service->getClient()->getAsync($originalUrl); |
There was a problem hiding this comment.
BatchLinkPreviewController::fetchUrls passes user-supplied URLs (from the urls request body) into getClient()->getAsync($originalUrl), allowing attackers to make the server issue arbitrary HTTP(S) requests on their behalf. Since isValidUrl/isUrlAllowed only check syntax, DNS resolution, and optional lists, but do not forbid internal hostnames or private network targets, this endpoint exposes a high-impact SSRF vector against internal services accessible from the server. Introduce robust host validation (e.g., blocking private/internal ranges and restricting to a safe allowlist) before scheduling these asynchronous requests.
Fix for v1.6.0 issue where HTTP errors exposed raw Guzzle exception messages with HTML content. Now returns translated site_cannot_be_reached message instead.