mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-16 08:35:07 +00:00
ignacionelson
6086821d6c
#1680 fixed the redirect rendered from an exception and said plainly what it did not cover: EnsureSetupIsComplete, EnsureAccountIsActive and EnforceTwoFactor answer before HandleInertiaRequests is ever entered, so a response they return never unwinds through Inertia's 302 to 303 upgrade either. Same 405, reached a different way — an account deactivated while its owner was part-way through a form, or one being made to enrol in two-factor. The rule now lives in one place rather than four. Three copies of "if the method is PUT, PATCH or DELETE" is how the fourth caller gets it wrong, and WriteSafeRedirect can carry the explanation of why 303 — which is worth more than the three lines it replaces, because nothing about a bare setStatusCode call says what a browser does with a 302. PUT /timezone is the route the setup test uses: it is one of only two writes a guest can reach and the only one that middleware does not exempt, so the case is real rather than defensive. All three new tests were run against the unfixed middleware and fail there. Extends the work of @denkfabrik-li, who found the gap and wrote it down.
61 lines
2.2 KiB
PHP
61 lines
2.2 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Support;
|
|
|
|
use Illuminate\Http\Request;
|
|
use Symfony\Component\HttpFoundation\Response;
|
|
|
|
/**
|
|
* A redirect answering a write has to be a 303, not a 302.
|
|
*
|
|
* A browser follows a 302 by replaying the request method on the new
|
|
* location — POST is the only one it downgrades to GET. So a PUT that
|
|
* gets redirected to the login page is replayed as `PUT /login`, which
|
|
* accepts only GET and POST, and the person is shown a 405 instead of
|
|
* the one thing they needed to read: sign in again. 303 means "see
|
|
* other, and follow it with GET", which is the only sensible next step
|
|
* after a write.
|
|
*
|
|
* Inertia's own middleware already does this for responses that pass
|
|
* back through it. Two kinds never do, which is the whole reason this
|
|
* exists:
|
|
*
|
|
* - A redirect rendered during **exception handling** — the guest
|
|
* redirect after AuthenticationException above all — never travels
|
|
* back through the middleware stack at all.
|
|
* - A redirect returned early by middleware that runs *before*
|
|
* HandleInertiaRequests: EnsureSetupIsComplete, EnsureAccountIsActive
|
|
* and EnforceTwoFactor. A response only unwinds through middleware it
|
|
* already entered, and those three answer before Inertia's is reached.
|
|
*
|
|
* Reads are left alone. A 302 answering a GET is correct, and replaying
|
|
* a GET is exactly the right thing to do.
|
|
*
|
|
* See issue #1673 and pull request #1680, which found and fixed the
|
|
* first of the two cases; this is the same rule, kept in one place so
|
|
* the second could not drift from it.
|
|
*/
|
|
final class WriteSafeRedirect
|
|
{
|
|
/**
|
|
* The methods a browser replays verbatim when following a 302.
|
|
*
|
|
* POST is deliberately absent: browsers already downgrade it to GET,
|
|
* which is why form posts never showed this bug and only the
|
|
* Inertia-style PUT/PATCH/DELETE saves did.
|
|
*/
|
|
private const REPLAYED_METHODS = ['PUT', 'PATCH', 'DELETE'];
|
|
|
|
public static function apply(Request $request, Response $response): Response
|
|
{
|
|
if ($response->getStatusCode() === Response::HTTP_FOUND
|
|
&& in_array($request->method(), self::REPLAYED_METHODS, true)) {
|
|
$response->setStatusCode(Response::HTTP_SEE_OTHER);
|
|
}
|
|
|
|
return $response;
|
|
}
|
|
}
|