Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Broken SetRequestMiddleware on Laravel < 6.0 #575

Merged
merged 4 commits into from Sep 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -4,6 +4,7 @@

- Fix not listening to queue events because `QueueManager` is registered as `queue` in the container and not by it's class name (#568)
- Fix status code not populated on transaction if response did not inherit from `Illuminate\Http\Response` like `Illuminate\Http\JsonResponse` (#573)
- Fix broken `SetRequestMiddleware` on Laravel < 6.0 (#575)

## 2.13.0

Expand Down
3 changes: 0 additions & 3 deletions composer.json
Expand Up @@ -33,9 +33,6 @@
"Sentry\\Laravel\\": "src/"
}
},
"suggest": {
"zendframework/zend-diactoros": "When using Laravel >=5.1 - <=6.9 this package can help get more accurate request info, not used on Laravel >=6.10 anymore (https://laravel.com/docs/5.8/requests#psr7-requests)"
},
"require-dev": {
"phpunit/phpunit": "^5.7 | ^6.5 | ^7.5 | ^8.4 | ^9.3",
"laravel/framework": "5.0 - 5.8 | ^6.0 | ^7.0 | ^8.0 | ^9.0",
Expand Down
3 changes: 3 additions & 0 deletions src/Sentry/Laravel/Http/LaravelRequestFetcher.php
Expand Up @@ -18,6 +18,9 @@ public function fetchRequest(): ?ServerRequestInterface
{
$container = Container::getInstance();

// If there is no request bound to the container
// we are not dealing with a HTTP request and there
// is no request to fetch for us so we can exit early.
if (!$container->bound('request')) {
return null;
}
Expand Down
48 changes: 31 additions & 17 deletions src/Sentry/Laravel/Http/SetRequestMiddleware.php
Expand Up @@ -4,41 +4,55 @@

use Closure;
use Illuminate\Container\Container;
use Illuminate\Contracts\Container\BindingResolutionException;
use Illuminate\Http\Request;
use Nyholm\Psr7\Factory\Psr17Factory;
use Psr\Http\Message\ServerRequestInterface;
use Sentry\State\HubInterface;
use Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory;

/**
* This middleware caches a PSR-7 version of the request as early as possible.
* This is done to prevent running into (mostly uploaded file) parsing failures.
*/
class SetRequestMiddleware
{
/**
* Handle an incoming request.
*
* @param \Illuminate\Http\Request $request
* @param \Closure $next
*
* @return mixed
*/
public function handle(Request $request, Closure $next)
{
$container = Container::getInstance();

if ($container->bound(HubInterface::class)) {
try {
$container->instance(
LaravelRequestFetcher::CONTAINER_PSR7_INSTANCE_KEY,
$container->make(ServerRequestInterface::class)
);
} catch (BindingResolutionException $e) {
// Ignore problems getting the PSR-7 server request instance here
// In the Laravel request fetcher we have other fallbacks for that
$psrRequest = $this->resolvePsrRequest($request);

if ($psrRequest !== null) {
$container->instance(LaravelRequestFetcher::CONTAINER_PSR7_INSTANCE_KEY, $psrRequest);
}
}

return $next($request);
}

/**
* This code was copied from the Laravel codebase which was introduced in Laravel 6.
*
* The reason we have it copied here is because older (<6.0) versions of Laravel use a different
* method to construct the PSR-7 request object which requires other packages to create that object
* but most importantly it does not function when those packages are not available resulting in errors
*
* So long story short, this is here to backport functionality to Laravel <6.0
* if we drop support for those versions in the future we can reconsider this and
* move back to using the container binding provided by Laravel for the PSR-7 object
*
* @see https://github.com/laravel/framework/blob/cb550b5bdc2b2c4cf077082adabde0144a72d190/src/Illuminate/Routing/RoutingServiceProvider.php#L127-L146
*/
private function resolvePsrRequest(Request $request): ?ServerRequestInterface
{
if (class_exists(Psr17Factory::class) && class_exists(PsrHttpFactory::class)) {
$psr17Factory = new Psr17Factory;

return (new PsrHttpFactory($psr17Factory, $psr17Factory, $psr17Factory, $psr17Factory))
->createRequest($request);
}

return null;
}
}
2 changes: 1 addition & 1 deletion src/Sentry/Laravel/ServiceProvider.php
Expand Up @@ -206,7 +206,7 @@ protected function configureAndRegisterClient(): void
});

$integrations[] = new SdkIntegration\RequestIntegration(
new LaravelRequestFetcher($app)
new LaravelRequestFetcher
);
}

Expand Down