Ensure the setAllowedMaxFileSize wont allow config values to exceed PHP limits

[nfauchelle]

Issue silverstripe/silverstripe-framework#10058

Fixes an issue where you could specify an upload validation size larger than the PHP allowed value. This will now check to ensure what is set on config won't exceed the PHP limit. This also uses Convert since File::ini2bytes is deprecated, added some tests

[emteknetnz]

Sorry about this. This is not going to be an easy to fix unfortunately. I've done some testing with both this PR and the corresponding framework PRs installed and I got some inconsistent results. A few things:

• default_max_file_size was respected when using the UploadField, though didn't seem to be working at all when uploading files via the file manager (asset-admin). This inconsistency to me is worse than just having it consistently ignored, it will lead to a confusing user experience.
• We can't make any assumptions about the php.ini max size, I don't even know what the limit of travis is, and it may change. I'm not sure if if it's possible ini_set() type of thing during the unit test and then reset it. We may need to detect if SapphireTest is running and return a fixed value from the maxUploadSizeFromPHPIni() if true
• There's no mention of default_max_file_size in the CMS4 dev docs - instead it only talks on the php.ini values being respected. So these docs also need to be updated. I've noticed there's a somewhat brief and poor mention of it in the CMS3 docs
• Framework has assets as a dependency, we'd also need to add composer constraints within the framework PR composer.json for the version of assets this is released in, which given this is effectively a functionality enhancement at this point, would need to be ^4.10. So we'd need to rebase/retarget these PR's to 4 and 1 respectively

[nfauchelle]

@emteknetnz
Thanks for the review. I will have a look into asset-admin and see there as needed.

With regards to ini_set, I am not calling that. I use ini_get and change my values to be higher for the test. I don't think anything needs to change with your second point.

Your third point is interesting, since it was working in 3 (I believe) and then breaks for people upgrading to 4, so this could be seen as a fix. If it's not documented in 4 now then no-one should be using it unless they were using it in 3. Either or. I will take a look at that asset-admin and see how much needs to be fixed there. I wonder if it has the same issue as my original issue :)

Thanks again!

[GuySartorelli]

• default_max_file_size was respected when using the UploadField, though didn't seem to be working at all when uploading files via the file manager (asset-admin). This inconsistency to me is worse than just having it consistently ignored, it will lead to a confusing user experience.

This is now resolved by silverstripe/silverstripe-asset-admin#1430

• We can't make any assumptions about the php.ini max size, I don't even know what the limit of travis is, and it may change. I'm not sure if if it's possible ini_set() type of thing during the unit test and then reset it. We may need to detect if SapphireTest is running and return a fixed value from the maxUploadSizeFromPHPIni() if true

I agree with the comments in #469 (comment) - it seems like no such assumptions are being made.

• There's no mention of default_max_file_size in the CMS4 dev docs - instead it only talks on the php.ini values being respected. So these docs also need to be updated. I've noticed there's a somewhat brief and poor mention of it in the CMS3 docs

I'll raise a new PR to update these docs - it'll be linked in the issue.

• Framework has assets as a dependency, we'd also need to add composer constraints within the framework PR composer.json for the version of assets this is released in, which given this is effectively a functionality enhancement at this point, would need to be ^4.10. So we'd need to rebase/retarget these PR's to 4 and 1 respectively

Updated the framework PR.

[GuySartorelli]

I've rebased and retargetted to 2 so that this can be included in a future minor release.