Problem/Motivation

The current implementation of the controller doesn't take into account the current entity cache tags/context.

Meaning that when the path file entity is updated (e.g. published/unpublished), the controller is unaware of those changes and will continue to serve the old response.

It also doesn't contain error handling for if the file entity or real file still exists, it should handle those situations with a 404 response.

Steps to reproduce

  • Create an unpublished path file entity.
  • Visit in incognito and receive a 403 response.
  • Publish the path file entity
  • The user still receives a 403 response despite it being published.

Proposed resolution

Provide better entity caching and error handling.

Remaining tasks

Provide a patch/MR.

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Issue fork path_file-3228300

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

codebymikey created an issue. See original summary.

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Version: 8.x-1.x-dev » 2.1.1
Issue tags: +finalist-sprint

batigolix opened merge request !18

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Title: Controller needs to provide better caching/error handling » Controller needs to provide better caching & error (file missing) handling
batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Status: Active » Needs review

Here is a first attempt

vinodhini.e’s picture

vinodhini.e commented

Hi,

Tested this issue on Drupal 11.4.2 and followed the steps provided in the issue summary. I was unable to reproduce the reported behavior. After publishing the Path File entity, the file was accessible without clearing caches. This is working as expected without applying the patch.

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented

@vinodhini.e Can you test it WITH the changes from the MR? For example test that changing the published state is directly visible for anonymous users? In that case we can set this RTBC. Thanks!

vinodhini.e’s picture

vinodhini.e commented
Status: Needs review » Reviewed & tested by the community

Thanks, @batigolix.

Applied MR !18 and retested on Drupal 11.4.2. I verified that changes to the Path File entity's published state are immediately reflected for anonymous users without clearing caches. Tested the scenario you mentioned, and everything worked as expected. Marking RTBC.

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Status: Reviewed & tested by the community » Fixed

@vinodhini.e many thanks. If you are interested in more improvements of the Path File module: there are various issues waiting for review: https://www.drupal.org/project/path_file/issues
If you use Slack send me a message at @batigolix

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • batigolix committed 9ddff044 on 2.1.x 
    fix: #3228300 Controller needs to provide better caching...

Status: Fixed » Closed (fixed)

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