Skip to content

Conversation

@patricksurry
Copy link

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.txt like:

15:05:37.852|ERROR|GETD|ERROR: omitting data (1020 bytes) for dref 243 to avoid exceeding message buffer for connection 1.

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.h

Testing

Tested via python client by requesting all 4600+ datarefs listed in Resources/plugins/DataRefs.txt in 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 in XPCLog.txt.

@patricksurry patricksurry changed the title adds message limit #defines, larger default buffer size, data truncation to avoid message overflow avoid crash due to message overflow via data truncation, adds message limit #defines, increases default buffer size Jan 3, 2022
if ((dataType & 16) == 16) // Integer array
{
const std::size_t TMP_SIZE = 200;
const std::size_t TMP_SIZE = XPC_MAX_DREF_VALUES;
Copy link
Author

Choose a reason for hiding this comment

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

unclear why this was limited to 200 when protocol is set to return up to 255

if ((dataType & 32) == 32) // Byte array
{
const std::size_t TMP_SIZE = 1024;
const std::size_t TMP_SIZE = XPC_MAX_DREF_VALUES;
Copy link
Author

Choose a reason for hiding this comment

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

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;
}
Copy link
Author

Choose a reason for hiding this comment

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

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.

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.

1 participant