aws / aws-sdk-net

The official AWS SDK for .NET. For more information on the AWS SDK for .NET, see our web site:
http://aws.amazon.com/sdkfornet/
Apache License 2.0
2.07k stars 862 forks source link

Critical DateTime to Epoch serialization issue for DynamoDb when date is beyond epoch supported date #3443

Open sander1095 opened 3 months ago

sander1095 commented 3 months ago

Describe the bug

There is a bug in the DynamoDB SDK when converting a DateTime to an epoch (integer). DateTime values that fall outside the supported epoch range (beyond 2038-01-19T03:14:07UTC) fail to convert to epoch and are instead sent to DynamoDB as DateTime objects. This results in inconsistent storage of DateTime values in DynamoDB.

Expected Behavior

The DateTime should always be serialized to epoch and converted back to DateTime when deserializing, regardless of the date.

This is the case when using scanamo in scala, for example, which is how I caught this!

Current Behavior

When a DateTime later than 2038-01-19T03:14:07UTC is stored, it is saved as a DateTime object instead of an epoch integer. The exception is caught and logged, but the original DateTime entry is returned without being converted to epoch.

I believe this is a critical issue, because this causes inconsistent data storage which can cause lots of issues when deserializing because the SDK most likely expects an epoch integer but receives a datetime!

Reproduction Steps

[DynamoDBTable("table")]
public class Item
{

    [DynamoDBHashKey]
    [DynamoDBProperty("id")]
    public string Id { get; set; } = null!;

    [DynamoDBProperty("date", StoreAsEpoch = true)]
    public DateTime Date { get; set; }
}

var db = app.Services.GetRequiredService<IDynamoDBContext>();
var item = new Item { Id = "1", Date = DateTime.Parse("2040-01-01T00:00:00") };
await db.SaveAsync(item);

Possible Solution

See PR #3442 for a solution and other alternatives

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

AWSSDK.DynamoDBv2 3.7.400.2

Targeted .NET Platform

.NET 8

Operating System and version

Windows 11

bhoradc commented 3 months ago

Hello @sander1095,

Thank you for reporting this issue and providing your proposed solution. I will further discuss this issue with the team and we shall get back to you on the PR review.

Regards, Chaitanya

sander1095 commented 3 months ago

Great! I just want to add 1 thing:

Another way to look at this issue:

With the code as-is, after 2038, using StoreAsEpoch will simply not do anything as it will always throw (performance hit!) and store as datetime anyway.

I believe this could be seen as a high priority issue (p1) from this angle :)