search

sonata-project / SonataBlockBundle

Symfony SonataBlockBundle
https://docs.sonata-project.org/projects/SonataBlockBundle
MIT License
413 stars 142 forks source link

Form validations of Block are bypassed when an Exception is thrown #1217

Open sad270 opened 1 day ago

sad270 commented 1 day ago

Environment

Sonata packages

show

``` $ composer show --latest 'sonata-project/*' Direct dependencies required in composer.json: sonata-project/admin-bundle 4.31.0 4.31.0 The missing Symfony Admin Generator sonata-project/doctrine-orm-admin-bundle 4.17.1 4.17.1 Integrate Doctrine ORM into the SonataAdminBundle sonata-project/media-bundle 4.14.0 4.14.1 Symfony SonataMediaBundle sonata-project/page-bundle 4.7.2 4.8.0 This bundle provides a Site and Page management through container and block services Transitive dependencies not required in composer.json: sonata-project/block-bundle 5.1.0 5.1.1 Symfony SonataBlockBundle sonata-project/doctrine-extensions 2.4.1 2.4.1 Doctrine2 behavioral extensions sonata-project/exporter 3.3.0 3.3.0 Lightweight Exporter library sonata-project/form-extensions 2.4.0 2.4.0 Symfony form extensions sonata-project/seo-bundle 3.8.0 3.8.0 Symfony SonataSeoBundle sonata-project/twig-extensions 2.4.0 2.5.0 Sonata twig extensions ```

Symfony packages

show

``` $ composer show --latest 'symfony/*' Direct dependencies required in composer.json: symfony/console 6.4.10 7.1.5 Eases the creation of beautiful and testable command line interfaces symfony/dotenv 6.4.10 7.1.5 Registers environment variables from a .env file symfony/flex 2.4.6 2.4.7 Composer plugin for Symfony symfony/framework-bundle 6.4.10 7.1.5 Provides a tight integration between Symfony components and the Symfony full-stack framework symfony/runtime 6.4.8 7.1.1 Enables decoupling PHP applications from global state symfony/yaml 6.4.8 7.1.5 Loads and dumps YAML files Transitive dependencies not required in composer.json: symfony/asset 6.4.8 7.1.1 Manages URL generation and versioning of web assets such as CSS stylesheets, JavaScript files and image files symfony/cache 6.4.10 7.1.5 Provides extended PSR-6, PSR-16 (and tags) implementations symfony/cache-contracts 3.5.0 3.5.0 Generic abstractions related to caching symfony/clock 6.4.8 7.1.1 Decouples applications from the system clock symfony/config 6.4.8 7.1.1 Helps you find, load, combine, autofill and validate configuration values of any kind symfony/dependency-injection 6.4.10 7.1.5 Allows you to standardize and centralize the way objects are constructed in your application symfony/deprecation-contracts 3.5.0 3.5.0 A generic function and convention to trigger deprecation notices symfony/doctrine-bridge 6.4.10 7.1.5 Provides integration for Doctrine with various Symfony components symfony/error-handler 6.4.10 7.1.3 Provides tools to manage errors and ease debugging PHP code symfony/event-dispatcher 6.4.8 7.1.1 Provides tools that allow your application components to communicate with each other by dispatching events and listening to them symfony/event-dispatcher-contracts 3.5.0 3.5.0 Generic abstractions related to dispatching event symfony/expression-language 6.4.8 7.1.4 Provides an engine that can compile and evaluate expressions symfony/filesystem 6.4.9 7.1.5 Provides basic utilities for the filesystem symfony/finder 6.4.10 7.1.4 Finds files and directories via an intuitive fluent interface symfony/form 6.4.10 7.1.5 Allows to easily create, process and reuse HTML forms symfony/http-client 6.4.10 7.1.5 Provides powerful methods to fetch HTTP resources synchronously or asynchronously symfony/http-client-contracts 3.5.0 3.5.0 Generic abstractions related to HTTP clients symfony/http-foundation 6.4.10 7.1.5 Defines an object-oriented layer for the HTTP specification symfony/http-kernel 6.4.10 7.1.5 Provides a structured process for converting a Request into a Response symfony/intl 6.4.8 7.1.5 Provides access to the localization data of the ICU library symfony/mime 6.4.9 7.1.5 Allows manipulating MIME messages symfony/options-resolver 6.4.8 7.1.1 Provides an improved replacement for the array_replace PHP function symfony/password-hasher 6.4.8 7.1.1 Provides password hashing utilities symfony/polyfill-intl-grapheme 1.30.0 1.31.0 Symfony polyfill for intl's grapheme_* functions symfony/polyfill-intl-icu 1.30.0 1.31.0 Symfony polyfill for intl's ICU-related data and classes symfony/polyfill-intl-idn 1.30.0 1.31.0 Symfony polyfill for intl's idn_to_ascii and idn_to_utf8 functions symfony/polyfill-intl-normalizer 1.30.0 1.31.0 Symfony polyfill for intl's Normalizer class and related functions symfony/polyfill-mbstring 1.30.0 1.31.0 Symfony polyfill for the Mbstring extension symfony/polyfill-php83 1.30.0 1.31.0 Symfony polyfill backporting some PHP 8.3+ features to lower PHP versions symfony/process 6.4.8 7.1.5 Executes commands in sub-processes symfony/property-access 6.4.8 7.1.4 Provides functions to read and write from/to an object or array using a simple string notation symfony/property-info 6.4.10 7.1.3 Extracts information about PHP class' properties using metadata of popular sources symfony/routing 6.4.10 7.1.4 Maps an HTTP request to a set of configuration variables symfony/security-acl 3.3.3 3.3.4 Symfony Security Component - ACL (Access Control List) symfony/security-bundle 6.4.10 7.1.4 Provides a tight integration of the Security component into the Symfony full-stack framework symfony/security-core 6.4.10 7.1.5 Symfony Security Component - Core Library symfony/security-csrf 6.4.8 7.1.1 Symfony Security Component - CSRF Library symfony/security-http 6.4.9 7.1.5 Symfony Security Component - HTTP Integration symfony/serializer 6.4.10 7.1.5 Handles serializing and deserializing data structures, including object graphs, into array structures or other formats like XML and JSON. symfony/service-contracts 3.5.0 3.5.0 Generic abstractions related to writing services symfony/stopwatch 6.4.8 7.1.1 Provides a way to profile code symfony/string 6.4.10 7.1.5 Provides an object-oriented API to strings and deals with bytes, UTF-8 code points and grapheme clusters in a unified way symfony/translation 6.4.10 7.1.5 Provides tools to internationalize your application symfony/translation-contracts 3.5.0 3.5.0 Generic abstractions related to translation symfony/twig-bridge 6.4.9 7.1.5 Provides integration for Twig with various Symfony components symfony/twig-bundle 6.4.8 7.1.5 Provides a tight integration of Twig into the Symfony full-stack framework symfony/validator 6.4.10 7.1.5 Provides tools to validate values symfony/var-dumper 6.4.10 7.1.5 Provides mechanisms for walking through any arbitrary PHP variable symfony/var-exporter 6.4.9 7.1.2 Allows exporting any serializable PHP data structure to plain PHP code ```

PHP version

$ php -v
PHP 8.3.12 (cli) (built: Sep 26 2024 22:38:42) (ZTS)
Copyright (c) The PHP Group
Zend Engine v4.3.12, Copyright (c) Zend Technologies
    with Zend OPcache v8.3.12, Copyright (c), by Zend Technologies
    with Xdebug v3.3.2, Copyright (c) 2002-2024, by Derick Rethans

Subject

When an exception is thrown while validating a block's form, nothing is done, and invalid value are stored in database.

Steps to reproduce

Change the method validate of RssBlockService to make them throw an exception :

    public function validate(ErrorElement $errorElement, BlockInterface $block): void
    {
        $errorElement
            ->with('settings[url]')
                ->addConstraint(new NotNull())
                ->addConstraint(new NotBlank(['please_throw_exception' => 'this option does not exist, so it will throw an exception']))
            ->end()
            ->with('settings[title]')
                ->addConstraint(new NotNull())
                ->addConstraint(new NotBlank())
                ->addConstraint(new Length(['max' => 50]))
            ->end();
    }

Now, add the RSS Feed block on a page, and we validate submit an empty form, we will only have the this value should not be null on the url field.

https://github.com/user-attachments/assets/0817fafb-0e7c-4f0e-a7dc-34b18e1a64ff

And if we fill the url field only, and submit

https://github.com/user-attachments/assets/25250d46-e6e2-468a-88a2-c1fd0cdce6bd

Expected results

We do not expect to save invalid data in DB and we expect an error with 500 status code response.

Actual results

Invalid data are stored in database. No errors or form invalidation message is returned.

sad270 commented 1 day ago

I found which catch catch the exception. But I don't understand, and i am confused, maybe someone can help me please 🙏🏻 ?

I don't know why we are catching \Exception ?

Why we only change the value of the inValidate property in the catch ?

What is the goal of this inValidate property ? It's seem's to ensure that the validate is done only one time, but the namming is not in accordance with, like it's seems to be a typo invalidate because I didn't understand why did we need to know if we are in validate method (like the namming of the property said) ?

So, I don't know if to fix it, we should re-throw the exception (like I did in #1218 ) or just remove the try catch ?

I investigate a little bit, the try and catch and the inValidate property are introduce by this commit https://github.com/sonata-project/SonataBlockBundle/commit/59a689d42eac5883c28f386422ac2d390db3a9a6 by @rande in 2013 ( 11 years ago ). Maybe you could help us 🙏🏻 ? Even if it can be hard to remember what's happen 11 years ago 😄