Contrib.social

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

Open on Drupal.org →
Created on 3 November 2023, 8 months ago
Updated 13 December 2023, 7 months 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 2 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

  • Issue created by @mikkmiggur
  • Comment 8 months ago →
    🇪🇪Estonia mikkmiggur
  • Comment 8 months ago →
    🇫🇷France nod_ Lille

    I think serving a 404 when there are no query parameter and the file doesn't already exist makes sense.

    If there are query parameters the errors are there to help understand what's happening so I still think an error is a appropriate response to a malformed request.

  • Comment 8 months ago →
    🇫🇷France nod_ Lille
  • Comment 8 months ago →
    🇺🇸United States cilefen

    I think there is already an issue for this.

  • Comment 7 months ago →
    🇪🇪Estonia mikkmiggur

    Tested with the latest Drupal 10.1.7
    [ANY D10.1 SITE]/sites/default/files/css/foo.css -> 400 "A client error happened"
    Why it can't be a normal 404 file not found?

  • Comment 7 months ago →
    🇺🇸United States cilefen

    404 is a client error: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status#client_error_re...

    Can we improve the issue title?

  • Comment 7 months ago →
    🇮🇳India SandeepSingh199

    There are two different things
    1. If the path of the url is valid and the file/image name is different/wrong, on that time it will display 404 A Client Error Happened (e.g. /sites/default/files/js/foo.jpg)
    2. If the file/image name is valid but path is invalid on that time it will display 404 Not Found, the normal messages. (e.g. /sites/default/files_1/js/foo.jpg).

    I think the message is okay for two different situation.

  • Comment 7 months ago →
    🇪🇪Estonia mikkmiggur

    /sites/default/files/foo.jpg - This gets 404 as it should.
    /sites/default/files/js/foo.jpg - This gets 400 and the client error page and Drupal writes to the logs client error. Anything not existing in the JS and CSS folder is causing that problem (/sites/default/files/js/error.exe). It's not about file types. Drupal not handling correctly not existing files in these folders.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024