Skip to content

Commit 114afb8

Browse files
DaneEverittzKoz210TrixterTheTux
committed
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 <zkoz210@users.noreply.github.com> Co-Authored-By: Stepan Fedotov <trixterthetux@users.noreply.github.com>
1 parent ca193de commit 114afb8

File tree

2 files changed

+18
-0
lines changed

2 files changed

+18
-0
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ This project follows [Semantic Versioning](http://semver.org) guidelines.
99
* Fixes bug where node creation API endpoint was not correctly requiring the `disk_overallocate` key.
1010
* Prevents an exception from being thrown when a database with the same name is created on two different hosts.
1111
* Fixes the redis password not saving correctly when setting up the environment from the command line.
12+
* Fixes a bug with transaction handling in many areas of the application that would cause validation error messages
13+
and other session data to not be persisted properly when using the database as the session driver.
1214

1315
### Changed
1416
* `allocation_limit` for servers now defaults to a null value, and is not required in PATCH/POST requests when adding

app/Exceptions/Handler.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use PDOException;
77
use Psr\Log\LoggerInterface;
88
use Illuminate\Container\Container;
9+
use Illuminate\Database\Connection;
910
use Illuminate\Auth\AuthenticationException;
1011
use Illuminate\Session\TokenMismatchException;
1112
use Illuminate\Validation\ValidationException;
@@ -137,6 +138,21 @@ class_basename($exception),
137138
*/
138139
public function render($request, Exception $exception)
139140
{
141+
$connections = Container::getInstance()->make(Connection::class);
142+
143+
// If we are currently wrapped up inside a transaction, we will roll all the way
144+
// back to the beginning. This needs to happen, otherwise session data does not
145+
// get properly persisted.
146+
//
147+
// This is kind of a hack, and ideally things like this should be handled as
148+
// much as possible at the code level, but there are a lot of spots that do a
149+
// ton of actions and were written before this bug discovery was made.
150+
//
151+
// @see https://github.com/pterodactyl/panel/pull/1468
152+
if ($connections->transactionLevel()) {
153+
$connections->rollBack(0);
154+
}
155+
140156
return parent::render($request, $exception);
141157
}
142158

0 commit comments

Comments
 (0)