diff --git a/src/Assets/Uploader.php b/src/Assets/Uploader.php index dc4b0d49c6..27c1548afc 100644 --- a/src/Assets/Uploader.php +++ b/src/Assets/Uploader.php @@ -4,7 +4,6 @@ use Facades\Statamic\Imaging\ImageValidator; use Statamic\Facades\Glide; -use Statamic\Support\Str; use Statamic\Support\Svg; use Symfony\Component\HttpFoundation\File\UploadedFile; @@ -58,7 +57,7 @@ private function write($sourcePath, $destinationPath) { $stream = fopen($sourcePath, 'r'); - if (config('statamic.assets.svg_sanitization_on_upload', true) && Str::endsWith($destinationPath, '.svg')) { + if (config('statamic.assets.svg_sanitization_on_upload', true) && trim(strtolower(pathinfo($destinationPath, PATHINFO_EXTENSION))) === 'svg') { $stream = Svg::sanitize(stream_get_contents($stream)); } diff --git a/src/Modifiers/CoreModifiers.php b/src/Modifiers/CoreModifiers.php index 1e07fec4aa..47e8dfcb1c 100644 --- a/src/Modifiers/CoreModifiers.php +++ b/src/Modifiers/CoreModifiers.php @@ -32,7 +32,6 @@ use Statamic\Support\Arr; use Statamic\Support\Dumper; use Statamic\Support\Html; -use Statamic\Support\MethodDenylist; use Statamic\Support\Str; use Statamic\Support\Traits\ChecksDumpability; use Statamic\View\Antlers\Language\Runtime\GlobalRuntimeState; @@ -904,12 +903,6 @@ public function get($value, $params) return Arr::get($array, $var); } - // Finally, try to call a method on the object - $method = Str::slug($var); - if (method_exists($item, $method) && ! MethodDenylist::blocks($method)) { - return $item->$method(); - } - // If after all is said and done, there's still nothing, just show the original value. return $value; } diff --git a/src/View/Antlers/Language/Runtime/PathDataManager.php b/src/View/Antlers/Language/Runtime/PathDataManager.php index 031eb04631..142535994e 100644 --- a/src/View/Antlers/Language/Runtime/PathDataManager.php +++ b/src/View/Antlers/Language/Runtime/PathDataManager.php @@ -863,9 +863,11 @@ private function reduceVar($path, $processorData = []) } if (is_object($this->reducedVar) && method_exists($this->reducedVar, $method = Str::camel($varPath)) && (new \ReflectionMethod($this->reducedVar, $method))->isPublic()) { - if (MethodDenylist::blocks($method)) { - // The method name derives from user-influenceable data, so never - // dispatch to methods that mutate or destroy data. Resolve to null. + // The method name derives from user-influenceable data, so never dispatch to + // methods that mutate or destroy data. Writing `{{ object.method }}` without + // parentheses calls the method just like `{{ object:method() }}` does, so both + // forms honor the `statamic.antlers.allowMethodsInContent` setting. + if (MethodDenylist::blocks($method) || (GlobalRuntimeState::$isEvaluatingUserData && ! GlobalRuntimeState::$allowMethodsInContent)) { $this->reducedVar = null; $this->didFind = false; $this->doBreak = true; diff --git a/tests/Antlers/Fixtures/MethodClasses/CallCounter.php b/tests/Antlers/Fixtures/MethodClasses/CallCounter.php index 41f051412f..77ef494b48 100644 --- a/tests/Antlers/Fixtures/MethodClasses/CallCounter.php +++ b/tests/Antlers/Fixtures/MethodClasses/CallCounter.php @@ -13,6 +13,13 @@ public function increment() return $this; } + public function incrementTwice() + { + $this->count += 2; + + return $this; + } + public function __toString(): string { return 'Count: '.$this->count; diff --git a/tests/Antlers/Runtime/MethodCallTest.php b/tests/Antlers/Runtime/MethodCallTest.php index d7f4782f13..a240e43215 100644 --- a/tests/Antlers/Runtime/MethodCallTest.php +++ b/tests/Antlers/Runtime/MethodCallTest.php @@ -343,6 +343,60 @@ public function test_method_calls_still_work_in_templates() ], false, true)); } + public function test_implicit_method_calls_blocked_in_user_content() + { + $counter = new CallCounter(); + + // Resolving a variable path to a zero-argument method is still a method + // call, so it honors the same setting as the explicit `:method()` syntax. + $this->assertSame('', $this->renderUserContent('{{ counter.increment }}', ['counter' => $counter])); + $this->assertSame('', $this->renderUserContent('{{ counter:increment }}', ['counter' => $counter])); + + // Str::camel() maps a snake_case path onto a camelCase method. + $this->assertSame('', $this->renderUserContent('{{ counter.increment_twice }}', ['counter' => $counter])); + + $this->assertSame('Count: 0', (string) $counter); + } + + public function test_implicit_method_calls_allowed_in_user_content_when_configured() + { + GlobalRuntimeState::$allowMethodsInContent = true; + + $object = new StringLengthObject('Hello'); + + $this->assertSame('5', $this->renderUserContent('{{ object.length }}', ['object' => $object])); + + GlobalRuntimeState::$allowMethodsInContent = false; + } + + public function test_implicit_method_calls_still_work_in_templates() + { + $object = new StringLengthObject('Hello'); + + $this->assertSame('5', $this->renderString('{{ object.length }}', [ + 'object' => $object, + ], false, true)); + + $this->assertSame('5', $this->renderString('{{ object:length }}', [ + 'object' => $object, + ], false, true)); + } + + private function renderUserContent($content, $data) + { + $textFieldtype = new Text(); + $field = new Field('text_field', [ + 'type' => 'text', + 'antlers' => true, + ]); + + $textFieldtype->setField($field); + + return $this->renderString('{{ text_field }}', array_merge($data, [ + 'text_field' => new Value($content, 'text_field', $textFieldtype), + ]), false, true); + } + public function test_nested_value_does_not_reset_user_data_flag() { $textFieldtype = new Text(); diff --git a/tests/Assets/AssetTest.php b/tests/Assets/AssetTest.php index de52a40a57..2ca7778862 100644 --- a/tests/Assets/AssetTest.php +++ b/tests/Assets/AssetTest.php @@ -43,10 +43,11 @@ use Symfony\Component\HttpFoundation\StreamedResponse; use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; +use Tests\WindowsHelpers; class AssetTest extends TestCase { - use PreventSavingStacheItemsToDisk; + use PreventSavingStacheItemsToDisk, WindowsHelpers; private $container; @@ -2089,6 +2090,48 @@ public function it_does_not_sanitizes_svgs_on_upload_when_behaviour_is_disabled( $this->assertStringContainsString('', $asset->contents()); } + #[Test] + #[DataProvider('unnormalizedSvgExtensionProvider')] + public function it_sanitizes_svgs_on_upload_regardless_of_how_the_extension_is_written($extension) + { + if (trim($extension) !== $extension) { + $this->markTestSkippedInWindows('Windows does not allow filenames with trailing whitespace.'); + } + + Event::fake(); + + // Disable filename lowercasing so the uppercase extension actually + // reaches the disk, otherwise it'd be normalized before we could + // prove the sanitization check itself is case insensitive. + config()->set('statamic.assets.lowercase', false); + + $asset = (new Asset)->container($this->container)->path($path = "path/to/asset.{$extension}")->syncOriginal(); + + Facades\AssetContainer::shouldReceive('findByHandle')->with('test_container')->andReturn($this->container); + Storage::disk('test')->assertMissing($path); + + $return = $asset->upload(UploadedFile::fake()->createWithContent("asset.{$extension}", '')); + + $this->assertEquals($asset, $return); + Storage::disk('test')->assertExists($path); + $this->assertEquals($path, $asset->path()); + + // Ensure the inline scripts were stripped out. + $this->assertStringNotContainsString('contents()); + $this->assertStringNotContainsString('Bad stuff could go in here.', $asset->contents()); + $this->assertStringNotContainsString('', $asset->contents()); + } + + public static function unnormalizedSvgExtensionProvider() + { + return [ + 'uppercase' => ['SVG'], + 'mixed case' => ['Svg'], + 'trailing whitespace' => ['svg '], + 'uppercase with trailing whitespace' => ['SVG '], + ]; + } + public static function nonGlideableFileExtensionsProvider() { return [ diff --git a/tests/Feature/Fieldtypes/FilesTest.php b/tests/Feature/Fieldtypes/FilesTest.php index 5b23a7069c..7701af9369 100644 --- a/tests/Feature/Fieldtypes/FilesTest.php +++ b/tests/Feature/Fieldtypes/FilesTest.php @@ -19,10 +19,11 @@ use Statamic\Fieldtypes\Files; use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; +use Tests\WindowsHelpers; class FilesTest extends TestCase { - use PreventSavingStacheItemsToDisk; + use PreventSavingStacheItemsToDisk, WindowsHelpers; public function setUp(): void { @@ -84,6 +85,48 @@ public function it_uploads_a_file($container, $isImage, $expectedPath, $expected } } + #[Test] + #[DataProvider('unnormalizedSvgExtensionProvider')] + public function it_sanitizes_svgs_on_upload_regardless_of_how_the_extension_is_written($extension) + { + if (trim($extension) !== $extension) { + $this->markTestSkippedInWindows('Windows does not allow filenames with trailing whitespace.'); + } + + Date::setTestNow(Date::createFromTimestamp(1671484636, config('app.timezone'))); + + $disk = Storage::fake('local'); + + $file = UploadedFile::fake()->createWithContent("test.{$extension}", ''); + + $this + ->actingAs(tap(User::make()->makeSuper())->save()) + ->post('/cp/fieldtypes/files/upload', ['file' => $file]) + ->assertOk() + ->assertJson([ + 'data' => [ + 'id' => $path = "1671484636/test.{$extension}", + ], + ]); + + $contents = $disk->get('statamic/file-uploads/'.$path); + + // Ensure the inline scripts were stripped out. + $this->assertStringNotContainsString('assertStringNotContainsString('Bad stuff could go in here.', $contents); + $this->assertStringNotContainsString('', $contents); + } + + public static function unnormalizedSvgExtensionProvider() + { + return [ + 'uppercase' => ['SVG'], + 'mixed case' => ['Svg'], + 'trailing whitespace' => ['svg '], + 'uppercase with trailing whitespace' => ['SVG '], + ]; + } + public static function uploadProvider() { return [ diff --git a/tests/Modifiers/GetTest.php b/tests/Modifiers/GetTest.php index 60a753c434..d8e0402261 100644 --- a/tests/Modifiers/GetTest.php +++ b/tests/Modifiers/GetTest.php @@ -22,7 +22,7 @@ public function it_gets_a_field_value() } #[Test] - public function it_returns_falsy_field_values_instead_of_falling_through_to_a_method() + public function it_returns_falsy_field_values_instead_of_the_original_value() { $item = new GetTestItem; @@ -32,45 +32,20 @@ public function it_returns_falsy_field_values_instead_of_falling_through_to_a_me } #[Test] - public function it_dispatches_to_a_non_destructive_accessor_method() + public function it_does_not_dispatch_to_methods() { $item = new GetTestItem; - $this->assertEquals('https://example.com', $this->modify($item, 'url')); - } - - #[Test] - public function it_does_not_dispatch_to_destructive_methods() - { - $item = new GetTestItem; - - $result = $this->modify($item, 'delete'); + $this->assertSame($item, $this->modify($item, 'url')); + $this->assertSame($item, $this->modify($item, 'delete')); $this->assertFalse($item->deleted); - $this->assertSame($item, $result); - } - - #[Test] - public function it_does_not_dispatch_to_destructive_methods_case_insensitively() - { - // Str::slug() lowercases the parameter (e.g. "deleteQuietly" => "deletequietly"), - // but method_exists() is case-insensitive, so the denylist must match regardless of case. - $item = new GetTestItem; - - $this->modify($item, 'deletequietly'); - $this->assertFalse($item->deletedQuietly); - - $this->modify($item, 'savequietly'); - $this->assertFalse($item->savedQuietly); } } class GetTestItem { public $deleted = false; - public $deletedQuietly = false; - public $saved = false; - public $savedQuietly = false; public function toArray() { @@ -91,19 +66,4 @@ public function delete() { $this->deleted = true; } - - public function deleteQuietly() - { - $this->deletedQuietly = true; - } - - public function save() - { - $this->saved = true; - } - - public function saveQuietly() - { - $this->savedQuietly = true; - } }