TeamMessage: internal crash on large team messages

[mellmann]

Setup: robots send large team messages > 128 byte. Behavior GUI: GC stops counting team messages (the number stays the same) everything else works as before.

Error message in terminal:

thread 'tokio-runtime-worker' panicked at 'called `Result::unwrap()` on an `Err` value: A message sent on a datagram socket was larger than the internal message buffer or some other network limit, or the buffer used to receive a datagram into was smaller than the datagram itself. (os error 10040)', D:\RoboCup\downloads\GameController3.git\game_controller_runtime\src\lib.rs:122:71

[ahasselbring]

https://github.com/rust-lang/rust/issues/55794

so I either have to use a buffer that is larger than the maximum UDP packet size on Windows or handle the error. The latter has the disadvantage that I don't get the sender address.

[mellmann]

In general: It's against the rules to use messages larger than 128bytes. So it would be good. Maybe display a large warning such that a referee can act acordingly?. What is the penalty for violation?

re: UDP size in Windows. I'm not sure what you mean. What is the buffer size that you are using now? The buffer is certainly larger than 128bytes. It was easily able to accomodate the message sizes allowed in old rules.

[mellmann]

I think the problem is here: the buffer is precisely enough to hold the 128 byte message.

https://github.com/ahasselbring/GameController3/blob/7a522a052779a822e05be3382dc3be290385f41f/game_controller_net/src/team_message_receiver.rs#LL66C17-L66C17

        let mut buffer = vec![0u8; TEAM_MESSAGE_MAX_SIZE + 1];

So the following test will never trigger ;) (or maybe for only for a message with 129 bytes)

too_long: length > TEAM_MESSAGE_MAX_SIZE

Maybe this later test is not necessary. Instead the message-too-large-error could be explicitely caught and handeled. Maybe with match? https://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/book/first-edition/error-handling.html#unwrapping-explained

(I'm not a rust programmer, so just the first idea that I found)

[ahasselbring]

The buffer is exactly one byte longer because on all operating systems with "standard" socket implementations (i.e. not Windows, but technically sockets are so broken that there is no "standard"), messages that are longer than the receive buffer are only truncated but do not raise an error. The one additional byte therefore allows detection if the packet was too long (or in other words, on Unix systems all messages that are too long are exactly 129 bytes long). Windows is (to my knowledge, and until you opened this issue I didn't even know about this at all) the only operating system where recv_from triggers an error when the received datagram does not fit into the user-supplied buffer. The problem with matching the result of the recv_from call is that we don't get a sender address in the error case (In Rust, you get either a success with the results (sender address and actual length) or en error (with an error code, but without results) - the length isn't needed in this error case because we know it's the full length of the buffer). Having the sender address allows inspecting the log file later to see which robot sent the illegal message (arguably a malicious agent could spoof it easily, but this won't happen in RoboCup).

In fact, I have already a local commit that just uses a larger buffer size on Windows (which is the simpler solution in my opinion), but sources differ on how large a UDP packet can be in the worst case. Also, I haven't booted a Windows yet to test it.

The current application behavior on platforms where recv_from behaves in the "standard" way is the same as if too many messages are sent: The number of goals drops to 0, future goals are not counted, and the number of goals and the number of messages are colored in fuchsia-400. (In fact, the message counter "freezes" too because future messages do not matter anymore, but I may change that so that you can still see them counting down.)

By the way, the old GameController used a message buffer of exactly the message length, so it would never detect if teams sent messages that were too large. There was probably no exception in that case either because the Java socket library ignores WSAEMSGSIZE on Windows.

[ahasselbring]

The problem should be fixed now.