Contrib.social

Asset serving endpoints: there should be 404 instead of Client error

Open on Drupal.org →
Created on 3 November 2023, about 1 year ago
Updated 13 December 2023, about 1 year ago

Problem/Motivation

When I'm requesting a not existing file (Example: /sites/default/files/api/foo.jpg) I get 404.
When I make the same request on the CSS (/sites/default/files/CSS/foo.jpg) or JS (/sites/default/files/js/foo.jpg) folder I get a Client error.
It happens with every file type even with CSS and JS files if these don't exist.
But I made this example on purpose with the jpg files to move this ticket focus away from assets serving there are lots of tickets about that.

Basically just requesting random file names and I'm able to set the "red" state on any Drupal 10.1 monitoring page because of errors.
It must be 404 for those cases not Drupal errors.

Steps to reproduce

On any of Drupal 10.1 (even in the 10.1.6) go to the URL SITENAME/sites/default/files/css/foo.jpg or SITENAME/sites/default/files/js/foo.jpg

Proposed resolution

In the Drupal\system\Controller\AssetControllerBase->deliver function if the file doesn't exist it will end up with an error.
We should get 404 for any random requests.

public function deliver(Request $request, string $file_name) {
    $uri = 'assets://' . $this->assetType . '/' . $file_name;

    // Check to see whether a file matching the $uri already exists, this can
    // happen if it was created while this request was in progress.
    if (file_exists($uri)) {
      return new BinaryFileResponse($uri, 200, [
        'Cache-control' => static::CACHE_CONTROL,
        // @todo: remove the explicit setting of Content-Type once this is
        // fixed in https://www.drupal.org/project/drupal/issues/3172550.
        'Content-Type' => $this->contentType,
      ]);
    }

    // First validate that the request is valid enough to produce an asset group
    // aggregate. The theme must be passed as a query parameter, since assets
    // always depend on the current theme.
    if (!$request->query->has('theme')) {
      throw new BadRequestHttpException('The theme must be passed as a query argument');
    }
🐛 Bug report
Status

Active

Version

10.1 ✨

Component
Asset library  →

Last updated 4 days ago

No maintainer
Created by

🇪🇪Estonia mikkmiggur

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024