setAmount can be used to create invalid items which break inventory saving

[Malfrador]

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

1. Set an ItemStack count to an invalid value, for example with player.getInventory().getItem(0).setAmount(128);
2. Disconnect to trigger an inventory save
3. An exception will be thrown.
4. 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