GetValFromStream uses ASSERT which is likely compiled out during release builds

[ann0see]

jamulus/src/protocol.cpp

Lines 2832 to 2833 in 0db5544

	Q_ASSERT ( ( iNumOfBytes > 0 ) && ( iNumOfBytes <= 4 ) );
	Q_ASSERT ( vecIn.Size() >= iPos + iNumOfBytes );

uses Q_ASSERT but they don't do anything (https://doc.qt.io/qt-6/qtassert.html) in release mode. We should check weather an if () statement isn't better suited.

I assume we need to crash in case of an invalid value?

[pljones]

Or drop the packet rather than crash. It's "noise on the line". It's too late to "drop" in the firewall sense but it's the next inspection point.

[ann0see]

The issue is then: What should the function return as value? 0? I don't think this is sound. If we had a std::optional we could return a failure.

[pljones]

OK, semantically it needs treating as an unrecognised request. If, by this point -- i.e. having been "recognised" -- we've already made the fatal mistake of assuming every identified packet is valid, then we've got a bigger problem.