Skip to content

fix(spec): accept Java's long memory-size units and reject overflow - #621

Merged
JingsongLi merged 1 commit into
apache:mainfrom
jackylee-ch:fix/memory-size-parity
Jul 28, 2026
Merged

fix(spec): accept Java's long memory-size units and reject overflow#621
JingsongLi merged 1 commit into
apache:mainfrom
jackylee-ch:fix/memory-size-parity

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

parse_memory_size only recognized short unit spellings, so a valid Java option
value such as target-file-size = '256 mebibytes' parsed as None and the caller
silently fell back to its default. It also multiplied unchecked:
'9007199254740993 kb' wraps to a negative size in a release build and panics in
a debug build.

Fix: accept every alias in Java MemorySize.MemoryUnit (bytes, kibibytes,
mebibytes, gibibytes, tebibytes alongside the short forms), and use
checked_mul so an overflowing value returns None — the callers then fall back
to their default, which is what Java's thrown "numeric overflow" leads to as well.

`parse_memory_size` only recognized short unit spellings, so a valid Java
option value such as `target-file-size = '256 mebibytes'` was silently
dropped and the caller fell back to its default. It also multiplied
unchecked: `'9007199254740993 kb'` wrapped to a negative size in a release
build and panicked in a debug build.

Accept every alias in Java `MemorySize.MemoryUnit` (`bytes`, `kibibytes`,
`mebibytes`, `gibibytes`, `tebibytes` alongside the short forms) and use
`checked_mul`, so an overflowing value returns `None` and the caller falls
back to its default instead of wrapping.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@JingsongLi
JingsongLi merged commit 009d610 into apache:main Jul 28, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants