Can't get S3FS installed correctly

When I want to download a file by clicking a link though, I hit a php error:

What does the HREF portion of the links look like?

Hi @cmlara and thanks for taking the time to look at this, your module is great.

Hovering over a url produces this format url:
https://xyz.abc.io/system/files/human_readable_label.pdf

Hovering over a url produces this format url:
https://xyz.abc.io/system/files/human_readable_label.pdf

That looks correct to me. I would suggest looking at \Drupal\system\PathProcessor\PathProcessorFiles or any other InboundPathPricessors your environment has.

Also check for any core patches that are changing routing processing or routing table order.

IIRC (not at at desk to validate) it’s normally the system.file route not the systems.private_file_download route that should be delivering private:// files, even with s3fs takeover. We intentionally don’t change that and rely on the existing core to handle private:// file delivery.

Or another realization:

Symfony\Component\Routing\Exception\InvalidParameterException: Parameter "filepath" for route "system.private_file_download" must match ".+" ("" given) to generate a corresponding URL. in Drupal\Core\Routing\UrlGenerator->doGenerate() (line 203 of core/lib/Drupal/Core/Routing/UrlGenerator.php)

I can’t think of any reason a file link should be generated during a private:// file download request……. The full stack trace (or failing that breakpoint in a dev lab) might be more revealing.

Hey there, thanks for these leads! I'll follow them in order here:

I would suggest looking at \Drupal\system\PathProcessor\PathProcessorFiles or any other InboundPathProcessor’s your environment has.

Diving in to these is something I was hesitant about, I couldn't understand how to define my own inbound path processor correctly; do I need to make a manual entry here with my own arbitrary labels? I think I have a stock-standard Inbound PathProcessorFiles.php at that location:

class PathProcessorFiles implements InboundPathProcessorInterface {

  /**
   * {@inheritdoc}
   */
  public function processInbound($path, Request $request) {
    if (strpos($path, '/system/files/') === 0 && !$request->query->has('file')) {
      $file_path = preg_replace('|^\/system\/files\/|', '', $path);
      $request->query->set('file', $file_path);
      print_r('ticket');
      return '/system/files';
    }
    print_r('no ticket');
    return $path;
  }

}

I'm really on new ground here, and I'd love to understand more; you can see I've got a couple of print_r statements to try and grasp what is happening here. I think I correctly understand that this method is called any time a link needs to be rendered server-side, and that I can add my own logic here to set variables like $file_path to be passed in to the request as URL parameters. As far as I can tell, this looks correct for handling inbound files.

I've also taken a look at my system.routing file, specifically the entries you mention there:

system.files:
  path: '/system/files/{scheme}'
  defaults:
    _controller: '\Drupal\system\FileDownloadController::download'
    scheme: private
  requirements:
    _access: 'TRUE'

system.private_file_download:
  path: /system/files/{filepath}
  defaults:
    _controller: '\Drupal\system\FileDownloadController::download'
  requirements:
    # Permissive regex to allow slashes in filepath see
    # http://symfony.com/doc/current/cookbook/routing/slash_in_parameter.html
    filepath: .+
    _access: 'TRUE'

I initially thought this was the issue, but anywhere I've seen documentation of these entries seems to make these look correct, including the regex pattern being passed to the filepath property. I've tinkered with arbitrarily changing these values to see what breaks, and this configuration seems to be correct (based on poorly evidenced deductive logic. I'm doing my best but man, am I out of my depth here!).

The full stack trace (or failing that breakpoint in a dev lab) might be more revealing.

Here's a look at the reports I know to gather:
- The Reports tab's Recent Log Messages
- The white error page resulting from visiting the link
Unfortunately, these two report nearly identical error messages and don't really have a stack trace as far as I can see. My diagnostic knowledge here is limited ^_^;;

I hope that's a helpful reply, thank you again for looking in to this with me.

As a follow up, I found my php server's error logs and they also print the same error:

[26-May-2023 09:26:46 America/New_York] Uncaught PHP Exception Symfony\Component\Routing\Exception\InvalidParameterException: "Parameter "filepath" for route "system.private_file_download" must match ".+" ("" given) to generate a corresponding URL." at /code/web/core/lib/Drupal/Core/Routing/UrlGenerator.php line 203

As a follow up, I found my php server's error logs and they also print the same error:

You might try following https://www.drupal.org/forum/support/post-installation/2018-07-18/enable... → to see if it provides any more details in any of the logs.

Unfortunately if you are not receiving a more detailed log with those steps you may need to setup a full debug environment using XDEBUG.

I'll note that while the files may look fine on disk, their usage can be overridden by additional modules during runtime, when I refer to 'looking at some_component' I intend to imply investing not just its source code, but everything that interacts with it and may be modifying how its utilized as part of the Drupal Container.

As noted in #5 its not normal for the UrlGenerator to be called as part of a private file download so this very much looks like a 'custom code' or a contrib module issue outside of S3FS. It could be anything from a hook_file_download() implementation to more complex routing overrides. If more verbose logging doesn't help, or if you can't setup XDEBUG you could just try disabling modules (in a test lab) until you find the one causing the issue.

Hi cmlara, I can't thank you enough for your valuable input on this matter; after combing through our modules a bit we were able to locate the issue coming from IMCE File manager somehow. I haven't pinned down exactly why and I'd love to have that answer, but I wanted to report here so the thread can be closed for tidiness. Thank you again and I hope you have a great Monday!

Thanks for following up @herbo4.

Ah IMCE. I don't believe I have heard of this particular issue before however we have seen problems with the IMCE module in the past. I still had IMCE in my dev environment so it was quick to fire this up and validate.

Partial stack-trace follows:

The website encountered an unexpected error. Please try again later.

Symfony\Component\Routing\Exception\InvalidParameterException: Parameter "filepath" for route "system.private_file_download" must match ".+" ("" given) to generate a corresponding URL. in Drupal\Core\Routing\UrlGenerator->doGenerate() (line 203 of core/lib/Drupal/Core/Routing/UrlGenerator.php). 

Drupal\Core\Routing\UrlGenerator->getInternalPathFromRoute() (Line: 291)
Drupal\Core\Routing\UrlGenerator->generateFromRoute() (Line: 105)
Drupal\Core\Render\MetadataBubblingUrlGenerator->generateFromRoute() (Line: 765)
Drupal\Core\Url->toString() (Line: 39)
Drupal\s3fs\StreamWrapper\PrivateS3fsStream->getExternalUrl() (Line: 103)
Drupal\Core\File\FileUrlGenerator->doGenerateString() (Line: 66)
Drupal\Core\File\FileUrlGenerator->generateAbsoluteString() (Line: 106)
Drupal\imce\Imce::processUserConf() (Line: 86)
Drupal\imce\Imce::userConf() (Line: 35)
Drupal\imce\Imce::userFM() (Line: 348)
Drupal\imce\Imce::accessFilePaths() (Line: 363)
Drupal\imce\Imce::accessFileUri() (Line: 37)
imce_file_download()
call_user_func_array() (Line: 409)

See: https://git.drupalcode.org/project/imce/-/blob/3.x/src/Imce.php#L102-L106

S3fs does 'relative path resolution' converting "private://." to "private://" because this is what a normal filesystem when working with paths. Drupal Core is a bit 'dumb' in this regard as it doesn't currently do this, though I belive this is something that may be re-considered in the future.

Using Drupal Core without the s3fs module would create an error if the following is called which is ultimately what IMCE is requesting.
\Drupal::service('file_url_generator')->generateAbsoluteString('private://')
Having looked at IMCE code I think they added the period to avoid that issue. If the IMCE team had used something like "scheme://.sample_filename" instead of "scheme://." this would have worked because relative path resolution would not replace the path.

This is discussed a bit in https://www.drupal.org/project/s3fs/issues/3314823#comment-14740259 → .

Before sa-core-2023-005 it may have been arguable that S3FS module should not be resolving "scheme://." to "scheme://" however after sa-core-2023-005 public disclosure I believe it indicates we are correct in resolving the path to reduce long term risk.

With that said even the method of changing to "scheme://.a_file_name.txt" would be a 'hack' in my opinion and IMCE really should not be depending upon the root scheme:// to determine URL's. The fact that URL resolution is a part of the hook_file_download() is another indicator raising the question is the imce_hook_file_download() possibly open to de-coupling.

I suspect the fact IMCE does "scheme://." is because they have already encountered this fault at one time in the past or during initial development trying to do "private://" where Drupal Core itself threw the same exception.

IMCE did work on this partly in https://www.drupal.org/project/imce/issues/3320228 → however they ended up doing so only for the browser not the IMCE class, though its the same root portion of code that was initially called out as being of concern.

I would suggest opening an issue with IMCE and reference this original report for them to investigate.

Postponing on IMCE module.

An issue has been opened in IMCE 🐛 Parameter "filepath" for route "system.private_file_download" must match ".+" Fixed

The IMCE maintainer reports that they have fixed the issue.

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