fix: Parse.Object.saveEventually fails if server is restarting while calling it

[mortenmo]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.
• Link this pull request to an issue.

Issue

Closes: 2028

As issue describes, save fails due to the ACL object being stored to JSON and back causing the following error when connection is back:

Error: ACL must be a Parse ACL.
    at ParseObjectSubclass.value (ParseObject.js:1259:16)
    at ParseObjectSubclass.value (ParseObject.js:1578:31)
    at EventuallyQueue.js:386:27

Approach

This is a quick fix using the saveJson format instead when storing the object locally. There might be a better representation of the object in a string store if someone can tell me what that is.

[parse-github-assistant]

Thanks for opening this pull request!

[mtrezza]

What's the diff to toJSON?

[mortenmo]

The main thing that makes this work is that _getSaveJSON() (outside the mock) does not include the ACL object while toJSON does. In the mocks, there is no real different why the tests didn't fail already they don't

When using saveEventually and connection is down, the object gets stored in the queue as json. Today it uses 'getJSON' and that includes a json representation of ACL. _getSaveJSON doesn't.

This json object is later used (line 276) to save the queued object when the server is back up:

await object.save(queueObject.object, queueObject.serverOptions);

save here does not support ACL in json format (or ACL at all, don't think you can change access outside of server).

There are TBH probably even better fixes here. We should probably only store the dirty fields of the object and not overwrite everything (but it does already make sure before line 276 that its not overwriting other things).

[mortenmo]

Reason why the EventuallyQueue tests didn't catch this is because ACLs aren't on the objects before they are loaded from the servers.

Doing this..

x = new Parse.Object('TestObject');
  ParseObject2 {id: undefined, _localId: undefined, _objCount: 1762, className: 'TestObject'}
x.set('foo', 'bar');
x.toJSON()
{foo: 'bar'}
x.save();

Would work fine. But in reality you often modify existing objects. When objects are loaded from server, the toJSON includes a lot more properties. Here is the same object:

x = await new Parse.Query('TestObject').find();
x[0].toJSON()

   {"foo":"bar",
    "createdAt":"2024-03-17T20:52:35.213Z",
    "updatedAt":"2024-03-17T20:53:30.133Z",
    "ACL":{"*":{"read":true,"write":true}},
    "objectId":"sZhK7r5owf"}

All tests in EventuallyQueue-test just creates non-server saved objects which doesn't catch this problem with "real" objects from servers.

example:

const object = new ParseObject('TestObject');
    object.set('foo', 'bar');
    object.set('test', '1234');

[dplewis]

@mortenmo Thanks for the PR! I think the validation check on ACL should removed instead of removing ACL from Eventually Queue. I honestly don't know why the ACL check is there. .validate is called in both save and set , save also calls set. set transform the ACL json into Parse.ACL (which has it own checks) then performs the .validate then throws the error.

The following integration test fails.

it('can set ACL from toJSON', async () => {
    Parse.User.enableUnsafeCurrentUser();
    const user = new Parse.User();
    const object = new TestObject();
    user.set('username', 'torn');
    user.set('password', 'acl');
    await user.signUp();
    const acl = new Parse.ACL(user);
    object.setACL(acl);
    const json = object.toJSON();
    await object.save(json); // This breaks ACL must be a Parse.ACL.
 });

Here is an integration test replicate the issue for this PR.

it('can saveEventually on object with ACL', async () => {
    Parse.User.enableUnsafeCurrentUser();
    const parseServer = await reconfigureServer();
    const user = new Parse.User();
    user.set('username', 'torn');
    user.set('password', 'acl');
    await user.signUp();

    const acl = new Parse.ACL(user);
    const object = new TestObject({ hash: 'saveSecret' });
    object.setACL(acl);

    await new Promise((resolve) => parseServer.server.close(resolve));

    await object.saveEventually();

    let length = await Parse.EventuallyQueue.length();
    assert(Parse.EventuallyQueue.isPolling());
    assert.strictEqual(length, 1);

    await reconfigureServer({});

    while (Parse.EventuallyQueue.isPolling()) {
      await sleep(100);
    }
    assert.strictEqual(Parse.EventuallyQueue.isPolling(), false);

    length = await Parse.EventuallyQueue.length();
    while (length) {
      await sleep(100); // <---------------------- stalls as data is lost
    }
    length = await Parse.EventuallyQueue.length();
    assert.strictEqual(length, 0);

    const query = new Parse.Query('TestObject');
    query.equalTo('hash', 'saveSecret');
    let results = await query.find();
    while (results.length === 0) {
      results = await query.find();
    }
    assert.strictEqual(results.length, 1);
});

Removing the ACL check will fix this issue.

[dplewis]

Fixed via #2097