Skip to content

Fix timestamp96 decoding at zero seconds - #1190

Merged
redboltz merged 1 commit into
msgpack:cpp_masterfrom
fhgffy:fix/timestamp96-zero-seconds-20261007
Oct 8, 2026
Merged

redboltz merged 1 commit into
msgpack:cpp_masterfrom
fhgffy:fix/timestamp96-zero-seconds-20261007

Conversation

@fhgffy

@fhgffy fhgffy commented Oct 7, 2026

Copy link
Copy Markdown

A timestamp96 value with seconds=0 and nanoseconds=500 currently decodes to 1 microsecond instead of 0 through both as and convert. With a seconds-resolution destination, it becomes 1 second. The equivalent timestamp64 representation correctly truncates to 0.

The timestamp96 decoder sends sec == 0 through the negative-second adjustment. Change both conditions to sec >= 0, so nonnegative fractions use the same truncation direction as timestamp64. Negative-second behavior is unchanged.

Add a regression with explicit timestamp96 bytes and a microsecond-resolution time_point, covering both public conversion paths. Using an explicit duration avoids hiding the bug on hosts whose system clock has nanosecond precision.

Fresh validation on Linux x86-64 with GCC 14.2.0 and Boost 1.85.0:

  • Added regression fails on the unmodified base with two assertions and passes with the fix
  • Full C++11 CTest suite: 39/39 passed
  • Targeted C++20 msgpack_cpp11 and object_with_zone tests with UBSan: 2/2 passed
  • Separate edge checks pass under C++11 and C++20, API versions 1/2/3, warning-as-error flags, and UBSan; equivalent timestamp32/64/96 values and negative/zero/positive neighboring seconds are covered
  • Independent review found no blocking issues; a separate deterministic property test passes 841,152 assertions per configuration across six duration types

Focused test: ./build/test/msgpack_cpp11 --run_test=system_clock_96_zero_seconds

Full C++20, non-Linux platforms, and out-of-range destination durations were not tested. Timestamp96 is a legal representation for these values under the timestamp specification, even when a serializer could choose a shorter format.

Treat zero seconds as nonnegative when converting the timestamp96
fraction to chrono durations. This keeps truncation consistent with
timestamp64 and leaves negative-second handling unchanged.

Add an explicit microsecond-resolution regression for both as and convert.
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.91%. Comparing base (39cc081) to head (0f0fdfb).
⚠️ Report is 24 commits behind head on cpp_master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@              Coverage Diff               @@
##           cpp_master    #1190      +/-   ##
==============================================
- Coverage       85.36%   79.91%   -5.46%     
==============================================
  Files              80       80              
  Lines            5099     5790     +691     
  Branches          507      629     +122     
==============================================
+ Hits             4353     4627     +274     
- Misses            746     1163     +417     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fhgffy
fhgffy marked this pull request as ready for review October 7, 2026 09:36
@redboltz

redboltz commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

@fhgffy , thank you for sending the PR. It looks good to me.

@redboltz
redboltz merged commit 2900c64 into msgpack:cpp_master Oct 8, 2026
25 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.

3 participants