Setting blog upload limit to 0 doesn't work

[boonebgorges]

There's a check in get_space_allowed() that looks like this:

$space_allowed = get_option( 'blog_upload_space' );
// ...
if ( empty( $space_allowed ) ... ) {
    $space_allowed = 100;
}

This means that setting 'blog_upload_space' to 0 can never work. This seems to be a bug: I'd expect that setting it to 0 would mean that uploads were effectively disabled for that site.

This behavior was previously noted https://core.trac.wordpress.org/ticket/19538#comment:10 and changed [19639], but was later reverted [19652] to avoid breaking existing behavior in a minor release.

[jorbin]

In 34606:

Revert change from true to false in multisite upload test

This change was introduced in [34601]. 0 is a special snowflake. The code treats it as if it should fall back to the default, rather than respecting 0 as 0. Essentially, 0 means 100. WAT? This has been discussed previusly in https://core.trac.wordpress.org/ticket/19538#comment:10. This test should return True right now, but the behavior should be discussed and in the future 0 may mean 0.

See #34037
Props jorbin, boonebgorges

[azaozz]

Seems the proper solution is to revert r19652. However wondering what changes to the UI will be needed to support this case (uploads are not allowed). Should the "Media" menu and the "Add Media" button above the editor be hidden? Or should they still be shown but with the appropriate error messages? Empty media library looks... sad.

[jeremyfelt]

In 34898:

Tests: Improve tests for get_space_allowed().

• Move all tests to a new multisite/getSpaceAllowed.php
• Store original blog_upload_space values before the class is loaded.
• Restore these values after each test tear down to avoid pollution.
• Add a comprehensive set of expectations and results via data provider.

See #34037.

[jeremyfelt]

In 34899:

Tests: Improve tests for is_upload_space_available().

• Move all tests to a new 'multisite/isUploadSpaceAvailable.php'
• Store original blog_upload_space values before the class is loaded.
• Restore these values after each test tear down to avoid pollution.

Commit to testing is_upload_space_available() itself by always filtering the amount of space used through pre_get_space_used. This allows us to sanely test without worrying about the local environment. In the future, we may be able to remove some of these tests as get_upload_space_available() tests are built out and reliable.

See #34037.

[jeremyfelt]

In 34900:

Tests: Account for non-numeric strings in tests for get_space_allowed().

Provides a specific test for a case where a non-numeric strings are assigned to blog_upload_space at both the site and network levels.

See #34037.

[jeremyfelt]

More history:

• [19314] set the check for the site level option to a strict === false before deferring to the network option. Some original tests are up here ​http://gsoc.svn.wordpress.org/2011/jakub.tyrcha/test_wpms.php
• [19639] introduced the ! is_numeric() check.

[jeremyfelt]

Once we fix get_space_allowed() so that 0 is a valid return, we then run into upload_is_user_over_quota():

	$space_allowed = get_space_allowed();
	if ( empty( $space_allowed ) || !is_numeric( $space_allowed ) )
		$space_allowed = 10; // Default space allowed is 10 MB

Which could then calculate slightly wrong. More tests needed!

[jeremyfelt]

In 34901:

Tests: Improve tests for upload_is_user_over_quota().

• Move all tests to a new 'multisite/uploadIsUserOverQuota.php'
• Use get_space_allowed and pre_get_space_used filters to help avoid test pollution.

See #34037.

[jeremyfelt]

In 34902:

Tests: Remove static capture of space used and space allowed.

Originally introduced in [34601], this was necessary to avoid test pollution. Now that things have been separated and written to play nicely, we can remove this.

See #34037.

[jeremyfelt]

34037.diff modifies get_uploads_space() to only accept a numeric value and drops the check for empty. It also adjusts the existing tests accordingly.

While the matching UI isn't perfect, it does exist for other cases where uploads have been restricted. The screenshots attached to this ticket show the current state when attempting to add media while space allowed is 0. We can probably improve this experience with a nicer message in another ticket.

[jeremyfelt]

In 35016:

MS: Allow for a blog_upload_space setting of 0 to restrict uploads.

Previously, an value matching empty() would have been bypassed in favor of the default setting for 100MB.

Related #19538, r19639, r19652, where we saw the bug, fixed the bug, and then unfixed the bug so that it was not a surprise in a point release.

See #34037.

[jeremyfelt]

In 35017:

MS: Handle the possibility of 0 when checking a user's upload quota.

Upload space of 0 is now more possible via r35016 and should be respected rather than modified to a default of 10MB.

Fixes #34037.