Contrib.social

InvalidStreamWrapperException wrong string formatting in message

Open on Drupal.org →
Created on 15 February 2023, over 1 year ago
Updated 27 February 2023, over 1 year ago

Problem/Motivation

The messages for Drupal\Core\File\Exception\InvalidStreamWrapperException thrown in Drupal\file\FileRepository aren't fromatted correctly.

The lines 91 and 135 of the FileRepository class look as follows:

throw new InvalidStreamWrapperException(sprintf('Invalid stream wrapper: %destination', ['%destination' => $destination]));

sprintf doesn't work like that. sprintf sees %d as a placeholder "estination" is not seen as a placeholder.
%d is a placeholder for a numer, therefore the following argument (['%destination' => $destination]) is converted to a number. The array is converted into 1 and %d is replaced with it.

The error message will therefore always be "Invalid stream wrapper: 1estination", regardless of the value of $destination.

Steps to reproduce

Use the "file.repository" service and either use the copy or writeData method and provide an invalid destination as the second parameter.

You should get an exception with the message "Invalid stream wrapper: 1estination".

Proposed resolution

Properly use sprint to look like this:

throw new InvalidStreamWrapperException(sprintf('Invalid stream wrapper: %s', $destination));

Remaining tasks

Implment a fix to create a correct error message.
Write tests

User interface changes

None

API changes

None

Data model changes

None

🐛 Bug report
Status

Closed: duplicate

Version

10.0 ✨

Component
File system  →

Last updated 1 day ago

Created by

🇨🇭Switzerland GRcwolf

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

  • Issue created by @GRcwolf
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇨🇭Switzerland GRcwolf

    I created a patch for the issue and tested it.
    It works as intended for me.

  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

    As a bug this will need a test case

  • Status changed to Closed: duplicate over 1 year ago
  • Comment over 1 year ago →
    🇨🇭Switzerland GRcwolf

    I've missed that this issue was already reported and fixed in #3328694.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024