Uploading files breaks drupal page cache

Personally I kind of like the idea of having a permission for this. Added an MR for review.
This definitely needs more tests though if this is the way we want to go.

Can the MR be updated for 11.x as the main development branch

This was added to fix "Files uploaded by anonymous users into a private file system can be accessed by other anonymous users" in SA-CORE-2017-003 → .

git log -S anonymous_allowed_file_ids indicates that this mechanism has its origins in SA-CORE-2017-003 → (Files uploaded by anonymous users into a private file system can be accessed by other anonymous users).

https://git.drupalcode.org/project/drupal/-/commit/c732355412b84a6f7079d...

The code comment at the top of the relevant hunk reads:

        // This case handles new nodes, or detached files. The user who uploaded
        // the file can always access if it's not yet used.

This indicates that this branch in anonymous_allowed_file_ids">FileAccessControlHandler::checkAccess() is only reached as long as the parent entity (e.g. media) is not saved yet.

I think that the file id should be cleared from the anonymous_allowed_file_ids session value as soon as the temporary upload file is moved to permanent storage. If anonymous_allowed_file_ids becomes empty, then it needs to be removed from the session. And as soon as the whole session is empty, it will be destroyed automatically.

Something like this could go into a response subscriber:

$fids = $session->get('anonymous_allowed_file_ids', []);
if (!empty($fids)) {
  $fids_unref = [];
  $files = $this->fileStorage->loadMultiple($fids);
  foreach ($files as $file) {
    $references = $this->fileUsage->listUsage($file);
    if (empty($references)) {
      $fids_unref[] = $file->id();
    }
  }
  if (empty($fids_unref)) {
    $session->remove('anonymous_allowed_file_ids');
  }
  else {
    $session->set('anonymous_allowed_file_ids', $fids_unref);
  }
}

You can try this in a custom module as a quick fix until somebody comes up with a clever approach.

Thanks, I'll try that! The only case this doesn't work is when the user aborts after the file is uploaded. The entity the file is being attached to is never saved and the file will remain temporary as long as defined in system.file:temporary_maximum_age.

For now I will test the response subscriber and probably add something custom to remove the files for anonymous users after a short period of time. Hopefully someone can think of a better way to clean the sessions for those cases!

I'll also close the MR for now, the branch was wrong and the approach is probably to complex.

seanb → closed merge request !12241