-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Make MSP more robust #2948
Make MSP more robust #2948
Conversation
92f5ac2
to
9bf11ec
Compare
@shellixyz if it's tested - I'm ok with merging it |
I’m just a bit worried of the exact size checks. Some buggy client implementation might be sending extra bytes in the payload, which have been harmless until now. With these changes, we might break them. Of course, if we were starting from scratch I’d enforce exact sizes, but at this point I’m not sure if checking for >= would be a better idea. |
@fiam good point. I'd just ensure we have no less bytes than we need |
No problems with the |
I tested this while doing normal configuration through the GUI. Not all the messages were tested. Basically all I did was add the Should I change the conditions to |
I think for compatibility reasons it's better to have |
…ze <=` for MSPv1 (compat)
I changed |
Make all MSP SET messages read operations safe and add a couple of bound checks.