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

[11.x] Provide an error message for PostTooLargeException #53301

Merged

Conversation

patrickomeara
Copy link
Contributor

The ValidatePostSize middleware ensures the POST is under the allowed size set in php.ini.

However the exception message is empty

{
   "message": ""
}

This PR adds a message.

@rodrigopedra
Copy link
Contributor

rodrigopedra commented Oct 25, 2024

Not all the Exceptions extending HttpException will have a message (I am saying not all, instead of none as being too busy right now to test all of them).

For example:

Route::get('test', function (Request $request) {
    // ensure result is a JSON
    $request->headers->set('Accept', 'application/json');

    return throw new NotFoundHttpException();
});

Will give you the same response:

{
    "message": ""
}

Also, in case this is desirable, please use the default HTTP message for this exception: Content Too Large (reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/413 )


EDIT:

Forgot to mention, that the response's status code (413) should suffice for client applications to know the error.

Although, the linked MDN article has a nice suggestion on informing the client of the actual accepted size limit. Which could be useful.

This test was producing a false positive where ->server('CONTENT_LENGTH') was returning an array. There is no need to mock Request here.

I've also added a pass to ensure it works
@patrickomeara patrickomeara force-pushed the post-too-large-exception-message branch from d4bd29a to ba00fa1 Compare October 25, 2024 23:31
@taylorotwell taylorotwell merged commit 57af5c3 into laravel:11.x Oct 29, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants