Object attributes transformed in array

[aclec]

New Issue Checklist

• I am not disclosing a vulnerability.
• I am not just asking a question.
• I have searched through existing issues.
• I can reproduce the issue with the latest versions of Parse Server and the Parse JS SDK.

Issue Description

When I get an Object attribute on my object with Parse 5.2, it returns an array instead of the expected object.
(Parse 5.1 return an Object)

Steps to reproduce

Get an Object attribute with parse 5.2

Server

• Parse Server version: 7.1

Database

• System (MongoDB or Postgres): MongoDB
• Database version: 7
• Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): Local

Client

• Parse JS SDK version: 5.2

Logs

Parse 5.1:

Parse 5.2:

[parse-github-assistant]

Thanks for opening this issue!

• 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

[mtrezza]

The server version did not change, only the JS SDK version? Could you open a PR with a failing test that demonstrates the issue?

[aclec]

Yes, the server is in 7.1.
The two screen was taken by changing only the sdk from 5.1 to 5.2
When I downgrade to 5.1, it reworks.

[mtrezza]

Could you add a test so we can verify the issue? And maybe share a simple example code here to understand how exactly you are getting to this result.

[mtrezza]

@aclec I have added a PR with a test, see parse-community/parse-server#9174. It's using the Parse JS SDK 5.2.0. I cannot get it to fail. Could you please take a look and share how we can reproduce this issue?

Edit: I was able to reproduce the issue, updated the test. The issue occurs when the field key is a number string. Could you please try this out with lower versions of the JS SDK and let us know which version introduced the issue? Please also try try the alpha versions.

[mtrezza]

The issue seems to come from #2120. A field value of object { 1: 1 } gets converted to an array [1] here:

Parse-SDK-JS/src/ObjectStateMutations.ts

Lines 188 to 198 in 4ce1f24

	// Check for JSON array { '0': { something }, '1': { something } }
	if (
	val &&
	typeof val === 'object' &&
	!Array.isArray(val) &&
	Object.keys(val).length > 0 &&
	Object.keys(val).some(k => !isNaN(parseInt(k))) &&
	!['sentPerUTCOffset', 'failedPerUTCOffset'].includes(attr)
	) {
	val = Object.values(val);
	}

I think there are 2 issues:

• If a key is a number, it is interpreted as an array index. But in JS a number string is a valid key name. So this representation may need to be changed because it's conflicting with valid JS object key names.
• There is a bug in the logic above where a key name containing a mix of numbers and letters gets converted to a number by parseInt despite containing letters. That leads to @aclec's object key 120x120 being converted to number 120 and the field value gets converted to an array, which is the reported issue.

I suspect a broader issue with how nested fields are represented:

// Check for JSON array { '0': { something }, '1': { something } }

also in:

Parse-SDK-JS/src/__tests__/ObjectStateMutations-test.js

Line 320 in 4ce1f24

items: { '0': { count: 20 }, '1': { count: 5 } },

A nested field should be represented with dots. If instead it's represented by an object where a number key represents an array index, it conflicts with objects that contain number keys. I believe the fix in #2194 does not really address the underlying issue. It excludes sentPerUTCOffset: { '1': { count: 20 } }, from the conversion based on the key name, but a fix should actually allow a key name to be a number.

A quick fix for @aclec's reported issue would be to change this check:

Parse-SDK-JS/src/ObjectStateMutations.ts

Line 194 in 4ce1f24

Object.keys(val).some(k => !isNaN(parseInt(k))) &&

to:

Object.keys(val).some(k => k === String(Number(k)) && Number.isInteger(Number(k)))

This avoids that the key 120x120 is converted to a number and then interpreted as an array index, but instead it's correctly identified as NaN. See #2201 where the test with key 1x1 passes.

But the underlying issue seems to be the conflicting representation, see also #2201 where the test with key 1 fails. Not sure how complex the solution is, maybe @dplewis has more insight here?

[aclec]

I see that .some() is being used. Shouldn't we, in addition to checking that it's a number, use .every() to ensure that all keys are numbers?
It might be a misunderstanding on my part, but it seems to me that the transformation occurs as soon as there is a number?

[mtrezza]

I think you could be right. But I also think this whole logic should be removed, because an array cannot be represented by key names that are numbers. Number strings are valid key names in JS and MongoDB.

Worst case we may have to remove the dot notation feature from the JS SDK.

[aclec]

Can you merge your pull request so that I can update my SDK to the next version?
Removing all the logic would be a breaking change for some people, I think.

We need to use dot notation to access nested values, but if it's not an array, we should not transform it. An object must stay an object.

The bug did not exist before. Maybe we should revert the changes?
Or is it better to delete the logic?

[mtrezza]

#2201 treats a symptom of something that is fundamentally broken, so I'm hesitant to merge it. I believe reverting #2120 and #2194 may be the correct way forward, since it seems to be based on an incorrect implementation. That would be a breaking change for people who use the feature, but the feature was adding a bug for other people, so the trade off I believe would be justified to merge it as a simple bug fix. However, the best way forward would be to fix the underlying bug.

[aclec]

I will try to create a patch today to pass the tests.

The goal is not to convert an object into an array; the object should remain as is until the end.

I will try today to see if we can reconcile everything.