From 90af079da8a8651572cbf45468aa7d41e80edff2 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Tue, 8 Dec 2020 15:19:12 -0600 Subject: [PATCH 1/4] Prevent invalid column names --- spec/CloudCode.spec.js | 15 +++++++++++++++ spec/ParseObject.spec.js | 11 +++++++++++ spec/Schema.spec.js | 10 ++++++++++ src/Controllers/SchemaController.js | 5 ++++- 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 2d660df99e..388371695a 100644 --- a/spec/CloudCode.spec.js +++ b/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(); + expect(false).toBe(true); + } 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')) { diff --git a/spec/ParseObject.spec.js b/spec/ParseObject.spec.js index 235bdbd8f9..93afdc8745 100644 --- a/spec/ParseObject.spec.js +++ b/spec/ParseObject.spec.js @@ -2045,4 +2045,15 @@ describe('Parse.Object testing', () => { const object = new Parse.Object('CloudCodeIsNew'); await object.save(); }); + + it('cannot save object with invalid field', async () => { + const obj = new TestObject(); + obj.set('className', 'bar'); + try { + await obj.save(); + expect(false).toBe(true); + } catch (e) { + expect(e.message).toBe('Invalid field name: className.'); + } + }); }); diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index 932eec16d9..6854d43851 100644 --- a/spec/Schema.spec.js +++ b/spec/Schema.spec.js @@ -123,6 +123,16 @@ describe('SchemaController', () => { ); }); + it('validate invalid column names', async () => { + const schema = await config.database.loadSchema(); + try { + await schema.validateObject('Stuff', { className: 'Unknown' }); + expect(false).toBe(true); + } catch (e) { + expect(e.message).toBe('Invalid field name: className.'); + } + }); + it('updates when new fields are added', done => { config.database .loadSchema() diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index a35126f38a..93e86e66a7 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -155,6 +155,8 @@ const requiredColumns = Object.freeze({ _Role: ['name', 'ACL'], }); +const invalidColumns = ['className', 'length']; + const systemClasses = Object.freeze([ '_User', '_Installation', @@ -427,8 +429,9 @@ function classNameIsValid(className: string): boolean { } // Valid fields must be alpha-numeric, and not start with an underscore or number +// must not be a reserved key function fieldNameIsValid(fieldName: string): boolean { - return classAndFieldRegex.test(fieldName); + return classAndFieldRegex.test(fieldName) && !invalidColumns.includes(fieldName); } // Checks that it's not trying to clobber one of the default fields of the class. From e4471bbab620af3c3ab301cde6bf34e000da6721 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Tue, 8 Dec 2020 16:50:29 -0600 Subject: [PATCH 2/4] remove className as invalid --- spec/ParseObject.spec.js | 18 +----------------- spec/ParseQuery.spec.js | 28 +++++++--------------------- spec/Schema.spec.js | 10 ---------- src/Controllers/SchemaController.js | 2 +- 4 files changed, 9 insertions(+), 49 deletions(-) diff --git a/spec/ParseObject.spec.js b/spec/ParseObject.spec.js index 93afdc8745..a81cae2540 100644 --- a/spec/ParseObject.spec.js +++ b/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(); @@ -2045,15 +2040,4 @@ describe('Parse.Object testing', () => { const object = new Parse.Object('CloudCodeIsNew'); await object.save(); }); - - it('cannot save object with invalid field', async () => { - const obj = new TestObject(); - obj.set('className', 'bar'); - try { - await obj.save(); - expect(false).toBe(true); - } catch (e) { - expect(e.message).toBe('Invalid field name: className.'); - } - }); }); diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index 846818fc9f..79b3d2c707 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -2860,31 +2860,17 @@ describe('Parse.Query testing', () => { }); }); - it('object with length', function (done) { + it('object with length', async () => { 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(); - } - ); + try { + await obj.save(); + expect(false).toBe(true); + } catch (e) { + expect(e.message).toBe('Invalid field name: length.'); + } }); it('include user', function (done) { diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index 6854d43851..932eec16d9 100644 --- a/spec/Schema.spec.js +++ b/spec/Schema.spec.js @@ -123,16 +123,6 @@ describe('SchemaController', () => { ); }); - it('validate invalid column names', async () => { - const schema = await config.database.loadSchema(); - try { - await schema.validateObject('Stuff', { className: 'Unknown' }); - expect(false).toBe(true); - } catch (e) { - expect(e.message).toBe('Invalid field name: className.'); - } - }); - it('updates when new fields are added', done => { config.database .loadSchema() diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index 93e86e66a7..900c9e2021 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -155,7 +155,7 @@ const requiredColumns = Object.freeze({ _Role: ['name', 'ACL'], }); -const invalidColumns = ['className', 'length']; +const invalidColumns = ['length']; const systemClasses = Object.freeze([ '_User', From 60bbd4288887eb126e33f40f9e8e72d9eae023b0 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Tue, 8 Dec 2020 22:23:48 -0600 Subject: [PATCH 3/4] remove className from beforeSave hook response --- spec/ParseObject.spec.js | 11 +++++++++++ src/Controllers/DatabaseController.js | 4 ++-- src/Controllers/HooksController.js | 1 + src/Controllers/SchemaController.js | 17 +++++++++++------ 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/spec/ParseObject.spec.js b/spec/ParseObject.spec.js index a81cae2540..aa545e3736 100644 --- a/spec/ParseObject.spec.js +++ b/spec/ParseObject.spec.js @@ -728,6 +728,17 @@ describe('Parse.Object testing', () => { }); }); + it('cannot save object with className field', async () => { + const obj = new TestObject(); + obj.set('className', 'bar'); + try { + await obj.save(); + expect(true).toBe(false); + } catch (e) { + expect(e.message).toBe('Invalid field name: className.'); + } + }); + it('old attribute unset then unset', function (done) { const TestObject = Parse.Object.extend('TestObject'); const obj = new TestObject(); diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 5b6bfc083a..21ba2e9477 100644 --- a/src/Controllers/DatabaseController.js +++ b/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}.` diff --git a/src/Controllers/HooksController.js b/src/Controllers/HooksController.js index 692b2c32a5..9cc5f427e8 100644 --- a/src/Controllers/HooksController.js +++ b/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 { diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index 900c9e2021..a5e7d2838a 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -424,19 +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 // must not be a reserved key -function fieldNameIsValid(fieldName: string): boolean { +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]) { @@ -979,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, @@ -1063,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}.`); } @@ -1157,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. From 56462c8ac809351cb6bc26ddc1ea66f8371bd138 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Wed, 9 Dec 2020 11:44:05 -0600 Subject: [PATCH 4/4] improve tests --- spec/CloudCode.spec.js | 2 +- spec/ParseObject.spec.js | 22 +++++++++++++--------- spec/ParseQuery.spec.js | 13 ------------- 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 388371695a..7bc39a43bb 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -225,7 +225,7 @@ describe('Cloud Code', () => { obj.set('foo', 'bar'); try { await obj.save(); - expect(false).toBe(true); + fail('should not succeed'); } catch (e) { expect(e.message).toBe('Invalid field name: length.'); } diff --git a/spec/ParseObject.spec.js b/spec/ParseObject.spec.js index aa545e3736..31855f9533 100644 --- a/spec/ParseObject.spec.js +++ b/spec/ParseObject.spec.js @@ -728,15 +728,19 @@ describe('Parse.Object testing', () => { }); }); - it('cannot save object with className field', async () => { - const obj = new TestObject(); - obj.set('className', 'bar'); - try { - await obj.save(); - expect(true).toBe(false); - } catch (e) { - expect(e.message).toBe('Invalid field name: className.'); - } + 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) { diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index 79b3d2c707..92daad50a6 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -2860,19 +2860,6 @@ describe('Parse.Query testing', () => { }); }); - it('object with length', async () => { - const TestObject = Parse.Object.extend('TestObject'); - const obj = new TestObject(); - obj.set('length', 5); - equal(obj.get('length'), 5); - try { - await obj.save(); - expect(false).toBe(true); - } catch (e) { - expect(e.message).toBe('Invalid field name: length.'); - } - }); - it('include user', function (done) { Parse.User.signUp('bob', 'password', { age: 21 }).then(function (user) { const TestObject = Parse.Object.extend('TestObject');