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

MimeType rule fails with large uploads #143

Closed
juniwalk opened this issue Jan 14, 2024 · 5 comments
Closed

MimeType rule fails with large uploads #143

juniwalk opened this issue Jan 14, 2024 · 5 comments

Comments

@juniwalk
Copy link

Hi,
I implemented your component in my app and I am having issue which I don't know how to solve.

When I set a rule to limit allowed mimetypes and I try to upload larger file, it fails on first chunk because FileUpload does not have mimetype available. I don't know if I should report this in nette/forms or here. If it is fixed in nette/forms and the rule fails, the upload fails.

$form->addFileUpload('upload')
	->addRule($form::MimeType, 'web.message.file-unknown-mimetype', $this::AllowedMimeTypes)
	->addRule($this->checkFreeSpace(...), 'web.message.file-size-limit-reached')
	->addRule($form::MaxFileSize, 'web.message.file-maximum-size', 5e7)	// 50 MB
	->addRule($form::MaxLength, 'web.message.file-maximum-number', 6);

The fail occurs in Validator.php in nette/forms because strtolower does not expect null.

public static function validateMimeType(Controls\UploadControl $control, $mimeType): bool
{
	$mimeTypes = is_array($mimeType) ? $mimeType : explode(',', $mimeType);
	foreach (static::toArray($control->getValue()) as $file) {
		$type = strtolower($file->getContentType());
		if (!in_array($type, $mimeTypes, true) && !in_array(preg_replace('#/.*#', '/*', $type), $mimeTypes, true)) {
			return false;
		}
	}

	return true;
}
@xificurk
Copy link
Contributor

Hi, thanks for the report. This is definitely a bug of this package.

@juniwalk
Copy link
Author

@xificurk Good to know I did not misreport this. Is there anything I can do to help?

@xificurk
Copy link
Contributor

I think, I've identified the problem, but still need to double check it and add tests. I'll have the fix out within a couple of days.

@xificurk
Copy link
Contributor

@juniwalk Can you please test current master to verify it fixes the problem in your setup?

@juniwalk
Copy link
Author

@xificurk Works great, thank you!

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

No branches or pull requests

2 participants