Introduce helper function for DISALLOW_FILE_MODS checks

[MaximeCulea]

Hi guys,

Before submitting a real issue and the according patch, I would like to know how you guys feel about the following situation.

As an agency, I do not allow my clients to update core, plugins, so on. So, I set the DISALLOW_FILE_MODS to true.

But by doing this, no one on site is anymore able to install a new language, translate i18n strings with Locostranslate, for ie, as they are based, and with good reasons, on the DISALLOW_FILE_MODS var. This is due of a maybe file creation.

For now, I described the current and "correct" way of using and doing it. But as far as WordPress evolved since v4.0.0, the available language dropdown and other features are limited as we could say "in the same basket".

My question is, why we couldn't create a new var which affect these languages concern DISALLOW_LANGUAGE_MODS ?

Talking about the core, it would have an effect on :

• wp_download_language_pack
• wp_can_install_language_pack

Waiting for you responses ! :raised_hands:

Hey there,

Thanks for your report!

There are indeed some valid reasons these functions rely on DISALLOW_FILE_MODS. But before we add another constant, we should rather think about introducing a new wp_disallow_file_mods() function that checks this function and is filterable. Example:

<?php
function wp_disallow_file_mods() { 
    return apply_filters( 'disallow_file_mods', defined( 'DISALLOW_FILE_MODS' ) && DISALLOW_FILE_MODS ); 
}

See #37436 for some patches that partly implemented this already.

This way, one can easily allow file mods under certain circumstances using the filter.

Related: #38664

If we'd go with a wp_disallow_file_mods() it should probably get a $context argument.

I like this way to handle this ! Thx for the feedback @ocean90 @swissspidy

Moreover, I agree with @ocean90 as we need to pass a
context var to define where it has been called.

If everyone agrees on this I can supply the patch soon ;)

Hah, I was actually going to suggest passing the context to the function and the filter as well.

@MaximeCulea Go for it :-)

Please find the according patch in attached files : patch-38673.diff.

By applying this patch, people can now filter the DISALLOW_FILE_MODS constant. So, to conclude on the way to add languages, or whatever, while DISALLOW_FILE_MODS is activated, it is only needed to filter wp_disallow_file_mods() like this :

<?php add_action( 'disallow_file_mods_download_language_pack', '__return_false' );

@swissspidy, I am now why wondering about doing this .. because we are like trespassing security behaviour of php constants which could not be redefined, isn't it ?

patch-38673.diff​ needs to follow the ​PHP Documentation Standards.

I am now why wondering about doing this .. because we are like trespassing security behaviour of php constants which could not be redefined, isn't it ?

I don't see a problem with that. Constants are not the ideal way to change these things so it makes sense to open it up.

I followed the doc to reformat the filter and add some phpdoc.

Is the following patch-38673-1.diff good now ? @swissspidy

Much better, thanks!

It seems that your last patch miss some lines of the wp_disallow_file_mods function.

Thx @Mista-Flo, had to reinstall git locally cause it crashed on my laptop. As the patch is easy, I just edit it with sublime, but now it is the full one : patch-38673-4.diff !

#38364 was marked as a duplicate.

@swissspidy, are we good here for merge proposal ? ;)

Related to this is #39677.

The original goal from this ticket can be achieved via either, but in the end I think both tickets are valid as they provide different kinds of flexibility.

@flixos90 yeap !

You are playing with roles while I am just refactoring way to check existing roles.

Two tickets providing, as you say, two different ways of flexibility :)

Moving the file mods check to a function is a good step nonetheless. It's not just because of the translations.

@swissspidy +1

In 40394:

Upgrade/Install: Introduce wp_disallow_file_mods() helper function.

This is a wrapper around the checks for the DISALLOW_FILE_MODS constant to determine whether file modifications are disallowed.

Props MaximeCulea.
Fixes #38673.

Just ran into this, and the function name seems incorrect.

How about wp_is_file_mod_allowed() instead?

(wp_disallow_file_mods() reads (to me) like I would use it to disallow file modifications.)

@johnjamesjacoby I totally agree with you on the name.

But as other constants through helpers, I wanted to keep the same (old) pattern as @swissspidy suggested it for naming the method.

Agreed with j³. The function name should use some form of _is_ terminology.

Okay, going to patch it in the 2-3 next hours, then ! :)

"wp_disallow_file_mods" > "wp_is_file_mod_allowed"

Here we are : patch-38673-5.diff.
I have renamed the func name from "wp_disallow_file_mods" to "wp_is_file_mod_allowed" and added some refactoring/documentation.

@MaximeCulea wp_is_file_mod_allowed() has the opposite meaning of wp_disallow_file_mods() so all the checks would need to be negated, i.e. wp_is_file_mod_allowed() === ! wp_disallow_file_mods()

changed the approach for the new function name

I uploaded a patch with the check of opposite asked by @swissspidy , I hope that is fine. I like to have that feature in the next release :-)

In 40638:

Upgrade/Install: After [40394], rename wp_disallow_file_mods() to wp_is_file_mod_allowed().

This makes it more clear what this function is about.

Props Mte90.
Fixes #38673.

In 40639:

Upgrade/Install: After [40638], make sure wp_is_file_mod_allowed() actually returns the right value.

See #38673.