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

Do not throw an exception on times before 19700101 #503

Merged

Conversation

eboasson
Copy link
Contributor

This stops throwing the C++ wrapper from throwing an exception in the dds:core::Time class on trying to construct a time stamp prior to the 00:00:00 UTC on 1 January 1970. There is no reason to do that and it makes it vulnerable to receiving samples with earlier time stamps.

It is tricky to test: Cyclone won't allow writing these. I've tested it in a rather convoluted manner by hand ...

This stops throwing the C++ wrapper from throwing an exception in the dds:core::Time class
on trying to construct a time stamp prior to the 00:00:00 UTC on 1 January 1970. There is
no reason to do that and it makes it vulnerable to receiving samples with earlier time
stamps.

Signed-off-by: Erik Boasson <[email protected]>
Copy link
Contributor

@noxpardalis noxpardalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine to me in principal and since Cyclone DDS (Core) prevents writing with such invalid timestamps and can handle receiving them it makes sense to propagate the handle of this to the Core. Being liberal in what we consume and conservative in what we produce and all that.

However, from an internal discussion I think some more work might be needed in the back and forth mapping between the Core and C++ bindings' notions of invalid timestamps (i.e. DDS_TIME_INVALID and DDSI_TIME_INVALID for the Core).

Approving this as it fixes an issue now and another PR/investigation can be done to fixup/confirm the mapping of timestamps.

@eboasson eboasson merged commit 86eda0c into eclipse-cyclonedds:master Aug 15, 2024
17 of 20 checks passed
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

Successfully merging this pull request may close these issues.

C++ DataReader throws exceptions when receiving samples with negative source_timestamp
2 participants