Image styles - thumbnails are broken in config page when private file system is used

Reroll for Drupal 9.5+

fixed issue:

Error messageWarning: Undefined variable $sample_image_uri in Drupal\image\Controller\ImageStyleDownloadController->deliver() (line 184 of core/modules/image/src/Controller/ImageStyleDownloadController.php).

Reroll for Drupal 9.5+ #21

#23 does not appear to be a complete patch, parts of it are already in Drupal Core 9.5 and on first glance it appears to be missing the previous work without an explanation as to why. Hiding the patch to avoid confusion.

Back to 11.x as the mainline, as a bug it’s going to need to be solved in latest version and backported.

Rerolled for 10.1.x.

For the tests.

Fix CCF Errors.

Please also check the tags. Was tagged for tests which are still needed

Issue summary will probably need work too

Hello, I've dedicated the last two days to creating tests for the 3127116-27.patch. This marks my first attempt at contributing a patch. I would appreciate it if you could review the changes and let me know if everything is in order. Thank you.

Ran the tests locally and they failed with

Behat\Mink\Exception\ExpectationException : Current response status code is 403, but 200 expected.

It's recommended to start using MRs as patches are going away.

Looking at your test though and for your first time think you did a pretty good job

$original_path = \Drupal::config('image.settings')->get('preview_image');

Could probably use the local config reference

Some comments throughout would help too for example here

    $preview_file = $style->buildUri($original_path);
    if (!file_exists($preview_file)) {
      $style->createDerivative($original_path, $preview_file);
    }

Also why the need for the file_exist?

Overall though good work!

Moving to NW for the issue summary update I meant to tag before. Filled in some sections but left the rest to TBD, if not applicable to this issue just replace with NA. Helps the review

Issue summary looks good.

Were some code suggestions in #32 also

I've created a merge request and made the suggested code modifications. Could you please review and confirm if everything is in order?

Ran test-only feature

There was 1 error:
1) Drupal\Tests\image\Functional\ImageAdminStylesTest::testPreviewImageShowInPrivateScheme
Behat\Mink\Exception\ExpectationException: Current response status code is 403, but 200 expected.
/builds/issue/drupal-3127116/vendor/behat/mink/src/WebAssert.php:794
/builds/issue/drupal-3127116/vendor/behat/mink/src/WebAssert.php:130
/builds/issue/drupal-3127116/core/modules/image/tests/src/Functional/ImageAdminStylesTest.php:561
/builds/issue/drupal-3127116/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
ERRORS!
Tests: 8, Assertions: 184, Errors: 1.

Which matches/followed the steps in the issue summary.

Not 100% sure about checking for a sample image but the issue is solved.

I'm triaging RTBC issues → . I skimmed the IS and the comments. I did not find any unanswered questions. However, there is an unresolved comment from alexpott.

I added a link to the answers about security to the Issue Summary to help anyone working on this to find that discussion.

I then read the MR and there are comments that should be changed. Because of the existing unresolved comment and my further ones, I am setting this back to Needs work. There isn't much to do though.

1 small nitpicky item. But appears rest of feedback has been resolved. Free to self RTBC after that 1 change.

Committed/pushed to 11.x, cherry-picked to 10.3.x and 10.2.x, thanks!

Automatically closed - issue fixed for 2 weeks with no activity.