Prevent invalid column names (className and length)

[dplewis]

Fixes: #7047, #6195

There could be more of the reserved fields that could mess up the database or produce unexpected results.

[codecov]

Codecov Report

Merging #7053 (56462c8) into master (b398894) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #7053      +/-   ##
==========================================
- Coverage   93.86%   93.85%   -0.01%     
==========================================
  Files         169      169              
  Lines       12437    12442       +5     
==========================================
+ Hits        11674    11678       +4     
- Misses        763      764       +1     

Impacted Files	Coverage Δ
src/Controllers/DatabaseController.js	95.31% <100.00%> (ø)
src/Controllers/HooksController.js	94.69% <100.00%> (+0.04%)	⬆️
src/Controllers/SchemaController.js	97.32% <100.00%> (+0.21%)	⬆️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js	92.92% <0.00%> (-0.67%)	⬇️
src/RestWrite.js	93.67% <0.00%> (+0.16%)	⬆️

---

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 b398894...56462c8. Read the comment docs.

[mtrezza]

Maybe also authData?

[mtrezza]

Just cosmetics...

[mtrezza]

LGTM 🚀

[penchef]

Hey,

so it turns out preventing the column name length is a breaking change (at least for some people). Having this breaking change wrapped up in the security relevant 4.5.0 release makes it a huge problem to stay on top of security issues. It would be nice to

1. allow length again for a 4.5.1 release
2. re-introduce this in a subsequent PR + [document|https://parseplatform.org/Parse-SDK-JS/api/master/Parse.Object.html] the prohibit column names

I'd suggest to rename forbidden fields to _length, _className etc. like parse does with the _User class. Adding a length field is not that unlikely.

[pausenabre]

What is the work around for this breaking change? We are currently using classes in production with the length field.

Do we need to do a migration of the Classes (PFObjects in iOS) that currently use the length field?
at the moment attempting to save objects with length prompts a Error 105, and we can not delete the field neither as we get the same error:

Error Domain=Parse Code=105 "Invalid field name: length." UserInfo={error=Invalid field name: length., NSLocalizedDescription=Invalid field name: length., code=105}

I would also suggest to allow the use of length again and some documentation on how to migrate existing classes using length or any other prohibit names.

[julianvogels]

To add to this (@pausenabre is my colleague):
We are using the a length column on one of our Parse classes. Haven't been aware that this is a restricted field. The issues we're seeing after upgrading to Parse Server 4.5.0 are that we can't save any objects with the length property set (from iOS).

As a workaround, we were trying to unset the length key in a beforeSave hook. That works great for any object which is newly created, but not for object updates (tested with REST API POST and PUT). When updating an object, the beforeSave hook is not called at all and Parse Server returns an error before I can unset the key. So the first solution would be to call the beforeSave hook for updates before returning the error about the length key.

We are also using the Parse iOS SDK local datastore, where in our production app, users have already saved objects which contain data for the length key. Once a user saves the local object to the server, they would see this error. We were not able to find a way to unset the length key client-side on iOS. That would be the second solution to the issue.

[mtrezza]

I agree that allowing virtually any keyword as field name would be reasonable. The currently "reserved" field names could be allowed by encoding fields differently in the Parse Object.

This would merit a new issue to have a discussion, as this PR has already been merged to fix another issue. Anyone please feel free to open a new issue to make the case for allowing restricted field names and we can try to work out a concept.