Attachments count inconsistency in the Restrictions line

[isaak654]

Description

This was reported on the Bug Reports board:
https://www.simplemachines.org/community/index.php?topic=582435.0

[sbulen]

I cannot reproduce this issue as described. I suspect #7497 might help, as it addresses "hidden" attachments that exist only in the session but are not visible in the UI.

I'll leave this open for now in case others can reproduce.

[jdarwood007]

https://github.com/SimpleMachines/SMF/blob/release-2.1/Sources/Post.php#L1229

This line of code is causing this. The dropzone code is using this to update the interface, while the UI natively doesn't appear to be.

[sbulen]

Yeah, the issue occurs if your SMF settings exceed your php.ini settings. I can reproduce it now if I do this. The PHP defaults are ~20.

So... I think the proper fix is to not allow your SMF settings to exceed your php.ini settings.

And the temp fix on smorg is to drop the SMF setting down to 20; it was never really 36 anyway...

[jdarwood007]

Well the UI was displaying 36 and not the 20, so it is using the wrong context variable there to display that.

[sbulen]

Correct - the 36 was entered as the SMF limit. The UI should never have allowed an SMF limit > the php.ini limit. I.e., it should never have allowed you to set it to 36.

The proper fix is to ensure the UI doesn't allow a # greater than php.ini allows, in this case, 20.

...and changing the SMF limit to 20, which it really was all along.

[jdarwood007]

Well the code below was using attachmentNumPerPostLimit from modSettings to display that restriction. It should instead be using, $context['num_allowed_attachments']. As it would otherwise be possible to still display this incorrectly.

[jdarwood007]

@sbulen I can put together a PR for this unless your working on one already.

[sbulen]

If the UI had a valid limit edit, attachmentNumPerPost would have been 20.

Keep in mind some folks may have that set to 4, or 8. The SMF default is 4.

It needs to honor what the admin sets in the UI, and further, the admin should not be allowed to set it to something that exceeds php.ini.

[sbulen]

@sbulen I can put together a PR for this unless your working on one already.

Depends on the fix... The PR should focus on the UI edit.

(And changing the limit on smorg.)

[jdarwood007]

Limit fixed on sm.org

I was thinking of changing that line to this:

		$context['num_allowed_attachments'] = $modSettings['attachmentNumPerPostLimit'] = min(ini_get('max_file_uploads'), (empty($modSettings['attachmentNumPerPostLimit']) ? 50 : $modSettings['attachmentNumPerPostLimit']));

Which will have modSettings show the UI limit when it is used later.

I was checking into the settings page, we show a "(0 to disable)" message, but if we update the string I would think it should say something like "(0 to disable, PHP max: ###" and link to the phpinfo. But it requires a language string update.

[jdarwood007]

I should point out, even though the PHP max is 20, that if you upload 20, you can then upload the last 16 separately. The PHP max would be only for what is sent in a single $FILES. So in reality, it could be higher than the PHP limit, but it would require multiple upload sessions.

[sbulen]

If you wish to support that, then it should have been displaying the 36 all along, not the 20, and ignoring the php.ini setting.

Go ahead, submit a PR, I'll test it.

[jdarwood007]

No, this change would make it show the 20. My reasoning is SMF won't know if the PHP ini changes, so if we are doing this, we need all user interfaces to reflect that 20 limit.