Skip to content

[3.0] Parse memory settings that carry no unit designator - #9324

Merged
Sesquipedalian merged 1 commit into
SimpleMachines:release-3.0from
albertlast:fix/sapi-memory-return-bytes
Aug 14, 2026
Merged

[3.0] Parse memory settings that carry no unit designator#9324
Sesquipedalian merged 1 commit into
SimpleMachines:release-3.0from
albertlast:fix/sapi-memory-return-bytes

Conversation

@albertlast

@albertlast albertlast commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Note

This change was produced by an LLM. The code, the commit message and this
description were all written by Claude (Anthropic), driven by @albertlast. It has
not yet had human code review.

Everything stated below was verified by actually running it, rather than only
reasoned about. Even so, please review it as untrusted work: the diagnosis may be
right while the fix is not what SMF would prefer stylistically or architecturally.

Description

Sapi::memoryReturnBytes() removes the last character of the value before parsing
the number, on the assumption that it is always a unit designator:

$num = \intval(substr($val, 0, \strlen($val) - 1));
$last = strtolower(substr($val, -1));

PHP's shorthand notation is optional, so a setting may be a plain byte count. When it
is, the last digit is eaten: '128' reads as 12, and '2097152' reads as 209715.

Sources/Graphics/Image.php does exactly that. It works out how much memory an image
needs and passes the raw byte count with no designator:

return Sapi::setMemoryLimit((string) $needed_memory, true);

so image resizing asks for a tenth of what it just calculated.

The other value carrying no designator is -1, which is how PHP spells "no limit".
substr('-1', 0, 1) is '-', and intval('-') is 0, so an unlimited server reports
zero bytes available. setMemoryLimit() then finds the current limit smaller than
anything at all and imposes one.

This strips the last character only when it is a designator PHP actually accepts, and
reports "no limit" as PHP_INT_MAX so that the four call sites comparing it against an
amount they need do not each have to special case it. The dead is_integer() check at
the top went with it; the parameter is typed string, so it never fired.

How this was verified

PHP 8.4.23. Same script before and after the change.

Parsing:

value before after
'256M' 268435456 268435456
'1G' 1073741824 1073741824
'512K' 524288 524288
'128' 12 128
'2097152' 209715 2097152
'-1' 0 PHP_INT_MAX
' 64M ' 67108864 67108864

Then through the caller, with memory_limit set to -1:

before:  setMemoryLimit('128M') -> memory_limit is now 128M
after:   setMemoryLimit('128M') -> memory_limit is still -1

Nothing that already worked changes: every value with a designator parses to the same
number as before.

php -l and PHP-CS-Fixer clean.

Relationship to other PRs

Sources/Sapi.php is not touched by any other open PR.

Found while writing unit tests for the stateless parts of the codebase (#9326). That
branch carries the regression tests for this fix, and reverting this file alone fails
exactly those tests. This PR is deliberately independent of it, so it can be merged on
its own.

Issues References (Fixes|Related|Closes)

  1. No existing issue found for this.
  2. Related: [3.0][Testing] Add a PHPUnit suite for the parts that need no database #9326

memoryReturnBytes() removed the last character of the value before
parsing the number, on the assumption that it is always a designator.
PHP's shorthand notation is optional, so a plain byte count loses its
last digit: '128' reads as 12, and '2097152' reads as 209715.

Graphics\Image does exactly that, passing a computed byte count with no
designator, so resizing an image asks for a tenth of the memory it just
worked out that it needs.

The other value with no designator is '-1', which means there is no
limit. It read as 0, because intval('-') is 0, so setMemoryLimit() found
the current limit to be smaller than anything and set one. On a server
with no memory limit, asking for 128M capped it at 128M.

Only strips the last character when it is one of the designators PHP
accepts, and reports "no limit" as PHP_INT_MAX so that the callers
comparing it against an amount they need do not each have to special
case it.

The dead is_integer() check went with it; the parameter is typed string.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Sesquipedalian

Sesquipedalian commented Aug 14, 2026

Copy link
Copy Markdown
Member

We should check whether this is also an issue in SMF 2.1 and fix it if so.

EDIT: Yes, it is. I'll submit a PR for that shortly.

@jdarwood007 jdarwood007 modified the milestones: 3.0 Alpha 6, 3.0 Alpha 5 Aug 14, 2026
@Sesquipedalian
Sesquipedalian merged commit f4eb5c2 into SimpleMachines:release-3.0 Aug 14, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants