Problem/Motivation

When HTTP middleware implemtations make calls for defined routes via calls like

/**
  * Route provider.
  *
  * @var \Drupal\Core\Routing\RouteProviderInterface
  */
protected $routeProvider;

public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = TRUE) {
...
  $matched_collection = $this->routeProvider->getRouteCollectionForRequest($request);
...
}

this results in the \Drupal::request() object being null in S3fsStream.php, which throws an error and causes a WSOD.

The attached patch first checks if the Request object is available before checking if HTTPS request should be forced. The approach taken in this patch is probably not ideal, because the null Request object means the S3 requests will be implicitly done over HTTP, but the module's settings allow HTTPS to be forced for all requests as a workaround.

This wasn't a problem for my particular project until #3180682: https not correctly detected in S3fsStream class made it into the latest release. Now the module relies on having the Request object available where it did not previously.

Steps to reproduce

  1. Enable s3fs
  2. Use the module to upload an image to S3
  3. Create an HTTP middleware that calls $matched_collection = $this->routeProvider->getRouteCollectionForRequest($request);. See BanMiddleware.php for the basic middleware pattern: https://api.drupal.org/api/drupal/core%21modules%21ban%21src%21BanMiddle...
  4. Clear cache
  5. Go to a page where the S3 file upload is referenced, such as /admin/content/files

Here, you'll see a WSOD.

Proposed resolution

Ideally, determine if there's a way to prevent S3fsStream::__construct() from being called when route definitions are requested.

Remaining tasks

TODO

User interface changes

Probably none.

API changes

Maybe some.

Data model changes

Probably none.

Comments

bobbygryzynger created an issue. See original summary.

bobbygryzynger’s picture

StatusFileSize
new742 bytes

Attaching patch.

bobbygryzynger’s picture

Issue summary: View changes
bobbygryzynger’s picture

Title: HTTP Middleware can cause the Request object to be null in S3fsStream.php » HTTP middleware implementations can result in the Request object being null in S3fsStream.php
bobbygryzynger’s picture

Issue summary: View changes
Status: Active » Needs review
Related issues: +#3180682: https not correctly detected in S3fsStream class
bobbygryzynger’s picture

Issue summary: View changes
cmlara’s picture

Sample stack trace:

The website encountered an unexpected error. Please try again later.
Error: Call to a member function isSecure() on null in Drupal\s3fs\StreamWrapper\S3fsStream->__construct() (line 195 of modules/custom/s3fs/src/StreamWrapper/S3fsStream.php). 

Drupal\s3fs\StreamWrapper\S3fsStream->__construct() (Line: 257)
Drupal\Component\DependencyInjection\Container->createService() (Line: 171)
Drupal\Component\DependencyInjection\Container->get() (Line: 131)
Drupal\Core\StreamWrapper\StreamWrapperManager->getWrapper() (Line: 96)
Drupal\Core\StreamWrapper\StreamWrapperManager->getViaScheme() (Line: 47)
Drupal\image\PathProcessor\PathProcessorImageStyles->processInbound() (Line: 70)
Drupal\Core\PathProcessor\PathProcessorManager->processInbound() (Line: 176)
Drupal\Core\Routing\RouteProvider->getRouteCollectionForRequest() (Line: 58)

Given the warnings on using Drupal::route() I guess we can call this a side effect of using the discouraged functions, but we don't have too much choice inside of S3fsStream as we need to get the request data to determine how we generate URL's and we do have a work around to always use secure URL's as you noted.

Looking at the stack trace I don't see much room for us to do much more here other than as the patch proposes. This would be caused by any module that needs to call the StreamWrappperManager which has to construct the StreamWrapper classes to do its job.

  • cmlara committed 4a81ebc on 8.x-3.x authored by bobbygryzynger
    Issue #3211523 by bobbygryzynger: HTTP middleware implementations can...
cmlara’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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