Enforce 65k character limit when attempting to update setting values.

[grimmdude]

Fixes #3081

Changes proposed in this pull request:

Throws a validation error if attempting to update setting values greater than 65k characters.

Reviewers should focus on:

Changing settings.

Screenshot

Necessity

• Has the problem that is being solved here been clearly explained?
• If applicable, have various options for solving this problem been considered?
• For core PRs, does this need to be in core, or could it be in an extension?
• Are we willing to maintain this for years / potentially forever?

Confirmed

• Frontend changes: tested on a local Flarum installation.
• Backend changes: tests are green (run composer test).
• Core developer confirmed locally this works as intended.
• Tests have been added, or are not appropriate here.

[askvortsov1]

Thank you for the PR! I think it might be more consistent with our current approach if the validation logic was put into a listener for Saving::class rather than directly in the controller. ValidateCustomLess is an example of this. Please don't hesitate to ask if you have any questions!

[askvortsov1]

Sorry I didn't mention this previously, but it would be preferable to wrap logic in a class, maybe ValidateSettingLength?

[askvortsov1]

A few more comments, sorry for the back-and-forth:

• We register ValidateCustomLess in the ForumServiceProvider because it's diretly relevant to the forum frontend (custom LESS isn't used on the forum side). However, I think it would be better to register this code in the SettingsServiceProvider, as this validation logic applies to all settings.
• As a convention, all our validators extend AbstractValidator (https://github.com/flarum/core/blob/312ff057f85d79548c35ec0f57459382d2f0d378/src/Foundation/AbstractValidator.php#L17-L17). I would prefer to either follow this pattern here, or for the new class to be an event listener (via invokable class).

[grimmdude]

No problem @askvortsov1, thanks for the input.

[grimmdude]

@askvortsov1, let me know what you think about that. Moved those settings event listeners to SettingsServiceProvider and used a more conventional approach for the new validation.

[askvortsov1]

@askvortsov1, let me know what you think about that. Moved those settings event listeners to SettingsServiceProvider and used a more conventional approach for the new validation.

Almost exactly what I intended, but in my previous comment I think I forgot to state that only the length validation logic should be moved; everything else is forum frontend dependent so it makes sense to keep it as is.

[askvortsov1]

Thank you!