Skip to content

Commit

Permalink
Close the PHP session before making HTTP requests
Browse files Browse the repository at this point in the history
Resolves #15643
  • Loading branch information
brandonkelly committed Sep 3, 2024
1 parent 48d0bb5 commit c53e361
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 18 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG-WIP.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
- Added `craft\base\ApplicationTrait::getEnvId()`. ([#15313](https://github.com/craftcms/cms/issues/15313))
- Added `craft\base\ElementInterface::getRootOwner()`. ([#15534](https://github.com/craftcms/cms/discussions/15534))
- Added `craft\elements\conditions\SiteGroupConditionRule`.
- Added `craft\helpers\Session::close()`.
- Added `craft\services\Sites::getEditableSitesByGroupId()`.
- `craft\helpers\Session` methods are now safe to call on console requests.
- Deprecated `craft\helpers\ElementHelper::rootElement()`. `craft\base\ElementInterface::getRootOwner()` should be used instead.
- Deprecated `craft\db\mysql\Schema::quoteDatabaseName()`.
- Deprecated `craft\db\pgqsl\Schema::quoteDatabaseName()`.
Expand All @@ -19,4 +21,5 @@
- MySQL mutex locks and PHP session names are now namespaced using the application ID combined with the environment name. ([#15313](https://github.com/craftcms/cms/issues/15313))
- `x-craft-preview` and `x-craft-live-preview` params are now hashed, and `craft\web\Request::getIsPreview()` will only return `true` if the param validates. ([#15605](https://github.com/craftcms/cms/discussions/15605))
- Generated URLs no longer include `x-craft-preview` or `x-craft-live-preview` query string params based on the requested URL, if either were set to an unverified string. ([#15605](https://github.com/craftcms/cms/discussions/15605))
- The PHP session is now closed before making API requests. ([#15643](https://github.com/craftcms/cms/issues/15643))
- Updated Twig to 3.12. ([#15568](https://github.com/craftcms/cms/discussions/15568))
4 changes: 4 additions & 0 deletions src/controllers/AppController.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use craft\helpers\DateTimeHelper;
use craft\helpers\Html;
use craft\helpers\Json;
use craft\helpers\Session;
use craft\helpers\Update as UpdateHelper;
use craft\helpers\UrlHelper;
use craft\models\Update;
Expand Down Expand Up @@ -96,6 +97,9 @@ public function actionResourceJs(string $url): Response
throw new BadRequestHttpException("$url does not appear to be a resource URL");
}

// Close the PHP session in case this takes a while
Session::close();

$response = Craft::createGuzzleClient()->get($url);
$this->response->setCacheHeaders();
$this->response->getHeaders()->set('content-type', 'application/javascript');
Expand Down
48 changes: 31 additions & 17 deletions src/helpers/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
namespace craft\helpers;

use Craft;
use craft\errors\MissingComponentException;
use yii\web\Session as YiiSession;

/**
Expand All @@ -18,6 +19,16 @@
*/
class Session
{
/**
* @see session()
*/
private static YiiSession|false|null $_session;

/**
* @see exists()
*/
private static bool $_exists = false;

/**
* Returns the session variable value with the session variable name.
*
Expand All @@ -42,7 +53,7 @@ public static function get(string $key): mixed
*/
public static function set(string $key, mixed $value): void
{
self::session()->set($key, $value);
self::session()?->set($key, $value);
}

/**
Expand All @@ -61,7 +72,6 @@ public static function remove(string $key): mixed

/**
* Removes all session variables.
*
*/
public static function removeAll(): void
{
Expand All @@ -85,13 +95,27 @@ public static function has(string $key): bool
return self::session()->has($key);
}


/**
* @return YiiSession
* Closes the session, if open.
*
* @since 4.12.0
*/
private static function session(): YiiSession
public static function close(): void
{
self::session()?->close();
}

private static function session(): ?YiiSession
{
return self::$_session ?? (self::$_session = Craft::$app->getSession());
if (!isset(self::$_session)) {
try {
self::$_session = Craft::$app->getSession();
} catch (MissingComponentException) {
self::$_session = false;
}
}

return self::$_session ?: null;
}

/**
Expand All @@ -106,19 +130,9 @@ public static function exists(): bool
}

// Keep re-checking until it does
return self::$_exists = self::session()->getIsActive() || self::session()->getHasSessionId();
return self::$_exists = self::session()?->getIsActive() || self::session()?->getHasSessionId();
}

/**
* @var YiiSession|null
*/
private static ?YiiSession $_session = null;

/**
* @var bool
*/
private static bool $_exists = false;

/**
* Resets the memoized database connection.
*
Expand Down
4 changes: 4 additions & 0 deletions src/services/Api.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use craft\helpers\Api as ApiHelper;
use craft\helpers\ArrayHelper;
use craft\helpers\Json;
use craft\helpers\Session;
use GuzzleHttp\Client;
use GuzzleHttp\Exception\RequestException;
use GuzzleHttp\RequestOptions;
Expand Down Expand Up @@ -118,6 +119,9 @@ public function getCountries(): array
*/
public function request(string $method, string $uri, array $options = []): ResponseInterface
{
// Close the PHP session in case this takes a while
Session::close();

$options = ArrayHelper::merge($options, [
'headers' => ApiHelper::headers(),
]);
Expand Down
4 changes: 4 additions & 0 deletions src/services/Webpack.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use craft\helpers\App;
use craft\helpers\ArrayHelper;
use craft\helpers\FileHelper;
use craft\helpers\Session;
use craft\helpers\StringHelper;
use Exception;
use GuzzleHttp\Exception\GuzzleException;
Expand Down Expand Up @@ -214,6 +215,9 @@ private function _isDevServerRunning(string $class, string $loopback): bool
return $this->_isDevServerRunning[$class] = $this->_matchAsset($this->_serverResponse[$loopback], $class);
}

// Close the PHP session in case this takes a while
Session::close();

// Make sure the request isn't too strict for people running the dev server using https and outside the container
$client = Craft::createGuzzleClient(['verify' => false]);
try {
Expand Down
3 changes: 2 additions & 1 deletion src/web/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use Craft;
use craft\helpers\ArrayHelper;
use craft\helpers\Session;
use craft\helpers\UrlHelper;
use Throwable;
use yii\base\Application as BaseApplication;
Expand Down Expand Up @@ -305,7 +306,7 @@ public function sendAndClose(): void
$this->send();

// Close the session.
Craft::$app->getSession()->close();
Session::close();

// In case we're running on php-fpm (https://secure.php.net/manual/en/book.fpm.php)
if (function_exists('fastcgi_finish_request')) {
Expand Down

0 comments on commit c53e361

Please sign in to comment.