ms-rest-js icon indicating copy to clipboard operation
ms-rest-js copied to clipboard

Date-only deserialization is not correct when local time uses negative UTC offset

Open skippone opened this issue 4 years ago • 4 comments

When operationSpec for an API response specifies an object property with format "Date" its value is not deserialized to Javascript Date object as expected.

Swagger / Open API example:

"dateOfIssue": {
          "format": "date",
          "description": "Date of issue",
          "type": "string"
        },

For example when value "2021-03-31" is supplied by API in aforementioned property dateOfIssue the resulting Date object is actually deserialized as "2021-03-30" because the local timezone offset is applied.

Code in question is: https://github.com/Azure/ms-rest-js/blob/f6f0d92d79a1dfa8d92ff0891b88bc6b7a349e69/lib/serializer.ts#L214-L215

This is due to a rather strange behaviour of Javascript Date object when local time is set to negative UTC offset and only a date value is passed to its contructor:

// Timezone set to -6
new Date("2021-03-31")  // is in fact interpreted as "Tue Mar 30 2021 18:00:00 GMT-0600 (Central Standard Time)"

The above issue does not apply to positive UTC offsets.

However, when time fraction is supplied (with midnight) it yields expected results:

// Timezone set to -6
// when provided with time fraction (and without explicit time zone) it is constructed properly
new Date("2021-03-31T00:00:00")  // is in fact interpreted as "Wed Mar 31 2021 00:00:00 GMT-0600 (Central Standard Time)"

I believe that Date only values should be interpreted strictly as local date regardless of currently used time zone offset. The following fix could be used to provide such a behaviour:

else if (mapperType.match(/^(Date)$/ig) !== null) {
      payload = new Date(responseBody + "T00:00:00");
}
else if (mapperType.match(/^(DateTime|DateTimeRfc1123)$/ig) !== null) {
      payload = new Date(responseBody);
}

The very same behaviour is actually also present in the newer version of azure-sdk-for-js.

skippone avatar Mar 31 '21 14:03 skippone

@skippone Thanks for the feedback! This is an interesting problem (as always for any Date related issues). We will investigate a bit deeper then get back to you.

jeremymeng avatar Apr 05 '21 17:04 jeremymeng

Appending the time portion sounds reasonable to me to ensure we have the same date from date-only strings. @chradek @xirzec @bterlson any concerns?

jeremymeng avatar Apr 19 '21 19:04 jeremymeng

Oh this is a fun one. I am not sure if there is going to be a bulletproof solution here, but as a default behavior this might be good enough.

The counter-example is a date-only string that is something like a validity period of a certificate, but I suppose most reasonable start boundaries might have time/timezone info rather than just a date?

xirzec avatar Apr 19 '21 21:04 xirzec

The counter-example is a date-only string that is something like a validity period of a certificate, but I suppose most reasonable start boundaries might have time/timezone info rather than just a date?

yes for things that are important I'd expect time instead of just a date.

jeremymeng avatar Jun 15 '21 20:06 jeremymeng