From 114afb86461b13d20c5bf5dcb3497721af43d475 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 2 Mar 2019 18:28:28 -0800 Subject: [PATCH] Fix error transaction handling when creating a server. There is a bug in the design of the application that affects users who encounter an exception under certain code pathways who are using the database to maintain their sessions. What is happening is that a transaction is started, and I made the mistake of just assuming it would auto-rollback once the exception was caught by the handler. This is technically true, since once the request terminates the transaction is discarded by the SQL server. However, this also means that the session data set on that request would not be persisted as it runs in a middleware termination function, after the transaction is started. Theoretically this would also affect any other terminable middleware as well, but the session is the only one I can think of right now Co-Authored-By: Oreo Oreoniv Co-Authored-By: Stepan Fedotov --- CHANGELOG.md | 2 ++ app/Exceptions/Handler.php | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 200e0b345..af079ff9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ This project follows [Semantic Versioning](http://semver.org) guidelines. * Fixes bug where node creation API endpoint was not correctly requiring the `disk_overallocate` key. * Prevents an exception from being thrown when a database with the same name is created on two different hosts. * Fixes the redis password not saving correctly when setting up the environment from the command line. +* Fixes a bug with transaction handling in many areas of the application that would cause validation error messages +and other session data to not be persisted properly when using the database as the session driver. ### Changed * `allocation_limit` for servers now defaults to a null value, and is not required in PATCH/POST requests when adding diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 9be53eb4f..b74a71ea7 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -6,6 +6,7 @@ use Exception; use PDOException; use Psr\Log\LoggerInterface; use Illuminate\Container\Container; +use Illuminate\Database\Connection; use Illuminate\Auth\AuthenticationException; use Illuminate\Session\TokenMismatchException; use Illuminate\Validation\ValidationException; @@ -137,6 +138,21 @@ class Handler extends ExceptionHandler */ public function render($request, Exception $exception) { + $connections = Container::getInstance()->make(Connection::class); + + // If we are currently wrapped up inside a transaction, we will roll all the way + // back to the beginning. This needs to happen, otherwise session data does not + // get properly persisted. + // + // This is kind of a hack, and ideally things like this should be handled as + // much as possible at the code level, but there are a lot of spots that do a + // ton of actions and were written before this bug discovery was made. + // + // @see https://github.com/pterodactyl/panel/pull/1468 + if ($connections->transactionLevel()) { + $connections->rollBack(0); + } + return parent::render($request, $exception); }