Skip to content
Snippets Groups Projects

Fix filter issue with images in private file directory

Open nico.b requested to merge issue/drupal-2528214:2528214-restrict-images-to into 11.x
2 unresolved threads

Merge request reports

Members who can merge are allowed to add commits.

Merge request pipeline #441830 passed with warnings

Pipeline: drupal-2528214

#441831

    Pipeline: drupal-2528214

    #441837

      Merge request pipeline passed with warnings for 09591e1e

      Approval is optional
      Ready to merge by members who can write to the target branch.
      • The source branch is 419 commits behind the target branch.
      • 1 commit will be added to 11.x.
      • Source branch will not be deleted.

      Activity

      Filter activity
      • Approvals
      • Assignees & reviewers
      • Comments (from bots)
      • Comments (from users)
      • Commits & branches
      • Edits
      • Labels
      • Lock status
      • Mentions
      • Merge request status
      • Tracking
      761 $private_image_path = 'private://' . $private_image_path;
      762 $private_image_path = urldecode($private_image_path);
      763 $image_object = \Drupal::service('image.factory')->get($private_image_path);
      764 if ($image_object->getFileSize()) {
      765 // The image has the right path. Erroneous images are dealt with below.
      766 continue;
      767 }
      768 }
      769 elseif (mb_substr($src, 0, $base_path_length) === $base_path) {
      756 770 // Remove the $base_path to get the path relative to the Drupal root.
      757 771 // Ensure the path refers to an actual image by prefixing the image source
      758 772 // with the Drupal root and running getimagesize() on it.
      759 773 $local_image_path = $local_dir . mb_substr($src, $base_path_length);
      760 774 $local_image_path = rawurldecode($local_image_path);
      775
      776 // Check for query string and remove it if present.
      • This says what is happening but not why - is it because the @getimagesize call fails below if there's a query string? If so it should mention that.

      • Please register or sign in to reply
    • catch @catch started a thread on the diff
    • 752 756 // Verify that $src starts with $base_path.
      753 757 // This also ensures that external images cannot be referenced.
      754 758 $src = $image->getAttribute('src');
      755 if (mb_substr($src, 0, $base_path_length) === $base_path) {
      759 if (mb_substr($src, 0, $private_path_length) === $private_path) {
      • Adding support for private stream wrappers alongside local seems a bit brittle - what about another local stream wrapper that's neither of these?

        At least checking for local, if not file existence, should happen generically based on the URL to the image rather than the directory location I think.

        Also, do we really need to check the file on disk here at all?

      • Please register or sign in to reply
      Please register or sign in to reply
      Loading