Fix exception when empty default value is passed for an egg variable, closes pterodactyl#934

Author: DaneEveritt

CHANGELOG.md

@@ -10,6 +10,7 @@ This project follows [Semantic Versioning](http://semver.org) guidelines.
 * `[rc.2]` — Fixes bug with server creation API endpoint that would fail to validate `allocation.default` correctly.
 * `[rc.2]` — Fix data integrity exception occuring due to invalid data being passed to server creation service on the API.
 * `[rc.2]` — Fix data integrity exception that could occur when an email containing non-username characters was passed.
+* `[rc.2]` — Fix data integrity exception occurring when no default value is provided for an egg variable.
 
 ### Added
 * Added ability to search the following API endpoints: list users, list servers, and list locations.

app/Services/Eggs/Variables/VariableUpdateService.php

@@ -27,21 +27,17 @@ public function __construct(EggVariableRepositoryInterface $repository)
     /**
      * Update a specific egg variable.
      *
-     * @param int|\Pterodactyl\Models\EggVariable $variable
-     * @param array                               $data
+     * @param \Pterodactyl\Models\EggVariable $variable
+     * @param array                           $data
      * @return mixed
      *
      * @throws \Pterodactyl\Exceptions\DisplayException
      * @throws \Pterodactyl\Exceptions\Model\DataValidationException
      * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
      * @throws \Pterodactyl\Exceptions\Service\Egg\Variable\ReservedVariableNameException
      */
-    public function handle($variable, array $data)
+    public function handle(EggVariable $variable, array $data)
     {
-        if (! $variable instanceof EggVariable) {
-            $variable = $this->repository->find($variable);
-        }
-
         if (! is_null(array_get($data, 'env_variable'))) {
             if (in_array(strtoupper(array_get($data, 'env_variable')), explode(',', EggVariable::RESERVED_ENV_NAMES))) {
                 throw new ReservedVariableNameException(trans('exceptions.service.variables.reserved_name', [
@@ -65,6 +61,7 @@ public function handle($variable, array $data)
         $options = array_get($data, 'options') ?? [];
 
         return $this->repository->withoutFreshModel()->update($variable->id, array_merge($data, [
+            'default_value' => array_get($data, 'default_value') ?? '',
             'user_viewable' => in_array('user_viewable', $options),
             'user_editable' => in_array('user_editable', $options),
         ]));

tests/Unit/Services/Eggs/Variables/VariableUpdateServiceTest.php

@@ -46,29 +46,30 @@ public function setUp()
     public function testVariableIsUpdatedWhenNoEnvironmentVariableIsPassed()
     {
         $this->repository->shouldReceive('withoutFreshModel')->withNoArgs()->once()->andReturnSelf()
-            ->shouldReceive('update')->with($this->model->id, [
+            ->shouldReceive('update')->with($this->model->id, m::subset([
                 'user_viewable' => false,
                 'user_editable' => false,
                 'test-data' => 'test-value',
-            ])->once()->andReturn(true);
+            ]))->once()->andReturn(true);
 
         $this->assertTrue($this->service->handle($this->model, ['test-data' => 'test-value']));
     }
 
     /**
-     * Test that a service variable ID can be passed in place of the model.
+     * Test that a null value passed in for the default is converted to a string.
+     *
+     * @see https://github.com/Pterodactyl/Panel/issues/934
      */
-    public function testVariableIdCanBePassedInPlaceOfModel()
+    public function testNullDefaultValue()
     {
-        $this->repository->shouldReceive('find')->with($this->model->id)->once()->andReturn($this->model);
         $this->repository->shouldReceive('withoutFreshModel')->withNoArgs()->once()->andReturnSelf()
             ->shouldReceive('update')->with($this->model->id, [
                 'user_viewable' => false,
                 'user_editable' => false,
-                'test-data' => 'test-value',
+                'default_value' => '',
             ])->once()->andReturn(true);
 
-        $this->assertTrue($this->service->handle($this->model->id, ['test-data' => 'test-value']));
+        $this->assertTrue($this->service->handle($this->model, ['default_value' => null]));
     }
 
     /**
@@ -84,11 +85,11 @@ public function testVariableIsUpdatedWhenValidEnvironmentVariableIsPassed()
             ])->once()->andReturn(0);
 
         $this->repository->shouldReceive('withoutFreshModel')->withNoArgs()->once()->andReturnSelf()
-            ->shouldReceive('update')->with($this->model->id, [
+            ->shouldReceive('update')->with($this->model->id, m::subset([
                 'user_viewable' => false,
                 'user_editable' => false,
                 'env_variable' => 'TEST_VAR_123',
-            ])->once()->andReturn(true);
+            ]))->once()->andReturn(true);
 
         $this->assertTrue($this->service->handle($this->model, ['env_variable' => 'TEST_VAR_123']));
     }
@@ -102,11 +103,11 @@ public function testVariableIsUpdatedWhenValidEnvironmentVariableIsPassed()
     public function testNullOptionValueIsPassedAsArray()
     {
         $this->repository->shouldReceive('withoutFreshModel')->withNoArgs()->once()->andReturnSelf()
-            ->shouldReceive('update')->with($this->model->id, [
+            ->shouldReceive('update')->with($this->model->id, m::subset([
                 'user_viewable' => false,
                 'user_editable' => false,
                 'options' => null,
-            ])->once()->andReturn(true);
+            ]))->once()->andReturn(true);
 
         $this->assertTrue($this->service->handle($this->model, ['options' => null]));
     }
@@ -124,11 +125,11 @@ public function testDataPassedIntoHandlerTakesLowerPriorityThanDataSet()
             ])->once()->andReturn(0);
 
         $this->repository->shouldReceive('withoutFreshModel')->withNoArgs()->once()->andReturnSelf()
-            ->shouldReceive('update')->with($this->model->id, [
+            ->shouldReceive('update')->with($this->model->id, m::subset([
                 'user_viewable' => false,
                 'user_editable' => false,
                 'env_variable' => 'TEST_VAR_123',
-            ])->once()->andReturn(true);
+            ]))->once()->andReturn(true);
 
         $this->assertTrue($this->service->handle($this->model, ['user_viewable' => 123456, 'env_variable' => 'TEST_VAR_123']));
     }