BookStackApp · ssddanbrown · Sep 8, 2023 · Sep 8, 2023 · Sep 8, 2023
diff --git a/app/Access/Oidc/OidcOAuthProvider.php b/app/Access/Oidc/OidcOAuthProvider.php
@@ -20,15 +20,8 @@ class OidcOAuthProvider extends AbstractProvider
 {
     use BearerAuthorizationTrait;
 
-    /**
-     * @var string
-     */
-    protected $authorizationEndpoint;
-
-    /**
-     * @var string
-     */
-    protected $tokenEndpoint;
+    protected string $authorizationEndpoint;
+    protected string $tokenEndpoint;
 
     /**
      * Scopes to use for the OIDC authorization call.
@@ -60,7 +53,7 @@ public function getResourceOwnerDetailsUrl(AccessToken $token): string
     }
 
     /**
-     * Add an additional scope to this provider upon the default.
+     * Add another scope to this provider upon the default.
      */
     public function addScope(string $scope): void
     {

diff --git a/app/Access/Oidc/OidcProviderSettings.php b/app/Access/Oidc/OidcProviderSettings.php
@@ -59,7 +59,7 @@ protected function validateInitial()
             }
         }
 
-        if (strpos($this->issuer, 'https://') !== 0) {
+        if (!str_starts_with($this->issuer, 'https://')) {
             throw new InvalidArgumentException('Issuer value must start with https://');
         }
     }

diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php
@@ -9,13 +9,13 @@
 use BookStack\Exceptions\StoppedAuthenticationException;
 use BookStack\Exceptions\UserRegistrationException;
 use BookStack\Facades\Theme;
+use BookStack\Http\HttpRequestService;
 use BookStack\Theming\ThemeEvents;
 use BookStack\Users\Models\User;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Facades\Cache;
 use League\OAuth2\Client\OptionProvider\HttpBasicAuthOptionProvider;
 use League\OAuth2\Client\Provider\Exception\IdentityProviderException;
-use Psr\Http\Client\ClientInterface as HttpClient;
 
 /**
  * Class OpenIdConnectService
@@ -26,7 +26,7 @@ class OidcService
     public function __construct(
         protected RegistrationService $registrationService,
         protected LoginService $loginService,
-        protected HttpClient $httpClient,
+        protected HttpRequestService $http,
         protected GroupSyncService $groupService
     ) {
     }
@@ -94,7 +94,7 @@ protected function getProviderSettings(): OidcProviderSettings
         // Run discovery
         if ($config['discover'] ?? false) {
             try {
-                $settings->discoverFromIssuer($this->httpClient, Cache::store(null), 15);
+                $settings->discoverFromIssuer($this->http->buildClient(5), Cache::store(null), 15);
             } catch (OidcIssuerDiscoveryException $exception) {
                 throw new OidcException('OIDC Discovery Error: ' . $exception->getMessage());
             }
@@ -111,7 +111,7 @@ protected function getProviderSettings(): OidcProviderSettings
     protected function getProvider(OidcProviderSettings $settings): OidcOAuthProvider
     {
         $provider = new OidcOAuthProvider($settings->arrayForProvider(), [
-            'httpClient'     => $this->httpClient,
+            'httpClient'     => $this->http->buildClient(5),
             'optionProvider' => new HttpBasicAuthOptionProvider(),
         ]);
 

diff --git a/app/Activity/DispatchWebhookJob.php b/app/Activity/DispatchWebhookJob.php
@@ -6,6 +6,7 @@
 use BookStack\Activity\Models\Webhook;
 use BookStack\Activity\Tools\WebhookFormatter;
 use BookStack\Facades\Theme;
+use BookStack\Http\HttpRequestService;
 use BookStack\Theming\ThemeEvents;
 use BookStack\Users\Models\User;
 use BookStack\Util\SsrUrlValidator;
@@ -14,7 +15,6 @@
 use Illuminate\Foundation\Bus\Dispatchable;
 use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Queue\SerializesModels;
-use Illuminate\Support\Facades\Http;
 use Illuminate\Support\Facades\Log;
 
 class DispatchWebhookJob implements ShouldQueue
@@ -49,25 +49,28 @@ public function __construct(Webhook $webhook, string $event, Loggable|string $de
      *
      * @return void
      */
-    public function handle()
+    public function handle(HttpRequestService $http)
     {
         $lastError = null;
 
         try {
             (new SsrUrlValidator())->ensureAllowed($this->webhook->endpoint);
 
-            $response = Http::asJson()
-                ->withOptions(['allow_redirects' => ['strict' => true]])
-                ->timeout($this->webhook->timeout)
-                ->post($this->webhook->endpoint, $this->webhookData);
-        } catch (\Exception $exception) {
-            $lastError = $exception->getMessage();
-            Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with error \"{$lastError}\"");
-        }
+            $client = $http->buildClient($this->webhook->timeout, [
+                'connect_timeout' => 10,
+                'allow_redirects' => ['strict' => true],
+            ]);
 
-        if (isset($response) && $response->failed()) {
-            $lastError = "Response status from endpoint was {$response->status()}";
-            Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with status {$response->status()}");
+            $response = $client->sendRequest($http->jsonRequest('POST', $this->webhook->endpoint, $this->webhookData));
+            $statusCode = $response->getStatusCode();
+
+            if ($statusCode >= 400) {
+                $lastError = "Response status from endpoint was {$statusCode}";
+                Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with status {$statusCode}");
+            }
+        } catch (\Exception $error) {
+            $lastError = $error->getMessage();
+            Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with error \"{$lastError}\"");
         }
 
         $this->webhook->last_called_at = now();

diff --git a/app/App/Providers/AppServiceProvider.php b/app/App/Providers/AppServiceProvider.php
@@ -9,16 +9,15 @@
 use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Page;
 use BookStack\Exceptions\BookStackExceptionHandlerPage;
+use BookStack\Http\HttpRequestService;
 use BookStack\Permissions\PermissionApplicator;
 use BookStack\Settings\SettingService;
 use BookStack\Util\CspService;
-use GuzzleHttp\Client;
 use Illuminate\Contracts\Foundation\ExceptionRenderer;
 use Illuminate\Database\Eloquent\Relations\Relation;
 use Illuminate\Support\Facades\Schema;
 use Illuminate\Support\Facades\URL;
 use Illuminate\Support\ServiceProvider;
-use Psr\Http\Client\ClientInterface as HttpClientInterface;
 
 class AppServiceProvider extends ServiceProvider
 {
@@ -39,6 +38,7 @@ class AppServiceProvider extends ServiceProvider
         SettingService::class => SettingService::class,
         SocialAuthService::class => SocialAuthService::class,
         CspService::class => CspService::class,
+        HttpRequestService::class => HttpRequestService::class,
     ];
 
     /**
@@ -51,7 +51,7 @@ public function boot()
         // Set root URL
         $appUrl = config('app.url');
         if ($appUrl) {
-            $isHttps = (strpos($appUrl, 'https://') === 0);
+            $isHttps = str_starts_with($appUrl, 'https://');
             URL::forceRootUrl($appUrl);
             URL::forceScheme($isHttps ? 'https' : 'http');
         }
@@ -75,12 +75,6 @@ public function boot()
      */
     public function register()
     {
-        $this->app->bind(HttpClientInterface::class, function ($app) {
-            return new Client([
-                'timeout' => 3,
-            ]);
-        });
-
         $this->app->singleton(PermissionApplicator::class, function ($app) {
             return new PermissionApplicator(null);
         });

diff --git a/app/Http/HttpClientHistory.php b/app/Http/HttpClientHistory.php
@@ -0,0 +1,33 @@
+<?php
+
+namespace BookStack\Http;
+
+use GuzzleHttp\Psr7\Request as GuzzleRequest;
+
+class HttpClientHistory
+{
+    public function __construct(
+        protected &$container
+    ) {
+    }
+
+    public function requestCount(): int
+    {
+        return count($this->container);
+    }
+
+    public function requestAt(int $index): ?GuzzleRequest
+    {
+        return $this->container[$index]['request'] ?? null;
+    }
+
+    public function latestRequest(): ?GuzzleRequest
+    {
+        return $this->requestAt($this->requestCount() - 1);
+    }
+
+    public function all(): array
+    {
+        return $this->container;
+    }
+}
diff --git a/app/Http/HttpRequestService.php b/app/Http/HttpRequestService.php
@@ -0,0 +1,70 @@
+<?php
+
+namespace BookStack\Http;
+
+use GuzzleHttp\Client;
+use GuzzleHttp\Handler\MockHandler;
+use GuzzleHttp\HandlerStack;
+use GuzzleHttp\Middleware;
+use GuzzleHttp\Psr7\Request as GuzzleRequest;
+use GuzzleHttp\Psr7\Response;
+use Psr\Http\Client\ClientInterface;
+
+class HttpRequestService
+{
+    protected ?HandlerStack $handler = null;
+
+    /**
+     * Build a new http client for sending requests on.
+     */
+    public function buildClient(int $timeout, array $options = []): ClientInterface
+    {
+        $defaultOptions = [
+            'timeout' => $timeout,
+            'handler' => $this->handler,
+        ];
+
+        return new Client(array_merge($options, $defaultOptions));
+    }
+
+    /**
+     * Create a new JSON http request for use with a client.
+     */
+    public function jsonRequest(string $method, string $uri, array $data): GuzzleRequest
+    {
+        $headers = ['Content-Type' => 'application/json'];
+        return new GuzzleRequest($method, $uri, $headers, json_encode($data));
+    }
+
+    /**
+     * Mock any http clients built from this service, and response with the given responses.
+     * Returns history which can then be queried.
+     * @link https://docs.guzzlephp.org/en/stable/testing.html#history-middleware
+     */
+    public function mockClient(array $responses = [], bool $pad = true): HttpClientHistory
+    {
+        // By default, we pad out the responses with 10 successful values so that requests will be
+        // properly recorded for inspection. Otherwise, we can't later check if we're received
+        // too many requests.
+        if ($pad) {
+            $response = new Response(200, [], 'success');
+            $responses = array_merge($responses, array_fill(0, 10, $response));
+        }
+
+        $container = [];
+        $history = Middleware::history($container);
+        $mock = new MockHandler($responses);
+        $this->handler = HandlerStack::create($mock);
+        $this->handler->push($history, 'history');
+
+        return new HttpClientHistory($container);
+    }
+
+    /**
+     * Clear mocking that has been set up for clients.
+     */
+    public function clearMocking(): void
+    {
+        $this->handler = null;
+    }
+}
diff --git a/app/Uploads/HttpFetcher.php b/app/Uploads/HttpFetcher.php
diff --git a/app/Uploads/UserAvatars.php b/app/Uploads/UserAvatars.php
@@ -3,20 +3,20 @@
 namespace BookStack\Uploads;
 
 use BookStack\Exceptions\HttpFetchException;
+use BookStack\Http\HttpRequestService;
 use BookStack\Users\Models\User;
 use Exception;
+use GuzzleHttp\Psr7\Request;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Str;
+use Psr\Http\Client\ClientExceptionInterface;
 
 class UserAvatars
 {
-    protected $imageService;
-    protected $http;
-
-    public function __construct(ImageService $imageService, HttpFetcher $http)
-    {
-        $this->imageService = $imageService;
-        $this->http = $http;
+    public function __construct(
+        protected ImageService $imageService,
+        protected HttpRequestService $http
+    ) {
     }
 
     /**
@@ -112,8 +112,10 @@ protected function createAvatarImageFromData(User $user, string $imageData, stri
     protected function getAvatarImageData(string $url): string
     {
         try {
-            $imageData = $this->http->fetch($url);
-        } catch (HttpFetchException $exception) {
+            $client = $this->http->buildClient(5);
+            $response = $client->sendRequest(new Request('GET', $url));
+            $imageData = (string) $response->getBody();
+        } catch (ClientExceptionInterface $exception) {
             throw new HttpFetchException(trans('errors.cannot_get_image_from_url', ['url' => $url]), $exception->getCode(), $exception);
         }
 
@@ -127,7 +129,7 @@ protected function avatarFetchEnabled(): bool
     {
         $fetchUrl = $this->getAvatarUrl();
 
-        return is_string($fetchUrl) && strpos($fetchUrl, 'http') === 0;
+        return str_starts_with($fetchUrl, 'http');
     }
 
     /**
-Original file line number
+Diff line change
@@ Expand Up / @@ -59,7 +59,7 @@ protected function validateInitial() @@
                 }
             }
-            if (strpos($this->issuer, 'https://') !== 0) {
+            if (!str_starts_with($this->issuer, 'https://')) {
                 throw new InvalidArgumentException('Issuer value must start with https://');
             }
         }
@@ Expand Down @@