Prevent invalid column names (className and length) (#7053)

* Prevent invalid column names

* remove className as invalid

* remove className from beforeSave hook response

* improve tests

Author: dplewis

spec/CloudCode.spec.js

@@ -216,6 +216,21 @@ describe('Cloud Code', () => {
     );
   });
 
+  it('test beforeSave with invalid field', async () => {
+    Parse.Cloud.beforeSave('BeforeSaveChanged', function (req) {
+      req.object.set('length', 0);
+    });
+
+    const obj = new Parse.Object('BeforeSaveChanged');
+    obj.set('foo', 'bar');
+    try {
+      await obj.save();
+      fail('should not succeed');
+    } catch (e) {
+      expect(e.message).toBe('Invalid field name: length.');
+    }
+  });
+
   it("test beforeSave changed object fail doesn't change object", async function () {
     Parse.Cloud.beforeSave('BeforeSaveChanged', function (req) {
       if (req.object.has('fail')) {

spec/ParseObject.spec.js

@@ -701,29 +701,24 @@ describe('Parse.Object testing', () => {
       });
   });
 
-  it('length attribute', function (done) {
+  it('acl attribute', function (done) {
     Parse.User.signUp('bob', 'password').then(function (user) {
       const TestObject = Parse.Object.extend('TestObject');
       const obj = new TestObject({
-        length: 5,
         ACL: new Parse.ACL(user), // ACLs cause things like validation to run
       });
-      equal(obj.get('length'), 5);
       ok(obj.get('ACL') instanceof Parse.ACL);
 
       obj.save().then(function (obj) {
-        equal(obj.get('length'), 5);
         ok(obj.get('ACL') instanceof Parse.ACL);
 
         const query = new Parse.Query(TestObject);
         query.get(obj.id).then(function (obj) {
-          equal(obj.get('length'), 5);
           ok(obj.get('ACL') instanceof Parse.ACL);
 
           const query = new Parse.Query(TestObject);
           query.find().then(function (results) {
             obj = results[0];
-            equal(obj.get('length'), 5);
             ok(obj.get('ACL') instanceof Parse.ACL);
 
             done();
@@ -733,6 +728,21 @@ describe('Parse.Object testing', () => {
     });
   });
 
+  it('cannot save object with invalid field', async () => {
+    const invalidFields = ['className', 'length'];
+    const promises = invalidFields.map(async field => {
+      const obj = new TestObject();
+      obj.set(field, 'bar');
+      try {
+        await obj.save();
+        fail('should not succeed');
+      } catch (e) {
+        expect(e.message).toBe(`Invalid field name: ${field}.`);
+      }
+    });
+    await Promise.all(promises);
+  });
+
   it('old attribute unset then unset', function (done) {
     const TestObject = Parse.Object.extend('TestObject');
     const obj = new TestObject();

spec/ParseQuery.spec.js

@@ -2860,33 +2860,6 @@ describe('Parse.Query testing', () => {
     });
   });
 
-  it('object with length', function (done) {
-    const TestObject = Parse.Object.extend('TestObject');
-    const obj = new TestObject();
-    obj.set('length', 5);
-    equal(obj.get('length'), 5);
-    obj.save().then(
-      function () {
-        const query = new Parse.Query(TestObject);
-        query.find().then(
-          function (results) {
-            equal(results.length, 1);
-            equal(results[0].get('length'), 5);
-            done();
-          },
-          function (error) {
-            ok(false, error.message);
-            done();
-          }
-        );
-      },
-      function (error) {
-        ok(false, error.message);
-        done();
-      }
-    );
-  });
-
   it('include user', function (done) {
     Parse.User.signUp('bob', 'password', { age: 21 }).then(function (user) {
       const TestObject = Parse.Object.extend('TestObject');

src/Controllers/DatabaseController.js

@@ -564,7 +564,7 @@ class DatabaseController {
                 }
                 const rootFieldName = getRootFieldName(fieldName);
                 if (
-                  !SchemaController.fieldNameIsValid(rootFieldName) &&
+                  !SchemaController.fieldNameIsValid(rootFieldName, className) &&
                   !isSpecialUpdateKey(rootFieldName)
                 ) {
                   throw new Parse.Error(
@@ -1213,7 +1213,7 @@ class DatabaseController {
               throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `Cannot sort by ${fieldName}`);
             }
             const rootFieldName = getRootFieldName(fieldName);
-            if (!SchemaController.fieldNameIsValid(rootFieldName)) {
+            if (!SchemaController.fieldNameIsValid(rootFieldName, className)) {
               throw new Parse.Error(
                 Parse.Error.INVALID_KEY_NAME,
                 `Invalid field name: ${fieldName}.`

src/Controllers/HooksController.js

@@ -242,6 +242,7 @@ function wrapToHTTPRequest(hook, key) {
         if (typeof result === 'object') {
           delete result.createdAt;
           delete result.updatedAt;
+          delete result.className;
         }
         return { object: result };
       } else {

src/Controllers/SchemaController.js

@@ -155,6 +155,8 @@ const requiredColumns = Object.freeze({
   _Role: ['name', 'ACL'],
 });
 
+const invalidColumns = ['length'];
+
 const systemClasses = Object.freeze([
   '_User',
   '_Installation',
@@ -422,18 +424,24 @@ function classNameIsValid(className: string): boolean {
     // Be a join table OR
     joinClassRegex.test(className) ||
     // Include only alpha-numeric and underscores, and not start with an underscore or number
-    fieldNameIsValid(className)
+    fieldNameIsValid(className, className)
   );
 }
 
 // Valid fields must be alpha-numeric, and not start with an underscore or number
-function fieldNameIsValid(fieldName: string): boolean {
-  return classAndFieldRegex.test(fieldName);
+// must not be a reserved key
+function fieldNameIsValid(fieldName: string, className: string): boolean {
+  if (className && className !== '_Hooks') {
+    if (fieldName === 'className') {
+      return false;
+    }
+  }
+  return classAndFieldRegex.test(fieldName) && !invalidColumns.includes(fieldName);
 }
 
 // Checks that it's not trying to clobber one of the default fields of the class.
 function fieldNameIsValidForClass(fieldName: string, className: string): boolean {
-  if (!fieldNameIsValid(fieldName)) {
+  if (!fieldNameIsValid(fieldName, className)) {
     return false;
   }
   if (defaultColumns._Default[fieldName]) {
@@ -976,7 +984,7 @@ export default class SchemaController {
   ) {
     for (const fieldName in fields) {
       if (existingFieldNames.indexOf(fieldName) < 0) {
-        if (!fieldNameIsValid(fieldName)) {
+        if (!fieldNameIsValid(fieldName, className)) {
           return {
             code: Parse.Error.INVALID_KEY_NAME,
             error: 'invalid field name: ' + fieldName,
@@ -1060,7 +1068,7 @@ export default class SchemaController {
       fieldName = fieldName.split('.')[0];
       type = 'Object';
     }
-    if (!fieldNameIsValid(fieldName)) {
+    if (!fieldNameIsValid(fieldName, className)) {
       throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `Invalid field name: ${fieldName}.`);
     }
 
@@ -1154,7 +1162,7 @@ export default class SchemaController {
     }
 
     fieldNames.forEach(fieldName => {
-      if (!fieldNameIsValid(fieldName)) {
+      if (!fieldNameIsValid(fieldName, className)) {
         throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `invalid field name: ${fieldName}`);
       }
       //Don't allow deleting the default fields.