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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inertia sends a second request when redirecting on a verb different than GET or POST #1703

Open
tinyoverflow opened this issue Oct 14, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@tinyoverflow
Copy link

tinyoverflow commented Oct 14, 2023

Version:

  • @inertiajs/vue3 version: 1.0.11

Describe the problem:

I'm using Laravel RateLimits in my application. The rate limit has a callback defined, which redirects back to the previous route with a toast message (withToast is simply a Response macro, which appends something to the session and is not related to this issue).

RateLimiter::for('...', function (Request $request) {
    return Limit::perMinute(2)->by($request->user()?->id ?: $request->ip())
        ->response(fn() => redirect()->back()->withToast('...', '...'));
});

When hitting the limit with a GET or POST request, the server redirects back and Inertia does a GET request to the redirected route, as expected.

But when hitting the limit with anything else, like a PUT or DELETE request, the server redirects back, but instead of Inertia doing a GET request to the redirect target, it keeps the HTTP verb and does a PUT or DELETE request instead.

Example with GET/POST:

  1. I'm on /settings
  2. Hit rate limit on POST /another
  3. Rate limit redirects back to /settings
  4. Inertia does a GET to /settings and updates the page, as expected

Example with other verbs:

  1. I'm on /settings
  2. Hit rate limit on DELETE /another
  3. Rate limit redirects back to /settings
  4. Inertia does a DELETE to /settings and fails, as /settings doesn't support DELETE requests (405)

Steps to reproduce:

  1. Add a rate limit with a similar code snippet as above.
  2. Use <Link> or router.delete to send a delete request to another route, which hits the rate limit until you're limited. Using either <Link> or router.VERB doesn't seem to make a difference.
  3. See the issue.
@RobertBoes
Copy link
Contributor

It sounds like these routes aren't passed through Inertia's middleware, as Inertia already deals with the behaviour you're describing.

When doing a redirect, Laravel would make a 302 response and according to the HTTP spec the same method must be used by the browser (which is also out of Inertia's control afaik). So, the Inertia middleware converts a 302 response to a 303 response, which indicates follow-up requests must be made with the GET method. This is explained in Inertia's docs; https://inertiajs.com/redirects

@tinyoverflow
Copy link
Author

As far as I can tell, only PATCH, PUT and DELETE are rewritten from 302 to 303:
https://github.com/inertiajs/inertia-laravel/blob/master/src/Middleware.php#L102

But you're absolutely right with the middleware thing: It seems that a request cancelled by the throttle middleware skips all other middlewares. The request won't even reach the Inertia middleware in the first place. I'm not entirely sure, but it might be due to the reason, that the throttle middleware stops the request by throwing an exception, which is then catched by the exception handler, which in turn creates the typical exception page (eg. 500, or the debugger).

However, manually setting the status code inside the RateLimit callback does the trick. Though I have a feeling, that there probably might be a better solution I just haven't discovered yet.

RateLimiter::for('settings', function (Request $request) {
    return Limit::perMinute(30)->by($request->user()?->id ?: $request->ip())
        ->response(fn() => redirect()->back(303)->withToast('...', '...'));
});

@shengslogar
Copy link

shengslogar commented May 30, 2024

I ran into this today with Laravel's authentication middleware. Sending a PUT request to a route that required authentication resulted in a "PUT method is not supported" exception since the auth 302 redirect was not rewritten to a 303.

The problem is the priority of middleware. Inertia middleware must be first to properly intercept and rewrite redirects every time. This isn't an issue with controllers, but is an issue with competing middleware. In Laravel 11, you can solve this by configuring your middleware priority:

// bootstrap/app.php

<?php

use Illuminate\Foundation\Application;
use Illuminate\Foundation\Configuration\Middleware;

return Application::configure(basePath: dirname(__DIR__))
    ->withRouting(
        web: __DIR__.'/../routes/web.php',
        // ...
    )
    ->withMiddleware(function (Middleware $middleware) {

        // Do this...
        $middleware->priority([
            // Setting this by itself is enough to give it priority, but you should include the rest of Laravel middleware here...
            // @see https://laravel.com/docs/11.x/middleware#sorting-middleware
            \App\Http\Middleware\HandleInertiaRequests::class,
        ]);

        // Prepending also works in most instances, but wouldn't recommend (see note below)...
        $middleware->web(
            append: [
                // Your normal "appended" middleware...
            ],
            prepend: [
                \App\Http\Middleware\HandleInertiaRequests::class,
            ]
        );

    })->create();

Note on prepending: I need to look into this further, but the way Laravel creates a final ordered list of middleware seems inconsistent. Appending something like throttle:web after \App\Http\Middleware\HandleInertiaRequests::class does not give Inertia priority due to default prioritization rules. However, using the prepend: [ \App\Http\Middleware\HandleInertiaRequests::class ] mechanism unexpectedly does give Inertia priority over throttle, even though both two middlewares are in a similar overall list order. (Something to do with this sorting logic, I believe.) Because of this, I would rely on setting priority over trying to prepend.

@reinink Would you consider a PR making note of this in the docs or perhaps publishing a service provider like Sanctum that takes care of this prioritization? Or, if this problem isn't passing the sniff test, I'll work on a minimal repro.

@shengslogar
Copy link

Reviewing Laravel's middleware sorting logic with a fresh brain, it appears middleware priority is applied relative to other rules. In other words, the only way to guarantee a middleware runs first is to set it with the highest priority AND prepend it to your stack of middleware.

Appending prioritized middleware simply ensures its final order relative to one another is in the order you set, but does not give it priority over other unprioritized middleware.

As a result, I would now recommend prepending Inertia over playing with priority for simplicity. As long as you don't have any other middleware subsequently prepending, this will guarantee Inertia always runs first.

This could also translate to a simple doc tweak:

use App\Http\Middleware\HandleInertiaRequests;

 ->withMiddleware(function (Middleware $middleware) {
-    $middleware->web(append: [
+    $middleware->web(prepend: [
        HandleInertiaRequests::class,
    ]);
})

@driesvints driesvints reopened this Jun 17, 2024
@driesvints driesvints added the bug Something isn't working label Jun 17, 2024
@shengslogar
Copy link

shengslogar commented Jun 26, 2024

Due to this change introduced in inertiajs/inertia-laravel#627 and released in inertiajs/inertia-laravel@1.3.0 on June 12th, validation errors are always empty if HandleInertiaRequests is prepended per my previous comment. This is because validation errors are now resolved immediately, versus through a deferred function, before \Illuminate\Session\Middleware\StartSession::class has been called.

The updated solution to this issue is to set middleware priority to ensure the following order:

\Illuminate\Session\Middleware\StartSession::class,
\App\Http\Middleware\HandleInertiaRequests::class,

Or, if you're not using partial reloads anywhere in your specific project, you could continue to prepend and override 'errors' to be a callable again: [ 'errors' => fn() => $this->resolveValidationErrors($request) ].

In my opinion, the most elegant fix would be to extract the Inertia middleware that is modifying the request response into a separate middleware file that is always given top priority via a service provider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants