fix: invalid date in object causes unhandled promise rejection in beforeSave trigger #7193

sunshineo · 2021-02-15T22:15:50Z

New Pull Request Checklist

I am not disclosing a vulnerability.
I am creating this PR in reference to an issue.

Issue Description

Related issue: #7192

Approach

TODOs before merging

Add test cases
Add entry to changelog
Add changes to documentation (guides, repository pages, in-code descriptions)
Add security check
Add new Parse Error codes to Parse JS SDK

… trigger

codecov · 2021-02-15T22:23:06Z

Codecov Report

Merging #7193 (e0bfdcb) into alpha (20fc4e2) will increase coverage by 0.31%.
The diff coverage is 100.00%.

❗ Current head e0bfdcb differs from pull request most recent head 4d9edb4. Consider uploading reports for the commit 4d9edb4 to get more accurate results

@@            Coverage Diff             @@
##            alpha    #7193      +/-   ##
==========================================
+ Coverage   93.82%   94.13%   +0.31%     
==========================================
  Files         182      182              
  Lines       13629    13635       +6     
==========================================
+ Hits        12787    12835      +48     
+ Misses        842      800      -42

Impacted Files	Coverage Δ
src/triggers.js	`95.42% <100.00%> (+0.06%)`	⬆️
src/RestWrite.js	`94.30% <0.00%> (-0.16%)`	⬇️
src/Adapters/Cache/RedisCacheAdapter.js	`87.71% <0.00%> (+75.43%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8cf0b5e...4d9edb4. Read the comment docs.

mtrezza · 2021-02-15T22:36:51Z

Thanks for this PR. Can you please add a test case that covers the issue this PR intends to fix?

sunshineo · 2021-02-16T18:03:06Z

I don't know how to add a test case that setup beforeSave on a class with Date. Can someone provide some pointers?

mtrezza · 2021-02-16T19:03:31Z

Please look into the .spec.js files, there are test cases for saving an object as well as the beforeSave trigger. Combining both should yield the test case needed.

This would also be the code example that I asked for in the related issue.

sunshineo · 2021-02-22T21:09:53Z

I still have no time to learn the test system. So I spent half an hour to setup a project to demo the bug
https://github.com/sunshineo/parse-bug-demo
I hope the README.md is enough, but I can answer any questions

mtrezza · 2021-02-23T19:55:06Z

Can you merge the latest commit of master?

mtrezza · 2021-02-23T19:56:33Z

From your comment:

I do not see any strong reason that we have to have 111 instead of 142. So I'll change the expected error message in the test and put it on my PR.

The error codes are not arbitrary, we have to pick the right one. As I commented in my PR, there may be an additional issue that we are getting two different error codes and messages for the same error.

mtrezza

Can you please also add a CHANGELOG entry?

src/triggers.js

…resave

sunshineo · 2021-03-10T18:56:47Z

@mtrezza Changelog added

sunshineo · 2021-03-13T01:05:12Z

Hello @mtrezza . I think I addressed everything. Let's merge it

CHANGELOG.md

src/triggers.js

sunshineo · 2021-03-17T23:47:21Z

@mtrezza Have a look now

sunshineo · 2021-03-30T22:33:57Z

Hello, can some one merge this? @mtrezza @ @dplewis @davimacedo

mtrezza · 2021-03-30T22:42:57Z

@sunshineo Can you please merge the current master into it to make the tests pass?

…resave

sunshineo · 2021-03-30T23:32:07Z

I just merged master into this branch.

sunshineo · 2021-06-30T20:43:07Z

@mtrezza @dplewis It's been a while, let's merge this. The change properly return error and fix the bug that hangs the whole server. The error code and message can be improved in the future if user feedback suggest it is confusing

# [5.3.0-alpha.3](parse-community/parse-server@5.3.0-alpha.2...5.3.0-alpha.3) (2022-03-27) ### Features * add MongoDB 5.2 support ([parse-community#7894](parse-community#7894)) ([6b4b358](parse-community@6b4b358))

…community#7911)

# [5.3.0-alpha.4](parse-community/parse-server@5.3.0-alpha.3...5.3.0-alpha.4) (2022-04-04) ### Bug Fixes * custom database options are not passed to MongoDB GridFS ([parse-community#7911](parse-community#7911)) ([a72b384](parse-community@a72b384))

…#7921)

)

# [5.3.0-alpha.5](parse-community/parse-server@5.3.0-alpha.4...5.3.0-alpha.5) (2022-04-09) ### Bug Fixes * security upgrade moment from 2.29.1 to 2.29.2 ([parse-community#7931](parse-community#7931)) ([6b68593](parse-community@6b68593))

…ty#7934)

# [5.3.0-alpha.6](parse-community/parse-server@5.3.0-alpha.5...5.3.0-alpha.6) (2022-04-11) ### Bug Fixes * peer dependency mismatch for GraphQL dependencies ([parse-community#7934](parse-community#7934)) ([b7a1d76](parse-community@b7a1d76))

# [5.3.0-alpha.7](parse-community/parse-server@5.3.0-alpha.6...5.3.0-alpha.7) (2022-04-25) ### Bug Fixes * security upgrade @parse/fs-files-adapter from 1.2.1 to 1.2.2 ([parse-community#7948](parse-community#7948)) ([20fc4e2](parse-community@20fc4e2))

zanderisrael · 2022-04-27T11:31:43Z

I still seeing this in 5.0, whats going on looks like a simple fix...
Is there a workaround I can do for now?

mtrezza · 2022-04-27T11:42:00Z

It seems the changes just need to be reviewed then we can release this as an alpha version.

mtrezza · 2022-04-27T11:43:06Z

src/triggers.js

+        try {
+          response['object'] = request.object._getSaveJSON();
+        } catch (error) {
+          // https://github.com/parse-community/parse-server/issues/7192


Can you pleas remove the comment, it's not necessary

mtrezza · 2022-04-27T11:43:51Z

src/triggers.js

+        try {
+          parseObjectJSON = parseObject.toJSON();
+        } catch (error2) {
+          // https://github.com/parse-community/parse-server/issues/7192


Please remove comment; I think also error2 is unused, so you could remove it as well?

parse-github-assistant · 2022-04-27T11:52:39Z

Thanks for opening this pull request!

🎉 We are excited about your hands-on contribution!

dblythy · 2022-04-28T04:15:40Z

spec/CloudCode.spec.js

+      { __type: 'Date', iso: 'invalidIsoDate' },
+      { __type: 'Date', iso: '' },
+    ];
+    for (const date of dates) {


prefer

await Promise.all(dates.map(async date => { ... }));

parse-community#7192 return validation error properly with beforeSave…

63c297e

… trigger

sunshineo mentioned this pull request Feb 23, 2021

Invalid date iso with beforeSave make server unhandled promise rejection and request hanging #7192

Open

5 tasks

parse-community#7192 add test for the fix

3b7c19a

Merge branch 'master' into validation-beforesave

f6a0c64

mtrezza requested changes Feb 23, 2021

View reviewed changes

src/triggers.js Show resolved Hide resolved

src/triggers.js Outdated Show resolved Hide resolved

src/triggers.js Show resolved Hide resolved

sunshineo added 2 commits March 10, 2021 10:54

Merge remote-tracking branch 'forkedfrom/master' into validation-befo…

7a8cace

…resave

Add changelog

69d92f0

sunshineo force-pushed the validation-beforesave branch from bb27d24 to 69d92f0 Compare March 10, 2021 18:56

No empty catch

2f707bd

mtrezza requested changes Mar 13, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

src/triggers.js Outdated Show resolved Hide resolved

src/triggers.js Show resolved Hide resolved

sunshineo added 2 commits March 14, 2021 12:26

Only try cath on the toJSON call

029625c

Better error message

ef3209a

mtrezza requested changes Mar 14, 2021

View reviewed changes

src/triggers.js Outdated Show resolved Hide resolved

sunshineo added 2 commits March 17, 2021 16:45

Improve changelog and error message

fc2d3a4

Merge branch 'master' into validation-beforesave

bc168b5

Merge remote-tracking branch 'forkedfrom/master' into validation-befo…

e4b0195

…resave

mtrezza and others added 19 commits March 27, 2022 22:44

feat: add MongoDB 5.2 support (parse-community#7894)

6b4b358

chore(release): 5.3.0-alpha.3 [skip ci]

75eca2d

# [5.3.0-alpha.3](parse-community/parse-server@5.3.0-alpha.2...5.3.0-alpha.3) (2022-03-27) ### Features * add MongoDB 5.2 support ([parse-community#7894](parse-community#7894)) ([6b4b358](parse-community@6b4b358))

refactor: upgrade mongodb from 4.3.1 to 4.4.1 (parse-community#7906)

132a3e0

ci: bump environment (parse-community#7907)

d74b6a3

refactor: add missing schema definitions (parse-community#7917)

2b0957c

fix: custom database options are not passed to MongoDB GridFS (parse-…

a72b384

…community#7911)

refactor: upgrade @apollo/client from 3.5.8 to 3.5.9 (parse-community…

ef3680a

…#7921)

fix: security upgrade moment from 2.29.1 to 2.29.2 (parse-community#7931

6b68593

)

chore(release): 5.3.0-alpha.5 [skip ci]

e88fb72

# [5.3.0-alpha.5](parse-community/parse-server@5.3.0-alpha.4...5.3.0-alpha.5) (2022-04-09) ### Bug Fixes * security upgrade moment from 2.29.1 to 2.29.2 ([parse-community#7931](parse-community#7931)) ([6b68593](parse-community@6b68593))

fix: peer dependency mismatch for GraphQL dependencies (parse-communi…

b7a1d76

…ty#7934)

chore(release): 5.3.0-alpha.6 [skip ci]

34b8110

# [5.3.0-alpha.6](parse-community/parse-server@5.3.0-alpha.5...5.3.0-alpha.6) (2022-04-11) ### Bug Fixes * peer dependency mismatch for GraphQL dependencies ([parse-community#7934](parse-community#7934)) ([b7a1d76](parse-community@b7a1d76))

refactor: upgrade @graphql-tools/links from 8.2.2 to 8.2.4 (parse-com…

43ad5b3

…munity#7932)

refactor: upgrade @graphql-tools/links from 8.2.4 to 8.2.6 (parse-com…

b45d44e

…munity#7935)

refactor: upgrade @graphql-tools/links from 8.2.6 to 8.2.7 (parse-com…

8b91961

…munity#7941)

refactor: upgrade body-parser from 1.19.1 to 1.19.2 (parse-community#…

122afcf

…7944)

refactor: upgrade @graphql-tools/links from 8.2.7 to 8.2.8 (parse-com…

8868628

…munity#7945)

fix: security upgrade @parse/fs-files-adapter from 1.2.1 to 1.2.2 (pa…

20fc4e2

…rse-community#7948)

Merge branch 'alpha' into validation-beforesave

4d9edb4

mtrezza requested a review from a team April 27, 2022 11:40

mtrezza requested changes Apr 27, 2022

View reviewed changes

mtrezza changed the title ~~fix validation error with beforeSave trigger~~ fix: invalid date in object causes unhandled promise rejection in beforeSaveTrigger Apr 27, 2022

mtrezza changed the title ~~fix: invalid date in object causes unhandled promise rejection in beforeSaveTrigger~~ fix: invalid date in object causes unhandled promise rejection in beforeSave trigger Apr 27, 2022

dblythy suggested changes Apr 28, 2022

View reviewed changes

mtrezza force-pushed the alpha branch 2 times, most recently from 59215e6 to e6d7d8f Compare May 1, 2022 02:29

Uh oh!

fix: invalid date in object causes unhandled promise rejection in beforeSave trigger #7193

Are you sure you want to change the base?

fix: invalid date in object causes unhandled promise rejection in beforeSave trigger #7193

Uh oh!

Conversation

sunshineo commented Feb 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New Pull Request Checklist

Issue Description

Approach

TODOs before merging

Uh oh!

codecov bot commented Feb 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mtrezza commented Feb 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sunshineo commented Feb 16, 2021

Uh oh!

mtrezza commented Feb 16, 2021

Uh oh!

sunshineo commented Feb 22, 2021

Uh oh!

mtrezza commented Feb 23, 2021

Uh oh!

mtrezza commented Feb 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrezza left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sunshineo commented Mar 10, 2021

Uh oh!

sunshineo commented Mar 13, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sunshineo commented Mar 17, 2021

Uh oh!

sunshineo commented Mar 30, 2021

Uh oh!

mtrezza commented Mar 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sunshineo commented Mar 30, 2021

Uh oh!

sunshineo commented Jun 30, 2021

Uh oh!

zanderisrael commented Apr 27, 2022

Uh oh!

mtrezza commented Apr 27, 2022

Uh oh!

mtrezza Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

mtrezza Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

parse-github-assistant bot commented Apr 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Thanks for opening this pull request!

Uh oh!

dblythy Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sunshineo commented Feb 15, 2021 •

edited

Loading

codecov bot commented Feb 15, 2021 •

edited

Loading

mtrezza commented Feb 15, 2021 •

edited

Loading

mtrezza commented Feb 23, 2021 •

edited

Loading

mtrezza commented Mar 30, 2021 •

edited

Loading

parse-github-assistant bot commented Apr 27, 2022 •

edited

Loading