From 31cce3ee80426f84d6de35a25c1c1910fc9b87a0 Mon Sep 17 00:00:00 2001 From: AileenCGN Date: Wed, 8 Nov 2017 11:25:31 +0700 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=97=9C=20Set=20db=20soft=20limits?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #8143 Sets soft limits for certain db fields: - `posts`: - `title`: 255 chars (current hard limit: 2,000 chars) - `meta_title`: 300 chars (current hard limit: 2,000 chars) - `meta_description`: 500 chars (current hard limit: 2,000 chars) - `users`: - `bio`: 200 chars (current hard limit: 65,535 chars) - `location`: 150 chars (current hard limit: 65,535 chars) - `meta_description`: 500 chars (current hard limit: 2,000 chars) - `meta_title`: 300 chars (current hard limit: 2,000 chars) - `tags`: - `description`: 500 chars (current hard limit: 65,535 chars) - `meta_title`: 300 chars (current hard limit: 2,000 chars) - `meta_description`: 500 chars (current hard limit: 2,000 chars) --- core/server/data/schema/schema.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/core/server/data/schema/schema.js b/core/server/data/schema/schema.js index 457d7b4cc0b8..aaa67ef79d84 100644 --- a/core/server/data/schema/schema.js +++ b/core/server/data/schema/schema.js @@ -2,7 +2,7 @@ module.exports = { posts: { id: {type: 'string', maxlength: 24, nullable: false, primary: true}, uuid: {type: 'string', maxlength: 36, nullable: false, validations: {isUUID: true}}, - title: {type: 'string', maxlength: 2000, nullable: false}, + title: {type: 'string', maxlength: 2000, nullable: false, validations: {isLength: {max: 255}}}, slug: {type: 'string', maxlength: 191, nullable: false, unique: true}, mobiledoc: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true}, html: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true}, @@ -14,8 +14,8 @@ module.exports = { status: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'draft'}, locale: {type: 'string', maxlength: 6, nullable: true}, visibility: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'public', validations: {isIn: [['public']]}}, - meta_title: {type: 'string', maxlength: 2000, nullable: true}, - meta_description: {type: 'string', maxlength: 2000, nullable: true}, + meta_title: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 300}}}, + meta_description: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 500}}}, author_id: {type: 'string', maxlength: 24, nullable: false}, created_at: {type: 'dateTime', nullable: false}, created_by: {type: 'string', maxlength: 24, nullable: false}, @@ -44,17 +44,17 @@ module.exports = { email: {type: 'string', maxlength: 191, nullable: false, unique: true, validations: {isEmail: true}}, profile_image: {type: 'string', maxlength: 2000, nullable: true}, cover_image: {type: 'string', maxlength: 2000, nullable: true}, - bio: {type: 'text', maxlength: 65535, nullable: true}, + bio: {type: 'text', maxlength: 65535, nullable: true, validations: {isLength: {max: 200}}}, website: {type: 'string', maxlength: 2000, nullable: true, validations: {isEmptyOrURL: true}}, - location: {type: 'text', maxlength: 65535, nullable: true}, + location: {type: 'text', maxlength: 65535, nullable: true, validations: {isLength: {max: 150}}}, facebook: {type: 'string', maxlength: 2000, nullable: true}, twitter: {type: 'string', maxlength: 2000, nullable: true}, accessibility: {type: 'text', maxlength: 65535, nullable: true}, status: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'active'}, locale: {type: 'string', maxlength: 6, nullable: true}, visibility: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'public', validations: {isIn: [['public']]}}, - meta_title: {type: 'string', maxlength: 2000, nullable: true}, - meta_description: {type: 'string', maxlength: 2000, nullable: true}, + meta_title: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 300}}}, + meta_description: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 500}}}, tour: {type: 'text', maxlength: 65535, nullable: true}, last_seen: {type: 'dateTime', nullable: true}, created_at: {type: 'dateTime', nullable: false}, @@ -116,12 +116,12 @@ module.exports = { id: {type: 'string', maxlength: 24, nullable: false, primary: true}, name: {type: 'string', maxlength: 191, nullable: false, validations: {matches: /^([^,]|$)/}}, slug: {type: 'string', maxlength: 191, nullable: false, unique: true}, - description: {type: 'text', maxlength: 65535, nullable: true}, + description: {type: 'text', maxlength: 65535, nullable: true, validations: {isLength: {max: 500}}}, feature_image: {type: 'string', maxlength: 2000, nullable: true}, parent_id: {type: 'string', nullable: true}, visibility: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'public', validations: {isIn: [['public', 'internal']]}}, - meta_title: {type: 'string', maxlength: 2000, nullable: true}, - meta_description: {type: 'string', maxlength: 2000, nullable: true}, + meta_title: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 300}}}, + meta_description: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 500}}}, created_at: {type: 'dateTime', nullable: false}, created_by: {type: 'string', maxlength: 24, nullable: false}, updated_at: {type: 'dateTime', nullable: true}, From 1b4cbe9297ff9d56f94de1455783494aef36693d Mon Sep 17 00:00:00 2001 From: AileenCGN Date: Thu, 9 Nov 2017 11:18:02 +0700 Subject: [PATCH 2/3] Specific error message for isLength validator - Added a specific error message in `validate` fn for `isLength` method. - `validate` will take the `tableName` value as optional argument to provide a user friendly error message when exceeding db soft limits - added more tests for importer --- core/server/data/validation/index.js | 68 ++++++++++----- .../data/importer/importers/data_spec.js | 84 ++++++++++++++++++- 2 files changed, 126 insertions(+), 26 deletions(-) diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js index e79bc8e3bf11..1b8371112321 100644 --- a/core/server/data/validation/index.js +++ b/core/server/data/validation/index.js @@ -190,7 +190,12 @@ validateSchema = function validateSchema(tableName, model) { // TODO: check if mandatory values should be enforced if (model[columnKey] !== null && model[columnKey] !== undefined) { - // check length + // check soft limits first, if present (validations: {isLength: {min, max}}) + if (_.has(schema[tableName][columnKey], 'validations.isLength')) { + validationErrors = validationErrors.concat(validate(strVal, columnKey, schema[tableName][columnKey].validations, tableName)); + } + + // check hard limits (maxlength) if (schema[tableName][columnKey].hasOwnProperty('maxlength')) { if (!validator.isLength(strVal, 0, schema[tableName][columnKey].maxlength)) { message = i18n.t('notices.data.validation.index.valueExceedsMaxLength', @@ -206,7 +211,7 @@ validateSchema = function validateSchema(tableName, model) { } } - // check validations objects + // check any other validations objects if (schema[tableName][columnKey].hasOwnProperty('validations')) { validationErrors = validationErrors.concat(validate(strVal, columnKey, schema[tableName][columnKey].validations)); } @@ -243,7 +248,7 @@ validateSettings = function validateSettings(defaultSettings, model) { matchingDefault = defaultSettings[values.key]; if (matchingDefault && matchingDefault.validations) { - validationErrors = validationErrors.concat(validate(values.value, values.key, matchingDefault.validations)); + validationErrors = validationErrors.concat(validate(values.value, values.key, matchingDefault.validations, 'settings')); } if (validationErrors.length !== 0) { @@ -253,24 +258,28 @@ validateSettings = function validateSettings(defaultSettings, model) { return Promise.resolve(); }; -// Validate default settings using the validator module. -// Each validation's key is a method name and its value is an array of options -// -// eg: -// validations: { isURL: true, isLength: [20, 40] } -// -// will validate that a setting's length is a URL between 20 and 40 chars. -// -// If you pass a boolean as the value, it will specify the "good" result. By default -// the "good" result is assumed to be true. -// -// eg: -// validations: { isNull: false } // means the "good" result would -// // fail the `isNull` check, so -// // not null. -// -// available validators: https://github.com/chriso/validator.js#validators -validate = function validate(value, key, validations) { +/** +* Validate keys using the validator module. +* Each validation's key is a method name and its value is an array of options +* eg: +* validations: { isURL: true, isLength: [20, 40] } +* will validate that a values's length is a URL between 20 and 40 chars. +* +* If you pass a boolean as the value, it will specify the "good" result. By default +* the "good" result is assumed to be true. +* eg: +* validations: { isNull: false } // means the "good" result would +* // fail the `isNull` check, so +* // not null. +* +* available validators: https://github.com/chriso/validator.js#validators +* @param {String} value the value to validate. +* @param {String} key the db column key of the value to validate. +* @param {Object} validations the validations object as described above. +* @param {String} tableName (optional) the db table of the value to validate, used for error message. +* @return {Array} returns an Array including the found validation errors (empty if none found); +*/ +validate = function validate(value, key, validations, tableName) { var validationErrors = []; value = _.toString(value); @@ -286,8 +295,21 @@ validate = function validate(value, key, validations) { validationOptions.unshift(value); - // equivalent of validator.isSomething(option1, option2) - if (validator[validationName].apply(validator, validationOptions) !== goodResult) { + // send a more userfriendly error message for the `isLength` validation, when soft limit is exceeded + if (validationName === 'isLength' && validator[validationName].apply(validator, validationOptions) !== goodResult) { + var message = i18n.t('notices.data.validation.index.valueExceedsMaxLength', + { + tableName: tableName || '', + columnKey: key, + maxlength: validations.isLength.max + }); + + validationErrors.push(new errors.ValidationError({ + message: message, + context: tableName ? tableName + '.' + key : key + })); + // equivalent of validator.isSomething(option1, option2) + } else if (validator[validationName].apply(validator, validationOptions) !== goodResult) { validationErrors.push(new errors.ValidationError({ message: i18n.t('notices.data.validation.index.validationFailed', {validationName: validationName, key: key}) })); diff --git a/core/test/integration/data/importer/importers/data_spec.js b/core/test/integration/data/importer/importers/data_spec.js index fbc56675bef9..53fef1ddcf24 100644 --- a/core/test/integration/data/importer/importers/data_spec.js +++ b/core/test/integration/data/importer/importers/data_spec.js @@ -572,14 +572,92 @@ describe('Import', function () { testUtils.fixtures.loadExportFixture('export-001', {lts:true}).then(function (exported) { exportData = exported; - // change title to 1001 characters - exportData.data.posts[0].title = new Array(2002).join('a'); + // change title to 300 characters (soft limit is 255) + exportData.data.posts[0].title = new Array(300).join('a'); exportData.data.posts[0].tags = 'Tag'; return dataImporter.doImport(exportData); }).then(function () { (1).should.eql(0, 'Data import should not resolve promise.'); }).catch(function (error) { - error[0].message.should.eql('Value in [posts.title] exceeds maximum length of 2000 characters.'); + error[0].message.should.eql('Value in [posts.title] exceeds maximum length of 255 characters.'); + error[0].errorType.should.eql('ValidationError'); + + Promise.all([ + knex('users').select(), + knex('posts').select(), + knex('tags').select() + ]).then(function (importedData) { + should.exist(importedData); + + importedData.length.should.equal(3, 'Did not get data successfully'); + + var users = importedData[0], + posts = importedData[1], + tags = importedData[2]; + + // we always have 1 user, the default user we added + users.length.should.equal(1, 'There should only be one user'); + + // Nothing should have been imported + posts.length.should.equal(0, 'Wrong number of posts'); + tags.length.should.equal(0, 'no new tags'); + + done(); + }); + }).catch(done); + }); + it('doesn\'t import a tag when meta title too long', function (done) { + var exportData; + + testUtils.fixtures.loadExportFixture('export-001', {lts:true}).then(function (exported) { + exportData = exported; + + // change meta_title to 305 characters (soft limit is 300) + exportData.data.tags[0].meta_title = new Array(305).join('a'); + return dataImporter.doImport(exportData); + }).then(function () { + (1).should.eql(0, 'Data import should not resolve promise.'); + }).catch(function (error) { + error[0].message.should.eql('Value in [tags.meta_title] exceeds maximum length of 300 characters.'); + error[0].errorType.should.eql('ValidationError'); + + Promise.all([ + knex('users').select(), + knex('posts').select(), + knex('tags').select() + ]).then(function (importedData) { + should.exist(importedData); + + importedData.length.should.equal(3, 'Did not get data successfully'); + + var users = importedData[0], + posts = importedData[1], + tags = importedData[2]; + + // we always have 1 user, the default user we added + users.length.should.equal(1, 'There should only be one user'); + + // Nothing should have been imported + posts.length.should.equal(0, 'Wrong number of posts'); + tags.length.should.equal(0, 'no new tags'); + + done(); + }); + }).catch(done); + }); + it('doesn\'t import a user user when bio too long', function (done) { + var exportData; + + testUtils.fixtures.loadExportFixture('export-001', {lts:true}).then(function (exported) { + exportData = exported; + + // change bio to 300 characters (soft limit is 200) + exportData.data.users[0].bio = new Array(300).join('a'); + return dataImporter.doImport(exportData); + }).then(function () { + (1).should.eql(0, 'Data import should not resolve promise.'); + }).catch(function (error) { + error[0].message.should.eql('Value in [users.bio] exceeds maximum length of 200 characters.'); error[0].errorType.should.eql('ValidationError'); Promise.all([ From 559f42279dabea36fa85e060a305cfd0d13273fa Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Thu, 9 Nov 2017 14:16:15 +0100 Subject: [PATCH 3/3] simplify logic for translating `isLength` for soft limits - dynamic translation key reading - you can define a custom translation for each validator --- core/server/data/validation/index.js | 81 +++++++++---------- core/server/i18n.js | 5 ++ core/server/translations/en.json | 5 +- .../data/importer/importers/data_spec.js | 2 + 4 files changed, 48 insertions(+), 45 deletions(-) diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js index 1b8371112321..83881fd6cf83 100644 --- a/core/server/data/validation/index.js +++ b/core/server/data/validation/index.js @@ -190,12 +190,7 @@ validateSchema = function validateSchema(tableName, model) { // TODO: check if mandatory values should be enforced if (model[columnKey] !== null && model[columnKey] !== undefined) { - // check soft limits first, if present (validations: {isLength: {min, max}}) - if (_.has(schema[tableName][columnKey], 'validations.isLength')) { - validationErrors = validationErrors.concat(validate(strVal, columnKey, schema[tableName][columnKey].validations, tableName)); - } - - // check hard limits (maxlength) + // check length if (schema[tableName][columnKey].hasOwnProperty('maxlength')) { if (!validator.isLength(strVal, 0, schema[tableName][columnKey].maxlength)) { message = i18n.t('notices.data.validation.index.valueExceedsMaxLength', @@ -211,9 +206,9 @@ validateSchema = function validateSchema(tableName, model) { } } - // check any other validations objects + // check validations objects if (schema[tableName][columnKey].hasOwnProperty('validations')) { - validationErrors = validationErrors.concat(validate(strVal, columnKey, schema[tableName][columnKey].validations)); + validationErrors = validationErrors.concat(validate(strVal, columnKey, schema[tableName][columnKey].validations, tableName)); } // check type @@ -259,28 +254,28 @@ validateSettings = function validateSettings(defaultSettings, model) { }; /** -* Validate keys using the validator module. -* Each validation's key is a method name and its value is an array of options -* eg: -* validations: { isURL: true, isLength: [20, 40] } -* will validate that a values's length is a URL between 20 and 40 chars. -* -* If you pass a boolean as the value, it will specify the "good" result. By default -* the "good" result is assumed to be true. -* eg: -* validations: { isNull: false } // means the "good" result would -* // fail the `isNull` check, so -* // not null. -* -* available validators: https://github.com/chriso/validator.js#validators -* @param {String} value the value to validate. -* @param {String} key the db column key of the value to validate. -* @param {Object} validations the validations object as described above. -* @param {String} tableName (optional) the db table of the value to validate, used for error message. -* @return {Array} returns an Array including the found validation errors (empty if none found); -*/ + * Validate keys using the validator module. + * Each validation's key is a method name and its value is an array of options + * eg: + * validations: { isURL: true, isLength: [20, 40] } + * will validate that a values's length is a URL between 20 and 40 chars. + * + * If you pass a boolean as the value, it will specify the "good" result. By default + * the "good" result is assumed to be true. + * eg: + * validations: { isNull: false } // means the "good" result would + * // fail the `isNull` check, so + * // not null. + * + * available validators: https://github.com/chriso/validator.js#validators + * @param {String} value the value to validate. + * @param {String} key the db column key of the value to validate. + * @param {Object} validations the validations object as described above. + * @param {String} tableName (optional) the db table of the value to validate, used for error message. + * @return {Array} returns an Array including the found validation errors (empty if none found); + */ validate = function validate(value, key, validations, tableName) { - var validationErrors = []; + var validationErrors = [], translation; value = _.toString(value); _.each(validations, function each(validationOptions, validationName) { @@ -295,23 +290,21 @@ validate = function validate(value, key, validations, tableName) { validationOptions.unshift(value); - // send a more userfriendly error message for the `isLength` validation, when soft limit is exceeded - if (validationName === 'isLength' && validator[validationName].apply(validator, validationOptions) !== goodResult) { - var message = i18n.t('notices.data.validation.index.valueExceedsMaxLength', - { - tableName: tableName || '', - columnKey: key, - maxlength: validations.isLength.max - }); + // equivalent of validator.isSomething(option1, option2) + if (validator[validationName].apply(validator, validationOptions) !== goodResult) { + // CASE: You can define specific translations for validators e.g. isLength + if (i18n.doesTranslationKeyExist('notices.data.validation.index.validationFailedTypes.' + validationName)) { + translation = i18n.t('notices.data.validation.index.validationFailedTypes.' + validationName, _.merge({ + validationName: validationName, + key: key, + tableName: tableName + }, validationOptions[1])); + } else { + translation = i18n.t('notices.data.validation.index.validationFailed', {validationName: validationName, key: key}); + } validationErrors.push(new errors.ValidationError({ - message: message, - context: tableName ? tableName + '.' + key : key - })); - // equivalent of validator.isSomething(option1, option2) - } else if (validator[validationName].apply(validator, validationOptions) !== goodResult) { - validationErrors.push(new errors.ValidationError({ - message: i18n.t('notices.data.validation.index.validationFailed', {validationName: validationName, key: key}) + message: translation })); } diff --git a/core/server/i18n.js b/core/server/i18n.js index d69cceeb8866..ac3a1afed4b9 100644 --- a/core/server/i18n.js +++ b/core/server/i18n.js @@ -80,6 +80,11 @@ I18n = { return matchingString; }, + doesTranslationKeyExist: function doesTranslationKeyExist(msgPath) { + var translation = I18n.findString(msgPath); + return translation !== blos.errors.errors.anErrorOccurred; + }, + /** * Setup i18n support: * - Load proper language file in to memory diff --git a/core/server/translations/en.json b/core/server/translations/en.json index e83cce6e7eb7..4c8c1913123d 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -586,7 +586,10 @@ "valueExceedsMaxLength": "Value in [{tableName}.{columnKey}] exceeds maximum length of {maxlength} characters.", "valueIsNotInteger": "Value in [{tableName}.{columnKey}] is not an integer.", "themeCannotBeActivated": "{themeName} cannot be activated because it is not currently installed.", - "validationFailed": "Validation ({validationName}) failed for {key}" + "validationFailed": "Validation ({validationName}) failed for {key}", + "validationFailedTypes": { + "isLength": "Value in [{tableName}.{key}] exceeds maximum length of {max} characters." + } } } } diff --git a/core/test/integration/data/importer/importers/data_spec.js b/core/test/integration/data/importer/importers/data_spec.js index 53fef1ddcf24..688139390104 100644 --- a/core/test/integration/data/importer/importers/data_spec.js +++ b/core/test/integration/data/importer/importers/data_spec.js @@ -606,6 +606,7 @@ describe('Import', function () { }); }).catch(done); }); + it('doesn\'t import a tag when meta title too long', function (done) { var exportData; @@ -645,6 +646,7 @@ describe('Import', function () { }); }).catch(done); }); + it('doesn\'t import a user user when bio too long', function (done) { var exportData;