Bug #1710
closeddisaster frequency value > 255 cause network problem
0%
Description
In packets.def the disaster frequency is UINT8, needs to be UINT32. The ruleset value could in theory be as high as 1000000 if you wanted the disaster to always happen. Currently values > 255 get truncated.
If game.ruleset has -
[disaster_5]
...
frequency=50000
get message -
"Trying to put 50000 into 8 bits; it will result 80 at receiving side."
Pretty rare, so I suggest targeting 3.3.0
Files
Updated by Marko Lindqvist 14 days ago
Dean Brown wrote:
In packets.def the disaster frequency is UINT8, needs to be UINT32. The ruleset value could in theory be as high as 1000000 if you wanted the disaster to always happen. Currently values > 255 get truncated.
The max should be 1000, I think, as it gets multiplied by the server setting that has max of 1000 (1000*1000=1000000). The server setting values that user may set should be meaningful, or the ruleset should lock the setting (to 1000).
Updated by Marko Lindqvist 12 days ago
Marko Lindqvist wrote in #note-2:
Dean Brown wrote:
In packets.def the disaster frequency is UINT8, needs to be UINT32. The ruleset value could in theory be as high as 1000000 if you wanted the disaster to always happen. Currently values > 255 get truncated.
The max should be 1000, I think
So correct type would be uint16. It's not that using the two extra bytes would be any way significant, but in the name of self-documenting code uint16 would show the intendet usage better.
Updated by Marko Lindqvist 12 days ago
- Target version changed from 3.3.0 to 3.3.0-npf
Updated by Marko Lindqvist 12 days ago
- Blocks Tasks #1712: S3_3 network protocol freeze (npf) added
Updated by Dean Brown 12 days ago
- File 1710.patch 1710.patch added
My example of frequency=50000 was a mistake on my part, I was wrong about what my server setting was. With server setting "disasters", 1000 I can change the usual disaster's frequency to 1 without changing their actual result chance of .001, and then frequency=255 for my added disaster gets me to .255, much closer to what I want. So 1000 as a max value seems right, and UINT16 is OK. Updated the patch.
There is no bounds checking in the ruleset load code, except implicit "read an int" must limit value to 2**31.
Updated by Marko Lindqvist 12 days ago
- Status changed from New to In Review
- Assignee set to Marko Lindqvist