Media files with large IDs not removed due to wrong path generated for a Media with no id

foucdeg commented 6 years ago

Environment

Ubuntu 16/04

Sonata packages

$ composer show --latest 'sonata-project/*'
sonata-project/admin-bundle              3.35.2 3.35.2 The missing Symfony Admin Generator
sonata-project/block-bundle              3.12.1 3.12.1 Symfony SonataBlockBundle
sonata-project/cache                     2.0.1  2.0.1  Cache library
sonata-project/core-bundle               3.9.1  3.11.0 Symfony SonataCoreBundle
sonata-project/datagrid-bundle           2.3.1  2.3.1  Symfony SonataDatagridBundle
sonata-project/doctrine-extensions       1.0.2  1.0.2  Doctrine2 behavioral extensions
sonata-project/doctrine-orm-admin-bundle 3.6.0  3.6.1  Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/easy-extends-bundle       2.5.0  2.5.0  Symfony SonataEasyExtendsBundle
sonata-project/exporter                  1.9.0  1.9.1  Lightweight Exporter library
sonata-project/media-bundle              3.15.0 3.15.0 Symfony SonataMediaBundle

Symfony packages

$ composer show --latest 'symfony/*'
symfony/monolog-bundle     v3.2.0  v3.3.0 Symfony MonologBundle
symfony/polyfill-apcu      v1.8.0  v1.8.0 Symfony polyfill backporting apcu_* functions to lower P...
symfony/polyfill-ctype     v1.8.0  v1.8.0 Symfony polyfill for ctype functions
symfony/polyfill-intl-icu  v1.8.0  v1.8.0 Symfony polyfill for intl's ICU-related data and classes
symfony/polyfill-mbstring  v1.8.0  v1.8.0 Symfony polyfill for the Mbstring extension
symfony/polyfill-php56     v1.8.0  v1.8.0 Symfony polyfill backporting some PHP 5.6+ features to l...
symfony/polyfill-php70     v1.8.0  v1.8.0 Symfony polyfill backporting some PHP 7.0+ features to l...
symfony/polyfill-util      v1.8.0  v1.8.0 Symfony utilities for portability of PHP codes
symfony/security-acl       v3.0.1  v3.0.1 Symfony Security Component - ACL (Access Control List)
symfony/swiftmailer-bundle v2.6.7  v3.2.2 Symfony SwiftmailerBundle
symfony/symfony            v3.4.11 v4.1.1 The Symfony PHP framework

PHP version

$ php -v
PHP 7.2.5-0ubuntu0.18.04.1 (cli) (built: May  9 2018 17:21:02) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies
    with Zend OPcache v7.2.5-0ubuntu0.18.04.1, Copyright (c) 1999-2018, by Zend Technologies
    with Xdebug v2.6.0, Copyright (c) 2002-2018, by Derick Rethans```

Subject

Media files not removed due to wrong path generated for a Media with no id. Entities in a Doctrine postRemove handler have no ID, therefore the generated path is incorrect.

Steps to reproduce

Setup the Bundle so that the path generator is DefaultGenerator.php
Place a debug breakpoint in BaseProvider.php line 200, here
Delete a Media with an id > 10000 (if you use a small test database this is how you might have missed this)

Expected results

The media file is deleted

Actual results

The media file is not deleted
At the breakpoint, $media has no id
therefore the path generated by DefaultGenerator is incorrect

Interestingly, this issue was previously fixed with #696 and partially again with #1442 (partially because it only fixed thumbnail deletion). I don't know what happened between the two, but the code change in #696 should have remained.

stale[bot] commented 4 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

foucdeg commented 4 years ago

@core23 I don't know if this issue still exists or not. Is there enough information to investigate?

core23 commented 4 years ago

@core23 I don't know if this issue still exists or not. Is there enough information to investigate?

You reported the issue. Please check or close if you don't know if this is still an error.

foucdeg commented 4 years ago

Well, I'll close it then. I'm not working on a Symfony project right now so I can't really check.

Nuscly commented 2 years ago

This bug is fixed on the 4.x branch, by storing the media states in clones. See here https://github.com/sonata-project/SonataMediaBundle/blob/4.x/src/Provider/BaseProvider.php#L179

jordisala1991 commented 2 years ago

https://github.com/sonata-project/SonataMediaBundle/pull/1592

It was fixed on 3.x too a long time ago.

sonata-project / SonataMediaBundle