From 51344572f2bbc31f4050d0f534b3fd521f5f8414 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Wed, 5 Aug 2020 14:38:51 -0500 Subject: [PATCH 01/10] fix casing issue with guarded --- .../Database/Eloquent/Concerns/GuardsAttributes.php | 2 +- tests/Integration/Database/EloquentModelTest.php | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php index 96317cf571bf..bf104ff51fbd 100644 --- a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php +++ b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php @@ -163,7 +163,7 @@ public function isFillable($key) */ public function isGuarded($key) { - return in_array($key, $this->getGuarded()) || $this->getGuarded() == ['*']; + return $this->getGuarded() == ['*'] || ! empty(preg_grep('/^'.preg_quote($key).'$/i', $this->getGuarded())); } /** diff --git a/tests/Integration/Database/EloquentModelTest.php b/tests/Integration/Database/EloquentModelTest.php index 143681c545ab..088ebfa8a89d 100644 --- a/tests/Integration/Database/EloquentModelTest.php +++ b/tests/Integration/Database/EloquentModelTest.php @@ -26,6 +26,15 @@ public function setUp() }); } + public function test_cant_update_guarded_attributes_using_different_casing() + { + $model = new TestModel2; + + $model->fill(['ID' => 123]); + + $this->assertNull($model->ID); + } + public function test_user_can_update_nullable_date() { $user = TestModel1::create([ From 049aa04d842f210bf7d5400ade2e9911f84e470e Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 6 Aug 2020 08:01:41 -0500 Subject: [PATCH 02/10] block json mass assignment --- .../Database/Eloquent/Concerns/GuardsAttributes.php | 4 ++++ tests/Integration/Database/EloquentModelTest.php | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php index bf104ff51fbd..10b772b782fa 100644 --- a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php +++ b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php @@ -163,6 +163,10 @@ public function isFillable($key) */ public function isGuarded($key) { + if (strpos($key, '->')) { + $key = Str::before($key, '->'); + } + return $this->getGuarded() == ['*'] || ! empty(preg_grep('/^'.preg_quote($key).'$/i', $this->getGuarded())); } diff --git a/tests/Integration/Database/EloquentModelTest.php b/tests/Integration/Database/EloquentModelTest.php index 088ebfa8a89d..634c51813992 100644 --- a/tests/Integration/Database/EloquentModelTest.php +++ b/tests/Integration/Database/EloquentModelTest.php @@ -35,6 +35,15 @@ public function test_cant_update_guarded_attributes_using_different_casing() $this->assertNull($model->ID); } + public function test_cant_update_guarded_attribute_using_json() + { + $model = new TestModel2; + + $model->fill(['id->foo' => 123]); + + $this->assertNull($model->id); + } + public function test_user_can_update_nullable_date() { $user = TestModel1::create([ From ff7119e216a9b25bc8f6029daf48477586be2109 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 6 Aug 2020 08:02:01 -0500 Subject: [PATCH 03/10] formatting --- src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php index 10b772b782fa..96664a968b52 100644 --- a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php +++ b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php @@ -167,7 +167,8 @@ public function isGuarded($key) $key = Str::before($key, '->'); } - return $this->getGuarded() == ['*'] || ! empty(preg_grep('/^'.preg_quote($key).'$/i', $this->getGuarded())); + return $this->getGuarded() == ['*'] || + ! empty(preg_grep('/^'.preg_quote($key).'$/i', $this->getGuarded())); } /** From 359232777f281e1057c647157829523e18c26da6 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 6 Aug 2020 08:14:29 -0500 Subject: [PATCH 04/10] add boolean --- src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php index 96664a968b52..49e6a8153773 100644 --- a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php +++ b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php @@ -163,7 +163,7 @@ public function isFillable($key) */ public function isGuarded($key) { - if (strpos($key, '->')) { + if (strpos($key, '->') !== false) { $key = Str::before($key, '->'); } From d6e493192c7ce70d9bbf5d54af15866c11d24676 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 6 Aug 2020 09:38:05 -0500 Subject: [PATCH 05/10] protect table names and guarded --- src/Illuminate/Database/Eloquent/Model.php | 12 +++++++++++- tests/Integration/Database/EloquentModelTest.php | 9 +++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Database/Eloquent/Model.php b/src/Illuminate/Database/Eloquent/Model.php index 600eda80c64b..76df7663e18d 100644 --- a/src/Illuminate/Database/Eloquent/Model.php +++ b/src/Illuminate/Database/Eloquent/Model.php @@ -4,6 +4,7 @@ use Exception; use ArrayAccess; +use LogicException; use JsonSerializable; use Illuminate\Support\Arr; use Illuminate\Support\Str; @@ -272,7 +273,16 @@ public function qualifyColumn($column) */ protected function removeTableFromKey($key) { - return Str::contains($key, '.') ? last(explode('.', $key)) : $key; + if (strpos($key, '.') !== false) { + if (! empty($this->getGuarded()) && + $this->getGuarded() !== ['*']) { + throw new LogicException("Mass assignment of Eloquent attributes including table names is unsafe when guarding attributes."); + } + + return last(explode('.', $key)); + } + + return $key; } /** diff --git a/tests/Integration/Database/EloquentModelTest.php b/tests/Integration/Database/EloquentModelTest.php index 634c51813992..3905d0badffa 100644 --- a/tests/Integration/Database/EloquentModelTest.php +++ b/tests/Integration/Database/EloquentModelTest.php @@ -44,6 +44,15 @@ public function test_cant_update_guarded_attribute_using_json() $this->assertNull($model->id); } + public function test_cant_mass_fill_attributes_with_table_names_when_using_guarded() + { + $this->expectException(\LogicException::class); + + $model = new TestModel2; + + $model->fill(['foo.bar' => 123]); + } + public function test_user_can_update_nullable_date() { $user = TestModel1::create([ From 0423b4591fe076edaeae294cced3ad04a6e20151 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 6 Aug 2020 09:39:11 -0500 Subject: [PATCH 06/10] Apply fixes from StyleCI (#33772) --- src/Illuminate/Database/Eloquent/Model.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Database/Eloquent/Model.php b/src/Illuminate/Database/Eloquent/Model.php index 76df7663e18d..add67e627417 100644 --- a/src/Illuminate/Database/Eloquent/Model.php +++ b/src/Illuminate/Database/Eloquent/Model.php @@ -276,7 +276,7 @@ protected function removeTableFromKey($key) if (strpos($key, '.') !== false) { if (! empty($this->getGuarded()) && $this->getGuarded() !== ['*']) { - throw new LogicException("Mass assignment of Eloquent attributes including table names is unsafe when guarding attributes."); + throw new LogicException('Mass assignment of Eloquent attributes including table names is unsafe when guarding attributes.'); } return last(explode('.', $key)); From a92c3ee2068ce510dd82d28e3031b994332e9f80 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 6 Aug 2020 09:54:27 -0500 Subject: [PATCH 07/10] dont allow mass filling with table names --- .../Database/Eloquent/Concerns/GuardsAttributes.php | 1 + src/Illuminate/Database/Eloquent/Model.php | 9 --------- tests/Integration/Database/EloquentModelTest.php | 4 ++-- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php index 49e6a8153773..e261e1ff7d03 100644 --- a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php +++ b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php @@ -152,6 +152,7 @@ public function isFillable($key) } return empty($this->getFillable()) && + strpos($key, '.') === false && ! Str::startsWith($key, '_'); } diff --git a/src/Illuminate/Database/Eloquent/Model.php b/src/Illuminate/Database/Eloquent/Model.php index add67e627417..667d36c757be 100644 --- a/src/Illuminate/Database/Eloquent/Model.php +++ b/src/Illuminate/Database/Eloquent/Model.php @@ -273,15 +273,6 @@ public function qualifyColumn($column) */ protected function removeTableFromKey($key) { - if (strpos($key, '.') !== false) { - if (! empty($this->getGuarded()) && - $this->getGuarded() !== ['*']) { - throw new LogicException('Mass assignment of Eloquent attributes including table names is unsafe when guarding attributes.'); - } - - return last(explode('.', $key)); - } - return $key; } diff --git a/tests/Integration/Database/EloquentModelTest.php b/tests/Integration/Database/EloquentModelTest.php index 3905d0badffa..e5666677e0db 100644 --- a/tests/Integration/Database/EloquentModelTest.php +++ b/tests/Integration/Database/EloquentModelTest.php @@ -46,11 +46,11 @@ public function test_cant_update_guarded_attribute_using_json() public function test_cant_mass_fill_attributes_with_table_names_when_using_guarded() { - $this->expectException(\LogicException::class); - $model = new TestModel2; $model->fill(['foo.bar' => 123]); + + $this->assertCount(0, $model->getAttributes()); } public function test_user_can_update_nullable_date() From 79590e868a5f32b01b01f826c049320b4a12e537 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 6 Aug 2020 09:55:00 -0500 Subject: [PATCH 08/10] Apply fixes from StyleCI (#33773) --- src/Illuminate/Database/Eloquent/Model.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Illuminate/Database/Eloquent/Model.php b/src/Illuminate/Database/Eloquent/Model.php index 667d36c757be..cd8a6dbdefe7 100644 --- a/src/Illuminate/Database/Eloquent/Model.php +++ b/src/Illuminate/Database/Eloquent/Model.php @@ -4,7 +4,6 @@ use Exception; use ArrayAccess; -use LogicException; use JsonSerializable; use Illuminate\Support\Arr; use Illuminate\Support\Str; From 97de42a6a424c76956f80f346161b2e2af9b9d6c Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Fri, 7 Aug 2020 10:01:01 -0500 Subject: [PATCH 09/10] [6.x] Verify column names are actual columns when using guarded (#33777) * verify column names are actual columns when using guarded * Apply fixes from StyleCI (#33778) * remove json check --- .../Eloquent/Concerns/GuardsAttributes.php | 31 +++++++++++++++++-- tests/Database/DatabaseEloquentModelTest.php | 14 +++++++-- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php index e261e1ff7d03..24d31b4b795e 100644 --- a/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php +++ b/src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php @@ -27,6 +27,13 @@ trait GuardsAttributes */ protected static $unguarded = false; + /** + * The actual columns that exist on the database and can be guarded. + * + * @var array + */ + protected static $guardableColumns = []; + /** * Get the fillable attributes for the model. * @@ -164,12 +171,30 @@ public function isFillable($key) */ public function isGuarded($key) { - if (strpos($key, '->') !== false) { - $key = Str::before($key, '->'); + if (empty($this->getGuarded())) { + return false; } return $this->getGuarded() == ['*'] || - ! empty(preg_grep('/^'.preg_quote($key).'$/i', $this->getGuarded())); + ! empty(preg_grep('/^'.preg_quote($key).'$/i', $this->getGuarded())) || + ! $this->isGuardableColumn($key); + } + + /** + * Determine if the given column is a valid, guardable column. + * + * @param string $key + * @return bool + */ + protected function isGuardableColumn($key) + { + if (! isset(static::$guardableColumns[get_class($this)])) { + static::$guardableColumns[get_class($this)] = $this->getConnection() + ->getSchemaBuilder() + ->getColumnListing($this->getTable()); + } + + return in_array($key, static::$guardableColumns[get_class($this)]); } /** diff --git a/tests/Database/DatabaseEloquentModelTest.php b/tests/Database/DatabaseEloquentModelTest.php index f50d0863df5b..f875252a843b 100755 --- a/tests/Database/DatabaseEloquentModelTest.php +++ b/tests/Database/DatabaseEloquentModelTest.php @@ -919,11 +919,21 @@ public function testUnderscorePropertiesAreNotFilled() public function testGuarded() { $model = new EloquentModelStub; + + EloquentModelStub::setConnectionResolver($resolver = m::mock('Illuminate\Database\ConnectionResolverInterface')); + $resolver->shouldReceive('connection')->andReturn($connection = m::mock(stdClass::class)); + $connection->shouldReceive('getSchemaBuilder->getColumnListing')->andReturn(['name', 'age', 'foo']); + $model->guard(['name', 'age']); $model->fill(['name' => 'foo', 'age' => 'bar', 'foo' => 'bar']); $this->assertFalse(isset($model->name)); $this->assertFalse(isset($model->age)); - $this->assertEquals('bar', $model->foo); + $this->assertSame('bar', $model->foo); + + $model = new EloquentModelStub; + $model->guard(['name', 'age']); + $model->fill(['Foo' => 'bar']); + $this->assertFalse(isset($model->Foo)); } public function testFillableOverridesGuarded() @@ -1829,7 +1839,7 @@ public function getDates() class EloquentModelSaveStub extends Model { protected $table = 'save_stub'; - protected $guarded = ['id']; + protected $guarded = []; public function save(array $options = []) { From 8191874460d485aab1d2286c089b1e1ca1baff77 Mon Sep 17 00:00:00 2001 From: Dries Vints Date: Thu, 13 Aug 2020 15:13:44 +0200 Subject: [PATCH 10/10] Apply fixes from StyleCI (#33857) --- src/Illuminate/Queue/Listener.php | 2 +- src/Illuminate/Support/Collection.php | 2 +- src/Illuminate/Support/helpers.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Queue/Listener.php b/src/Illuminate/Queue/Listener.php index 024d87d8c8ed..ffc3dc8d3dc1 100755 --- a/src/Illuminate/Queue/Listener.php +++ b/src/Illuminate/Queue/Listener.php @@ -232,7 +232,7 @@ public function memoryExceeded($memoryLimit) */ public function stop() { - die; + exit; } /** diff --git a/src/Illuminate/Support/Collection.php b/src/Illuminate/Support/Collection.php index 38dd526f0365..528a833376ea 100644 --- a/src/Illuminate/Support/Collection.php +++ b/src/Illuminate/Support/Collection.php @@ -275,7 +275,7 @@ public function dd(...$args) call_user_func_array([$this, 'dump'], $args); - die(1); + exit(1); } /** diff --git a/src/Illuminate/Support/helpers.php b/src/Illuminate/Support/helpers.php index c52fd66fa742..a574f7d151a4 100755 --- a/src/Illuminate/Support/helpers.php +++ b/src/Illuminate/Support/helpers.php @@ -559,7 +559,7 @@ function dd(...$args) (new Dumper)->dump($x); } - die(1); + exit(1); } }