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

Packet Integrity Check #245

Closed
yhju opened this issue Mar 26, 2013 · 10 comments
Closed

Packet Integrity Check #245

yhju opened this issue Mar 26, 2013 · 10 comments

Comments

@yhju
Copy link
Contributor

yhju commented Mar 26, 2013

Actual implementation to prevent communication failure due to malformed bytestream.

@ghost ghost assigned yhju Mar 26, 2013
@stonier
Copy link
Member

stonier commented Mar 26, 2013

What are you thinking about here YH? I haven't been able to think up any improvements on this one.

@stonier
Copy link
Member

stonier commented Aug 20, 2013

Follow this up and close it.

@yhju
Copy link
Contributor Author

yhju commented Aug 26, 2013

Done by 4648538.

@yhju yhju closed this as completed Aug 26, 2013
@yhju yhju reopened this Aug 29, 2013
@yhju
Copy link
Contributor Author

yhju commented Aug 29, 2013

found another place for improvements

@yhju
Copy link
Contributor Author

yhju commented Aug 29, 2013

Done.

@yhju yhju closed this as completed Aug 29, 2013
@stonier
Copy link
Member

stonier commented Aug 29, 2013

This a relatively large commit - can you briefly explain what you did here?

Was there also a second commit? If so, link it.

@yhju
Copy link
Contributor Author

yhju commented Aug 30, 2013

All related commits are already listed in this page.

9 month ago, I added length field in sub-payload of protocol for compatibility among various firmware - driver pairs. Issue #168.

But, driver was not actually using it in packet validation. There was also no actual exception handling on validation failure, when packet class return false on deserialize call, such as removing malformed sub-payload. It will fall in infinite while loop.

Commit Summary

4648538:

  • added packet validity check in each packet class.

27c62af:

  • added fixPayload function, by moving code of default case in packet parsing.
  • when packet class return false on deserialize call, fixPayload will actually fix it.
  • added getPayload function on packet_handler that provide just payload only, except header, length field of payload, and checksum for more clear packet parsing and processing.

@stonier
Copy link
Member

stonier commented Aug 30, 2013

Ok, thanks.

Do we need to test this thoroughly next week or have you been using it for a while?

@yhju
Copy link
Contributor Author

yhju commented Aug 30, 2013

It is very recent commit, and I tested it shortly. We need some test.

@stonier
Copy link
Member

stonier commented Aug 31, 2013

I would prefer not to have critical driver functionality updates going in the day before a major release without any testing.

Would have been better more than a week ago, or delayed until some decent testing on it saw it through. We'll manage this time, but please be a bit more careful before a major product release next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants