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

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

Open
skippone opened this issue Mar 31, 2021 · 4 comments
Assignees

Comments

@skippone
Copy link

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:

} else if (mapperType.match(/^(Date|DateTime|DateTimeRfc1123)$/gi) !== null) {
payload = new Date(responseBody);

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.

@jeremymeng
Copy link
Contributor

@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
Copy link
Contributor

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

@xirzec
Copy link
Member

xirzec commented Apr 19, 2021

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?

@jeremymeng
Copy link
Contributor

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.

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

No branches or pull requests

3 participants