avoid crash due to message overflow via data truncation, adds message limit #defines, increases default buffer size #270
Conversation
patricksurry
changed the title
| @@ -357,7 +357,7 @@ namespace XPC | |||
| } | |||
| if ((dataType & 16) == 16) // Integer array | |||
| { | |||
| const std::size_t TMP_SIZE = 200; | |||
| const std::size_t TMP_SIZE = XPC_MAX_DREF_VALUES; | |||
There was a problem hiding this comment.
unclear why this was limited to 200 when protocol is set to return up to 255
| @@ -382,7 +382,7 @@ namespace XPC | |||
| } | |||
| if ((dataType & 32) == 32) // Byte array | |||
| { | |||
| const std::size_t TMP_SIZE = 1024; | |||
| const std::size_t TMP_SIZE = XPC_MAX_DREF_VALUES; | |||
There was a problem hiding this comment.
strange to have a different cap of 1024 here, since requests ask for max size of 255 values (so count fits in one byte), so even if we find 1024 values we then discard down to 255 anyway
| dataSize, i, connection.id); | ||
| count = 0; | ||
| dataSize = 0; | ||
| } |
There was a problem hiding this comment.
this omits data if it would prevent writing at least a single count byte for all remaining drefs. this will always succeed as long as the message size is larger than the max allowed dref count, e.g. XPC_MAX_MESSAGE_SIZE > 255 + message header.
if there's concern about some clients not dealing with empty value arrays (I only tested the python client where it's fine), this could be changed to return only a single value from the array if there's insufficient space to write 1 byte + 1 float for all remaining drefs.
Summary
This pull request addresses issue 226 where requesting too many datarefs and/or datarefs with large numbers of values could overflow the fixed message buffer of 4096 bytes and cause XPlane to crash to the desktop.
This PR extends the idea in PR266 but rather than simply aborting the message response completely, a dataref that would overflow the buffer is replaced with an empty list (a value count of 0 and no data), allowing a valid response to the client. In those cases, the client will see an empty list for the offending dataref(s) rather than the expected dataref content, and an error message will be logged in
XPCLog.txtlike:This PR also increases the default message buffer from 4096 to 16384 bytes (which is still safe for a UDP packet, and the max message size requested by the Python client), and replaces several other numeric constants repeated throughout the code with #define'd constant values via
XPCLimits.hTesting
Tested via python client by requesting all 4600+ datarefs listed in
Resources/plugins/DataRefs.txtin blocks of 250. No segfault was observed, and drefs that previously would have overflowed are successfully parsed as an empty list by the Python client, with expected error messages logged inXPCLog.txt.