open62541 icon indicating copy to clipboard operation
open62541 copied to clipboard

fix(pubsub): fixed program crashing when sending too long strings

Open MichalTyndel opened this issue 2 years ago • 4 comments

Trying to send too long string would result in program crashing: when the string length exceeded maxStringLength, lengthDifference would underflow and we would try to zero out gigabytes of memory. Added check to avoid that.

Would that be the correct approach? to just return with error and try sending the message later? Is this the correct statuscode to return in such case?

MichalTyndel avatar Jan 25 '23 13:01 MichalTyndel

@andreasebner please have a look

jpfr avatar Jan 25 '23 22:01 jpfr

The mentioned error-case should definitely be handled. The standard itself is not very precise on how the handling should look like. In my opinion, there are 2 options:

  1. Check length and return with an error code (this PR).
  2. Encode the string up to the defined maxStringLength without an error. (e.g. maxStringLength = 3; content "hello" --> encode "hel"). @jpfr Do you prefer one of these solutions? @MichalTyndel I think your return code is fine. Another option would be UA_STATUSCODE_BADOUTOFRANGE.

andreasebner avatar Jan 26 '23 10:01 andreasebner

returning an error is preferred.

jpfr avatar Jan 31 '23 14:01 jpfr

Adjusted the code to work with new master. Error is returned as discussed. This disables writerGroup, but it's better than crashing the program. This is out of scope of this PR, but IMO it should be considered how to handle such errors without stopping the writer(or writerGroup).

MichalTyndel avatar Feb 01 '24 12:02 MichalTyndel