[SERVER-34949] Improved safety for time_support.h API Created: 11/May/18 Updated: 28/Jun/18 Resolved: 07/Jun/18 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Querying |
| Affects Version/s: | 3.7.9 |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Derick Rethans | Assignee: | Justin Seyster |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||||||||||||||||
| Backport Requested: |
v4.0
|
||||||||||||||||||||||||||||||||||
| Steps To Reproduce: | Apply the patch in https://github.com/mongodb/mongo/compare/master...derickr:objectid Recompile agg_test:
Run ./build/ninja/mongo/db/pipeline/agg_expression_test Result:
|
||||||||||||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||||||||||||
| Description |
|
As part of a specification that I am writing on ObjectIds, I am investigating how drivers and the server handle ObjectId timestamps beyond the positive signed 32-bit range. Because of that, I wrote a C++ unit test to test what the server did with $convert. The result is an abort of the server. |
| Comments |
| Comment by Justin Seyster [ 07/Jun/18 ] |
|
After some discussion, the consensus is that the changes in SERVER-34975 are sufficient for the current release and that any future-proofing would be best done as part of SERVER-29627. |
| Comment by Justin Seyster [ 18/May/18 ] |
|
This failure occurs because 1) the OID-to-date conversion is treating the embedded timestamp as a signed int (as noted in SERVER-34975) and 2) the dateToISOStringUTC function doesn't want to print any date represented as a negative timestamp.
Fixing the problem in SERVER-34975 is sufficient to allow the proposed test (posted in the "Steps to reproduce") to run successfully. However, in discussing the problem, I discovered that some of the time_support.h API (including datToISOStringUTC) is not resilient to imports that overflow the max time_t value. I don't know if we have any platforms with a 32-bit time_t, but I have some proposed changes that will make us a bit safer in that case. |