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

Revisit the binary protocol between RTI and federates #410

Open
erlingrj opened this issue Apr 8, 2024 · 5 comments
Open

Revisit the binary protocol between RTI and federates #410

erlingrj opened this issue Apr 8, 2024 · 5 comments

Comments

@erlingrj
Copy link
Collaborator

erlingrj commented Apr 8, 2024

The current binary protocol between RTI and federates is somewhat brittle. Different packages are identified by just a magic byte. Another bug #409 lead to the RTI decoding arbitrary messages from the incoming socket. It would not have been a problem if the binary protocol was a little more robust. E.g we could do:

| magic start bytes | msg_type | length | header checksum | data | magic stop bytes |

This is just a quick idea, we can argue what would be the best protocol. But I do not think we should worry a lot about adding a couple of bytes to each package if it increases the robustness.

@edwardalee
Copy link
Contributor

This sounds like a good idea to me. Perhaps @Jakio815 could comment on how to make this compatible with encryption.

@Jakio815
Copy link
Collaborator

Jakio815 commented Apr 8, 2024

I kind of didn't understand fully. So, we have lots of types of messages going on, you mean we should kind of make a fixed structure of message used in every message?

I didn't get what you mean the 'magic start and end bytes'. @erlingrj

@erlingrj
Copy link
Collaborator Author

Yes, I would propose to have all messages start with the same header. By magic byte start/end bytes I mean that we should pick some random bytes to serve as delimiters. E.g. we could pick 0x4C 0x41 as the "magic start byte" and 0x41 0x4C as the magic end bytes. These happen to be LF and FL if decoded as ASCII .

@Jakio815
Copy link
Collaborator

Jakio815 commented May 2, 2024

@erlingrj I've got some questions, and I will be able to work on this from next week.
So I get the part of |msg type| length | and especially totally agree with the length.

I have two questions.

  1. Header checksum - AFAIK TCP/IP itself has header checksums. Is it a common practice to have checksums in the application layer? I agree that it will make the code more robust, but are there any cases this catching errors?
  2. Magic bytes -
    I'm curious what kind of packages you checked. Also, if we have the length of the payload of the message, do we need this?

@erlingrj
Copy link
Collaborator Author

erlingrj commented May 2, 2024

The measures I am outlining is meant to catch runtime bugs and application bugs. So they are not needed if the code is perfect. Magic bytes and header checksums makes it very likely that the you in fact have a valid message. It would have caught the error here: #409

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

No branches or pull requests

3 participants