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

ISO DateTime parsing returning wrong Date value #9941

Closed
jclx opened this issue Apr 11, 2023 · 17 comments
Closed

ISO DateTime parsing returning wrong Date value #9941

jclx opened this issue Apr 11, 2023 · 17 comments

Comments

@jclx
Copy link

jclx commented Apr 11, 2023

In recent release it looks like Date-FNS package was removed and TypeOrm is doing it's own parsing but the calculation for calculation is problematic.
In src/util/DateUtils.ts this method was added to replace date-fns.parseISO

 private static parseDateAsISO(dateString: string): Date {
        const date = new Date(dateString)
        const offset = date.getTimezoneOffset() * 60 * 1000
        const utc = date.getTime() + offset

        return new Date(utc)
    }

However the above code returns the datetime with wrong values.
Example:
Given this string: '2023-04-11T16:56:57.572Z'
The above function returns: '2023-04-11T23:56:57.572Z' depending on what timezone your environment is configured for, this example it is -07:00.
Where as date-fns.parseISO() returns: '2023-04-11T16:56:57.572Z' which seems correct.

@ColiZei
Copy link

ColiZei commented Apr 12, 2023

Thanks @jclx !
I faced the same problem after updating to Version 0.3.13.

I got an offset of 1 hour.
All my end to end tests failed after the update.

Moving back to 0.3.12 the end to end tests working fine.
In 0.3.14 the issue still exists.

@fdundjer
Copy link

Thanks for reporting this!
We have the same issue.
We will have to stick to 0.3.12 until this issue is resolved.

@danielpreisinger
Copy link

As it seems, changing @Column types time stamp with time zone to timestamptz fixes this issue for me.

@jclx
Copy link
Author

jclx commented Apr 13, 2023

We can't use 0.3.12 because of windows path bug which is fixed in 0.3.13 but now the timestamp issue.
Thankfully we have testcases that caught this issue and it didn't make it into main code line.

@bubbletap-david-im
Copy link

bubbletap-david-im commented Apr 18, 2023

We have same issue too.

in my case,

typeorm timezone config: 'Z'
DB: MYSQL
DB TImezone: SYSTEM(docker default), so UTC
request save entity time: "2023-01-01T14:00:00.000Z"
saved datetime column: 2023-01-01T05:00:00.000 <- minus 9 hours (-09:00)
findOne datetime column: 2023-01-01T05:00:00.000Z
nestjs service locale: Asia/Seoul (UTC + 09:00)

if service locale change

test case is success
ex)
$ TZ=UTC nest start [service-name]

request save entity time: "2023-01-01T14:00:00.000Z"
saved datetime column: 2023-01-01T14:00:00.000 <- Same
findOne datetime column: 2023-01-01T14:00:00.000Z

@ColiZei
Copy link

ColiZei commented Apr 21, 2023

This ticket got no attention?
We need to update typeorm due to security issues.

@pleerock
Copy link
Member

Please check the CHANGELOG for the affected version, and ping the author of the changes. If you'll create a PR with a clear reproduction of the problem, I'll take a look on it.

@cschuff
Copy link

cschuff commented Apr 21, 2023

Here is an example transformer as a workaround that seems to work for me:

  @Column({
    type: 'datetime',
    nullable: true,
    transformer: {
      to(value: Date | string | null): string {
        return value instanceof Date ? value.toISOString() : value;
      },
      from(value: string | null): Date {
        return value ? new Date(`${value}Z`) : null;
      },
    },
  })

What do you think?

@alanscordoba
Copy link

I'm with the same problem. When recording Datetime type field data in Mysql, it does not respect the correct time. In version 0.3.12 it works correctly.

@ColiZei
Copy link

ColiZei commented Apr 22, 2023

Please check the CHANGELOG for the affected version, and ping the author of the changes. If you'll create a PR with a clear reproduction of the problem, I'll take a look on it.

Hey @pleerock
Here is you can reproduce the problem: https://github.com/ColiZei/typeorm-datetime-issue
Please Read the Readme to reproduce.

Thank!

Hope that helps.

@robinelvin
Copy link

robinelvin commented Apr 26, 2023

Same problem here with MySQL using 0.3.15, downgrading to 0.3.12 fixes the issue.

@mason-proground
Copy link

I have also downgraded to 0.3.12 due to this issue.

felix-gohla added a commit to giz-berlin/typeorm that referenced this issue May 2, 2023
Add a test for date handling functions that seem to ignore the timezone offsets on insert.

Related to typeorm#9941
Introduced in typeorm#9634
@felix-gohla felix-gohla mentioned this issue May 3, 2023
7 tasks
@felix-gohla
Copy link
Contributor

The issue is that there's a difference between how dates (only) and date-times are parsed in ECMA-script. The newly introduced parser (which does not use date-fns) ignores differences between those two and also seems to ignore if a date is given with a timezone according to ISO8601.

@pleerock I created #10011 for a fix. Maybe there's some potential for discussion, however.

felix-gohla added a commit to giz-berlin/typeorm that referenced this issue May 3, 2023
Add a test for date handling functions that seem to ignore the timezone offsets on insert.

Related to typeorm#9941
Introduced in typeorm#9634
felix-gohla added a commit to giz-berlin/typeorm that referenced this issue May 3, 2023
Add handling for the different timezones during parsing ISO dates as there's
a difference in date parsing and date-times parsing in the ECMA-script specs.
Fix the bit where even dates with given timezones were parsed as local time.

Related To typeorm#9941
felix-gohla added a commit to giz-berlin/typeorm that referenced this issue May 3, 2023
Add handling for the different timezones during parsing ISO dates as there's
a difference in date parsing and date-times parsing in the ECMA-script specs.
Fix the bit where even dates with given timezones were parsed as local time.

Related To typeorm#9941
felix-gohla added a commit to giz-berlin/typeorm that referenced this issue May 3, 2023
Add handling for the different timezones during parsing ISO dates as there's
a difference in date parsing and date-times parsing in the ECMA-script specs.
Fix the bit where even dates with given timezones were parsed as local time.

Related To typeorm#9941
felix-gohla added a commit to giz-berlin/typeorm that referenced this issue May 3, 2023
Add a test for date handling functions that seem to ignore the timezone offsets on insert.

Related to typeorm#9941
Introduced in typeorm#9634
felix-gohla added a commit to giz-berlin/typeorm that referenced this issue May 3, 2023
Add handling for the different timezones during parsing ISO dates as there's
a difference in date parsing and date-times parsing in the ECMA-script specs.
Fix the bit where even dates with given timezones were parsed as local time.

Related To typeorm#9941
@jnm733
Copy link

jnm733 commented May 7, 2023

I have now the same issue and also had to downgrade to 0.3.12

I have my local Timezone configured as "Europe/Madrid" (GMT+2)

If I save the value "2023-05-07 14:50:45.768", the real value saved on database is "2023-05-07 12:50:45.767000" and when queried return the value "2023-05-07T10:50:45.770Z" what is wrong.

However, If I save the value "2023-05-07 14:53:13.150Z", the real value saved on database is "2023-05-07 14:53:13.166000" and when queried return the value "2023-05-07T12:53:13.164Z", so it's correct, but on previous versions I did not have to send that "Z" in the end.

My schema is that:
created_at: {
type: 'datetime',
nullable: false,
createDate: true,
},
updated_at: {
type: 'datetime',
nullable: true,
updateDate: true,
},

@pleerock
Copy link
Member

pleerock commented May 9, 2023

Okay, as it always with dates - things are complicated.

I had to revert the previous changes, because it's the most safe solution at the moment.

Meanwhile, we need to analyze all the issues with dates, and came up with some solution most compliant with widely / future adopted standards.

Thanks for the investigation @felix-gohla

@pleerock pleerock closed this as completed May 9, 2023
@NITHISH-1609
Copy link

@pleerock It is still happening in 0.3.17

@ladislasdellinger
Copy link

ladislasdellinger commented Oct 25, 2023

Any update on this issue ? It seems to be still happening for me too in 0.3.17...

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 a pull request may close this issue.