sqlite date hydration is susceptible to corruption

[jacobg]

Issue type:

[ ] question
[X] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[X] cordova
[ ] mongodb
[ ] mssql
[ ] mysql / mariadb
[ ] oracle
[ ] postgres
[ ] cockroachdb
[X] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[X] latest
[ ] @next
[ ] 0.x.x (or put your version here)

Steps to reproduce or a small repository showing the problem:

The AbstractSqliteDriver.prepareHydratedValue function contains this code:

            /**
             * Fix date conversion issue
             *
             * If the format of the date string is "2018-03-14 02:33:33.906", Safari (and iOS WKWebView) will convert it to an invalid date object.
             * We need to modify the date string to "2018-03-14T02:33:33.906Z" and Safari will convert it correctly.
             *
             * ISO 8601
             * https://www.w3.org/TR/NOTE-datetime
             */
            if (value && typeof value === "string") {
                value = value.replace(" ", "T") + "Z";
            }

            value = DateUtils.normalizeHydratedDate(value);

This code works fine if the datetime text stored in sqlite is of the format "2018-03-14 02:33:33.906". But sqlite also supports storing datetime strings in ISO 8601 format "2018-03-14T02:33:33.906Z". So if it's already stored in sqlite in ISO 8601 format, then typeorm will change the string to "2018-03-14T02:33:33.906ZZ", i.e., two Zs the end. Or alternatively, it would change "2018-03-14T02:33:33.906+00:00" to "2018-03-14T02:33:33.906+00:00Z", which is also invalid. The result is that typeorm will call the Date constructor on the invalid date string, and the Date object will also be an invalid date when the entity class is instantiated.

The reason this code normally works fine is that if the database row was saved using typeorm, then typeorm will save the date in the expected format. But if the database row was already created outside typeorm, then it is valid to save a sqlite database row in that ISO 8601 format. So because it's valid sqlite datetime format, typeorm should also support it.

I think the correct solution would be to only change the date string if the date string format exactly matches what is expected, i.e., a regex like /\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d\.\d\d\d/.

[Kononnable]

@jacobg Why don't you make a PR with such changes? We will see if such approach doesn't break any tests etc.

[jacobg]

Thanks @Kononnable. I'm working on that.

I'm trying to write a unit test for it, but when I call npm run test, the unit test is not copied to the build\compiled folder, and thus it is not run. However, the test entity class I defined is copied. Here are screenshots. Am I doing something wrong?

[Kononnable]

I don't have a clue why it would behave like that. Push changes to your fork or add a PR with an annotation that it didn't compiled as expected and we will see what we can do. Without access to the code it will be difficult to determine what's wrong.

[jacobg]

Hi @Kononnable, Here's my fork:
https://github.com/jacobg/typeorm/tree/issue-3949

It has the unit test, but it doesn't have the fix yet, but I need to get the unit test running first.

I just cloned the repo, and ran:

npm i
npm test

But it ignores my unit test, and it's not under build/compiled/test, although the test entity class is.

Thanks!

[jacobg]

Haha. I accidentally added a space to the end of the filename, and so tsc skipped it.

[jacobg]

@Kononnable
Would you mind please taking a look at my PR, and either merging it or giving feedback? Thanks!

#4050