Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GETD check remaining buffer size before adding data #266

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daemotron
Copy link

Summary

This pull request addresses #226

It adds a check to MessageHandlers::HandleGetD, ensuring no data will be added to the response buffer if the remaining space in the buffer is not sufficient to accept the data. As described in #226, this can lead to a segmentation fault, resulting in a CTD of X-Plane itself.

Consequences:

The current fix leaves the request unanswered (no message being sent), so the connection will time out on client side.

Alternate Solutions:

  1. Instead of aborting, send an incomplete result (use break instead of return in line 547).
  2. Instead of aborting, send a dedicated message, indicating that too many data were requested.

Advantages of alternate solutions

Client receives at least some data (1) or clear information why the request failed (2)

Disadvantages of alternate solutions

Requires modification of all client libraries to

  • check the response actually contained the number of data refs indicated in byte 5 (1)
  • interpret the response when an error message is submitted

Conclusion

The current fix is not perfect, but prevents the simulator from crashing, and doesn't require modification of the client libraries.

@patricksurry
Copy link

Here's an improvement on this idea that will return a valid response to the client in these cases: #270

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.

2 participants