-
Notifications
You must be signed in to change notification settings - Fork 11k
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
[5.1] [5.2] Required file rules dont work when file size greater than max_file_upload size #8203
Comments
Duplicate of #8202? |
@GrahamCampbell - no - separate issue. #8202 is when post > This is when file > |
I've been digging around the validation rules - and I have an idea how we might solve this, plus improve the validation rules at the same time. The validation rule
The only way a The problem is that if the file upload had an error, then the I propose we remove that And we add a The
Then in your validation rules you just have to do
or you can do
Which defaults to min size of and you can easily combine it with
or like this:
|
Pretty decent imo, good job, but i see some problems too
What about executing all those check that you described in the previous message automatically if the field is file and has ANY rule? Neither new rule is needed, nor min and max broken then. It looks hard but not impossible |
How is this a breaking change? You can just continue to use your current validation rules using But in the 5.1 upgrade docs - I am assuming we would suggest people update their file validations to use the new rule, as it provides better file and error handling in some situations - but there is nothing forcing them to do this. People can also use the rule like this if they really want to - it would still work:
|
@taylorotwell - if I do this a PR to the 5.1 branch - is this something your likely to accept? I'm thinking I can switch PR #8215 to the 5.1 branch as well - then as part of the overall 5.1 upgrade docs we can mention there is an improvement to the file upload validation handling and error catching. Although there shouldnt be any BC from either PR - at least people would be aware of a change during the upgrade process. Thoughts? |
Although I may not hold much weight as I am new here, I would agree with @arrilot that it would be better to modify the existing "required" rule than to force users to add a new "file" rule to any file upload validations. A bugfix should not require end users to take an action, it should be invisible to them. Although adding a "file" validation rule is something that can be discussed as a new feature, it's separate from the bug immediately at hand. |
I disagree. Using the "required" rule creates two problems;
And because this will go to the 5.1 branch (if Taylor accepts it) - then it just forms part of the upgrade docs. There is no forcing people to change - they can keep their current code as is. But if you want to improve the validation and file error handling, then you can update your code to use these rules. |
@taylorotwell - can you please provide some guidance on your thoughts on this please? Are you happy if I do a PR to the 5.1 branch with some code similar to the above idea? (And I'll change #8215 to also be to the 5.1 branch). |
Ping @taylorotwell. |
Sure send a PR. |
Ping @theshiftexchange. |
Hi, Tested in Laravel 5.2.39, use case: $file = Request::file('file');
var_dump($file->isValid()); // => false
$this->validate(request(), [
'file' => 'file|max:300', // => Not check
// 'file' => 'required' // But this is checked, and failed: The file field is required.
]); |
@amonger I think we need better solution, because we need the validation message, not an exception... The problem here is all validation rules related to file (file, max, min, size, mimetypes, ..) will not run if the file is not successfully uploaded. The reason is that they are not implicit rules, so they are not validatable. So should we make file-related rules to be implicit rules? Or do we have any better approaches? |
Hi
|
@vigneshmuthukrishnan Verify that |
sorry @znck im new in laravel i don't file path in client_max_body_size pls help me |
Try adding Also check |
I'm work in local and I'm windows user then use in XAMPP |
{!! Form::open(array('route' => 'resizeImagePost','enctype' => 'multipart/form-data')) !!} {!! Form::close() !!} |
|
Error : |
@vigneshmuthukrishnan Better; take it to forum |
Hi @znck Now i change php.ini 'upload_max_filesize' and 'post_max_size' The localhost page isn’t working localhost is currently unable to handle this request. |
This my new errors: MethodNotAllowedHttpException in RouteCollection.php line 218: |
This issue has come up before: #4467 & #4226
@GrahamCampbell said in #4467 that there is an existing pull that deals with this - but it is not fixed - it is still broken - and I cant find any PR that seems to deal with this?
The problem is simple:
max_file_upload
(but less thanpost_max_size
)file
because it is larger thanmax_file_upload
required
check - sees the file is not there, and reports "The file is required".Obvious this is very confusing to users - because they did attach a file - but the website is telling them they didnt.
The general solution would be to add a simple check to see if there is an error on the
$_FILES
global - which would indicate there was a file attached. But then you run into a new problem - if the file isrequired
, but it is not actually there - then you cant letrequired
pass - it must fail.So I think the solution is to return a custom message for file validations - with some checks as to why the
required
failed?I'm happy to do the PR myself - but I'm looking for some guidance/direction on how we can solve this problem...
The text was updated successfully, but these errors were encountered: