From 0f3cb442271a94d8af78f4bac531b0845d30be8a Mon Sep 17 00:00:00 2001 From: Joerg Henning Date: Sun, 27 Mar 2016 18:19:32 +0800 Subject: [PATCH] deps: validator@5.1.0 closes #6462 - monkey-patch validator.extends() since it was dropped by validator @5.0.0 - coerce input to string prior to validation (custom toString func) - need to handle boolean validation based on column type not isIn() - use `lodash.tostring` to convert input values to strings --- core/server/data/schema/schema.js | 6 +-- core/server/data/validation/index.js | 37 ++++++++++--- core/server/models/post.js | 5 +- core/server/models/user.js | 3 ++ core/server/translations/en.json | 1 + .../integration/model/model_posts_spec.js | 52 +++++++++++++++++++ .../integration/model/model_users_spec.js | 14 +++++ package.json | 3 +- 8 files changed, 109 insertions(+), 12 deletions(-) diff --git a/core/server/data/schema/schema.js b/core/server/data/schema/schema.js index 6bb36fc952..b15079f454 100644 --- a/core/server/data/schema/schema.js +++ b/core/server/data/schema/schema.js @@ -8,8 +8,8 @@ module.exports = { mobiledoc: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true}, html: {type: 'text', maxlength: 16777215, fieldtype: 'medium', nullable: true}, image: {type: 'text', maxlength: 2000, nullable: true}, - featured: {type: 'bool', nullable: false, defaultTo: false, validations: {isIn: [[0, 1, false, true]]}}, - page: {type: 'bool', nullable: false, defaultTo: false, validations: {isIn: [[0, 1, false, true]]}}, + featured: {type: 'bool', nullable: false, defaultTo: false}, + page: {type: 'bool', nullable: false, defaultTo: false}, status: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'draft'}, language: {type: 'string', maxlength: 6, nullable: false, defaultTo: 'en_US'}, visibility: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'public', validations: {isIn: [['public']]}}, @@ -157,7 +157,7 @@ module.exports = { app_id: {type: 'integer', nullable: false, unsigned: true, references: 'apps.id'}, relatable_id: {type: 'integer', nullable: false, unsigned: true}, relatable_type: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'posts'}, - active: {type: 'bool', nullable: false, defaultTo: true, validations: {isIn: [[0, 1, false, true]]}}, + active: {type: 'bool', nullable: false, defaultTo: true}, created_at: {type: 'dateTime', nullable: false}, created_by: {type: 'integer', nullable: false}, updated_at: {type: 'dateTime', nullable: true}, diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js index f5c9b42898..450f7c078e 100644 --- a/core/server/data/validation/index.js +++ b/core/server/data/validation/index.js @@ -1,11 +1,13 @@ var schema = require('../schema').tables, _ = require('lodash'), validator = require('validator'), + assert = require('assert'), Promise = require('bluebird'), errors = require('../../errors'), config = require('../../config'), readThemes = require('../../utils/read-themes'), i18n = require('../../i18n'), + toString = require('lodash.tostring'), validateSchema, validateSettings, @@ -14,8 +16,20 @@ var schema = require('../schema').tables, availableThemes; +function assertString(input) { + assert(typeof input === 'string', 'Validator js validates strings only'); +} + +// extends has been removed in validator >= 5.0.0, need to monkey-patch it back in +validator.extend = function (name, fn) { + validator[name] = function () { + var args = Array.prototype.slice.call(arguments); + assertString(args[0]); + return fn.apply(validator, args); + }; +}; + // Provide a few custom validators -// validator.extend('empty', function empty(str) { return _.isEmpty(str); }); @@ -39,22 +53,32 @@ validateSchema = function validateSchema(tableName, model) { validationErrors = []; _.each(columns, function each(columnKey) { - var message = ''; + var message = '', + strVal = toString(model[columnKey]); // check nullable if (model.hasOwnProperty(columnKey) && schema[tableName][columnKey].hasOwnProperty('nullable') && schema[tableName][columnKey].nullable !== true) { - if (validator.isNull(model[columnKey]) || validator.empty(model[columnKey])) { + if (validator.empty(strVal)) { message = i18n.t('notices.data.validation.index.valueCannotBeBlank', {tableName: tableName, columnKey: columnKey}); validationErrors.push(new errors.ValidationError(message, tableName + '.' + columnKey)); } } + // validate boolean columns + if (model.hasOwnProperty(columnKey) && schema[tableName][columnKey].hasOwnProperty('type') + && schema[tableName][columnKey].type === 'bool') { + if (!(validator.isBoolean(strVal) || validator.empty(strVal))) { + message = i18n.t('notices.data.validation.index.valueMustBeBoolean', {tableName: tableName, columnKey: columnKey}); + validationErrors.push(new errors.ValidationError(message, tableName + '.' + columnKey)); + } + } + // TODO: check if mandatory values should be enforced if (model[columnKey] !== null && model[columnKey] !== undefined) { // check length if (schema[tableName][columnKey].hasOwnProperty('maxlength')) { - if (!validator.isLength(model[columnKey], 0, schema[tableName][columnKey].maxlength)) { + if (!validator.isLength(strVal, 0, schema[tableName][columnKey].maxlength)) { message = i18n.t('notices.data.validation.index.valueExceedsMaxLength', {tableName: tableName, columnKey: columnKey, maxlength: schema[tableName][columnKey].maxlength}); validationErrors.push(new errors.ValidationError(message, tableName + '.' + columnKey)); @@ -63,12 +87,12 @@ validateSchema = function validateSchema(tableName, model) { // check validations objects if (schema[tableName][columnKey].hasOwnProperty('validations')) { - validationErrors = validationErrors.concat(validate(model[columnKey], columnKey, schema[tableName][columnKey].validations)); + validationErrors = validationErrors.concat(validate(strVal, columnKey, schema[tableName][columnKey].validations)); } // check type if (schema[tableName][columnKey].hasOwnProperty('type')) { - if (schema[tableName][columnKey].type === 'integer' && !validator.isInt(model[columnKey])) { + if (schema[tableName][columnKey].type === 'integer' && !validator.isInt(strVal)) { message = i18n.t('notices.data.validation.index.valueIsNotInteger', {tableName: tableName, columnKey: columnKey}); validationErrors.push(new errors.ValidationError(message, tableName + '.' + columnKey)); } @@ -142,6 +166,7 @@ validateActiveTheme = function validateActiveTheme(themeName) { // available validators: https://github.com/chriso/validator.js#validators validate = function validate(value, key, validations) { var validationErrors = []; + value = toString(value); _.each(validations, function each(validationOptions, validationName) { var goodResult = true; diff --git a/core/server/models/post.js b/core/server/models/post.js index 02309f09bd..fea1f2ade6 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -12,6 +12,7 @@ var _ = require('lodash'), config = require('../config'), baseUtils = require('./base/utils'), i18n = require('../i18n'), + toString = require('lodash.tostring'), Post, Posts; @@ -178,11 +179,11 @@ Post = ghostBookshelf.Model.extend({ ghostBookshelf.Model.prototype.saving.call(this, model, attr, options); - this.set('html', converter.makeHtml(this.get('markdown'))); + this.set('html', converter.makeHtml(toString(this.get('markdown')))); // disabling sanitization until we can implement a better version title = this.get('title') || i18n.t('errors.models.post.untitled'); - this.set('title', title.trim()); + this.set('title', toString(title).trim()); // ### Business logic for published_at and published_by // If the current status is 'published' and published_at is not set, set it to now diff --git a/core/server/models/user.js b/core/server/models/user.js index fb1a03e268..e1e6fb20ce 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -10,6 +10,7 @@ var _ = require('lodash'), validation = require('../data/validation'), events = require('../events'), i18n = require('../i18n'), + toString = require('lodash.tostring'), bcryptGenSalt = Promise.promisify(bcrypt.genSalt), bcryptHash = Promise.promisify(bcrypt.hash), @@ -361,6 +362,8 @@ User = ghostBookshelf.Model.extend({ userData = this.filterData(data), roles; + userData.password = toString(userData.password); + options = this.filterOptions(options, 'add'); options.withRelated = _.union(options.withRelated, options.include); diff --git a/core/server/translations/en.json b/core/server/translations/en.json index 767bd408e6..6ed44d16f8 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -532,6 +532,7 @@ "validation": { "index": { "valueCannotBeBlank": "Value in [{tableName}.{columnKey}] cannot be blank.", + "valueMustBeBoolean": "Value in [settings.key] must be one of true, false, 0 or 1.", "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.", diff --git a/core/test/integration/model/model_posts_spec.js b/core/test/integration/model/model_posts_spec.js index d59daeb3eb..e5fb06053f 100644 --- a/core/test/integration/model/model_posts_spec.js +++ b/core/test/integration/model/model_posts_spec.js @@ -383,6 +383,36 @@ describe('Post Model', function () { }).catch(done); }); + it('can change title to number', function (done) { + var postId = 1; + + PostModel.findOne({id: postId}).then(function (results) { + should.exist(results); + var post = results.toJSON(); + post.title.should.not.equal('123'); + return PostModel.edit({title: 123}, _.extend({}, context, {id: postId})); + }).then(function (edited) { + should.exist(edited); + edited.attributes.title.should.equal('123'); + done(); + }).catch(done); + }); + + it('can change markdown to number', function (done) { + var postId = 1; + + PostModel.findOne({id: postId}).then(function (results) { + should.exist(results); + var post = results.toJSON(); + post.title.should.not.equal('123'); + return PostModel.edit({markdown: 123}, _.extend({}, context, {id: postId})); + }).then(function (edited) { + should.exist(edited); + edited.attributes.markdown.should.equal('123'); + done(); + }).catch(done); + }); + it('can publish draft post', function (done) { var postId = 4; @@ -819,6 +849,28 @@ describe('Post Model', function () { }).catch(done); }); + it('can add, with title being a number', function (done) { + var newPost = testUtils.DataGenerator.forModel.posts[2]; + + newPost.title = 123; + + PostModel.add(newPost, context).then(function (createdPost) { + should.exist(createdPost); + done(); + }).catch(done); + }); + + it('can add, with markdown being a number', function (done) { + var newPost = testUtils.DataGenerator.forModel.posts[2]; + + newPost.markdown = 123; + + PostModel.add(newPost, context).then(function (createdPost) { + should.exist(createdPost); + done(); + }).catch(done); + }); + it('can add, with previous published_at date', function (done) { var previousPublishedAtDate = new Date(2013, 8, 21, 12); diff --git a/core/test/integration/model/model_users_spec.js b/core/test/integration/model/model_users_spec.js index ba60860db0..abce219449 100644 --- a/core/test/integration/model/model_users_spec.js +++ b/core/test/integration/model/model_users_spec.js @@ -128,6 +128,20 @@ describe('User Model', function run() { }).catch(done); }); + it('can set password of only numbers', function () { + var userData = testUtils.DataGenerator.forModel.users[0]; + + // avoid side-effects! + userData = _.cloneDeep(userData); + userData.password = 12345678; + + // mocha supports promises + return UserModel.add(userData, context).then(function (createdUser) { + should.exist(createdUser); + // cannot validate password + }); + }); + it('can find by email and is case insensitive', function (done) { var userData = testUtils.DataGenerator.forModel.users[2], email = testUtils.DataGenerator.forModel.users[2].email; diff --git a/package.json b/package.json index 6e15ca88bf..908bf34ec6 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "jsonpath": "0.2.2", "knex": "0.10.0", "lodash": "3.10.1", + "lodash.tostring": "4.1.2", "moment": "2.11.2", "morgan": "1.6.1", "multer": "1.1.0", @@ -64,7 +65,7 @@ "showdown-ghost": "0.3.6", "sqlite3": "3.1.1", "unidecode": "0.1.8", - "validator": "4.5.0", + "validator": "5.1.0", "xml": "1.0.1" }, "optionalDependencies": {