setAmount can be used to create invalid items which break inventory saving
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 12.7k
- Forks
- 3.5k
- Avg merge
- 3d 13h
- Merged PRs (30d)
- 11
Description
Expected behavior
ItemStack#setAmount should be restricted to 1-99 and refuse values outside the allowed range, instead of directly setting the count field of the underlying NMS stack without any checks.
Observed/Actual behavior
ItemStack#setAmount can be used to set the count to any value. Setting it to a value outside the allowed range of 1-99 causes saving to break for the inventory the item stack is in (e.g. the player's), as the ItemStack codec will throw when trying to parse the item:
IllegalStateException: Value must be within range [1;99]: 128
The entire inventory will not save, not just the invalid stack:
[03:38:25 WARN]: Failed to save player data for Malfrador
net.minecraft.ReportedException: Saving entity NBT
Steps/models to reproduce
- Set an ItemStack count to an invalid value, for example with
player.getInventory().getItem(0).setAmount(128); - Disconnect to trigger an inventory save
- An exception will be thrown.
- Log back in and notice your inventory did not save
Plugin and Datapack List
not relevant
Paper version
This server is running Paper version 1.21-109-master@5a5035b (2024-07-23T08:16:30Z) (Implementing API version 1.21-R0.1-SNAPSHOT)
Will affect all versions since 1.20.6.
Other
While its not really the job of the API to protect people from doing stupid things, this is imo bad enough that there should be a check in place.
This is additionally made worse by the fact that before 1.20.6, item stack counts up to 127 were handled by the game perfectly fine. Someone may use an older plugin that uses this behaviour and accidentally break their inventories in the process. IMO throwing an exception here would be needed
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating ItemStack#setAmount in the Paper API implementation and inspect nearby ItemStack tests, if present. Verify the allowed count boundaries and the behavior for invalid values; done means invalid counts can no longer create stacks that break inventory saving.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend-api-design
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100