diff --git a/packages/security/lib/string.js b/packages/security/lib/string.js index 3dc1865ea..ce4866409 100644 --- a/packages/security/lib/string.js +++ b/packages/security/lib/string.js @@ -1,9 +1,19 @@ const slugify = require('@tryghost/string').slugify; +// @TODO: the safe() function can possibly be removed from here if Ghost uses slugify() directly instead + /** - * @param {string} string - * @param {{importing?: boolean}} [options] + * @param {string} string - the string we want to slugify + * @param {object} [options] - filter options + * @param {bool} [options.importing] - don't perform optional cleanup, e.g. removing extra dashes + * @param {bool} [options.unicodeSlugs] - don't perform optional transliteration, e.g. keep smörgåsbord as it is instead of turning it into smorgasbord + * @param {string} [options.slugSeparator] - separator to be used for the slugs, can be ` `, `_` or `-`, defaults to `-` + * @returns {string} - a slugified string */ module.exports.safe = function safe(string, options = {}) { - return slugify(string, { requiredChangesOnly: options.importing === true }); + return slugify(string, { + requiredChangesOnly: options.importing === true, + unicodeSlugs: options.unicodeSlugs === true, + slugSeparator: options.slugSeparator + }); }; diff --git a/packages/security/test/string.test.js b/packages/security/test/string.test.js index 0f572be34..0b339af59 100644 --- a/packages/security/test/string.test.js +++ b/packages/security/test/string.test.js @@ -15,7 +15,7 @@ describe('Lib: Security - String', function () { assert.equal(result, ''); }); - it('should remove non ascii characters', function () { + it('should remove non-letter characters', function () { const result = security.string.safe('howtowin✓', options); assert.equal(result, 'howtowin'); }); @@ -37,9 +37,9 @@ describe('Lib: Security - String', function () { // note: This is missing the soft-hyphen char that isn't much-liked by linters/browsers/etc, // it passed the test before it was removed const result = security.string.safe( - '!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~¡¢£¤¥¦§¨©ª«¬®¯°±²³´µ¶·¸¹º»¼½¾¿', + '!"#$%&\'()*+,-./:;<=>?@[\\]^`{|}~¡¢£¤¥¦§¨©ª«¬®¯°±²_³´µ¶·¸¹º»¼½¾¿', ); - assert.equal(result, '_-c-y-ss-c-a-r-deg-23up-1o-1-41-23-4'); + assert.equal(result, 'a-2_3u-1o-1-41-23-4'); }); it('should replace all of the foreign chars in ascii', function () { @@ -48,7 +48,7 @@ describe('Lib: Security - String', function () { ); assert.equal( result, - 'aaaaaaaeceeeeiiiidnoooooxouuuuythssaaaaaaaeceeeeiiiidnooooo-ouuuuythy', + 'aaaaaaae-ceeeeiiiidnooooo-ouuuuythssaaaaaaaeceeeeiiiidnooooo-ouuuuythy', ); }); @@ -83,16 +83,33 @@ describe('Lib: Security - String', function () { }); it('should properly handle unicode punctuation conversion', function () { + // note: the previous unidecode transformation handled this differently than anyascii, so this is + // a compromise that's "good enough" and gives the most optimal results for most languages + // result using unidecode was: nijian-wei-iganaika-zai-du-que-ren-sitekudasai-zai-du-miip-misitekudasai const result = security.string.safe( 'に間違いがないか、再度確認してください。再読み込みしてください。', options, ); assert.equal( result, - 'nijian-wei-iganaika-zai-du-que-ren-sitekudasai-zai-du-miip-misitekudasai', + 'ni-jian-weiiganaika-zai-du-que-renshitekudasai-zai-dumi-yumishitekudasai', ); }); + it('should not transliterate the slugs if the unicodeSlugs flag is passed', function () { + let result = security.string.safe('Ett smörgåsbord från Sydkorea: 스뫼르고스보르드', { + unicodeSlugs: true, + }); + assert.equal(result, 'ett-smörgåsbord-från-sydkorea-스뫼르고스보르드'); + }); + + it('should not replace existing dashes and underscores when the slugSeparator is set to spaces', function () { + let result = security.string.safe('Herr./Klaus-Jürgen_44', { + slugSeparator: ' ', + }); + assert.equal(result, 'herr klaus-jurgen_44'); + }); + it('should not lose or convert dashes if options are passed with truthy importing flag', function () { let result = security.string.safe('-slug-with-starting-ending-and---multiple-dashes-', { importing: true, @@ -104,7 +121,7 @@ describe('Lib: Security - String', function () { let result = security.string.safe("-slug-&with-✓-invalid-characters-に'", { importing: true, }); - assert.equal(result, '-slug--with--invalid-characters-ni'); + assert.equal(result, '-slug--with---invalid-characters-ni'); }); }); }); diff --git a/packages/validator/lib/validator.js b/packages/validator/lib/validator.js index c6650c185..99431978c 100644 --- a/packages/validator/lib/validator.js +++ b/packages/validator/lib/validator.js @@ -47,7 +47,11 @@ validators.isEmptyOrURL = function isEmptyOrURL(str) { validators.isSlug = function isSlug(str) { assertString(str); - return validators.matches(str, /^[a-z0-9\-_]+$/); + // The slugs should always be normalized with NFC before being used, but some languages rely on + // combining marks to create letters. To avoid misuse, the slugify() function only generates slugs + // with a natural number of combining marks. Marks in the beginning of a word means they're invalid, + // and in the rest of the slug a maximum of three combining marks is permitted to each letter. + return validators.matches(str, /^(?:[\p{L}\p{N}][\p{Mn}\p{Mc}]{0,3}|[ _-])+$/u); }; validators.isEmail = function isEmail(str, options = { legacy: true }) { diff --git a/packages/validator/test/internals.test.js b/packages/validator/test/internals.test.js index 2dc13b2a6..41c9f98cf 100644 --- a/packages/validator/test/internals.test.js +++ b/packages/validator/test/internals.test.js @@ -46,7 +46,35 @@ describe('Validator internals', function () { it('isSlug validates slug format', function () { assert.equal(validator.isSlug('a-valid_slug-1'), true); - assert.equal(validator.isSlug('not valid slug'), false); + assert.equal(validator.isSlug('not? valid slug'), false); + }); + + it('isSlug accepts slugs generated without optional changes', function () { + assert.equal(validator.isSlug('-separator-in-beginning'), true); + assert.equal(validator.isSlug('-separator-in-beginning-and-end-'), true); + assert.equal(validator.isSlug('multiple---separators-in--use'), true); + assert.equal(validator.isSlug('different__kinds-of separator___in-the-same-text'), true); + }); + + it('isSlug rejects slugs with too many combining marks, but accept some of them', function () { + // note that this might break some text editors, so it might need to be removed + assert.equal(validator.isSlug('ghost-blo̵̯͝ǵ'), true); + assert.equal(validator.isSlug('G̸̛̦̼̜̱̹̦̲̩̰̀̓̆̇̔̎̒̎h̸͕̹̤̿͌́͊͋̈̂͗̕o̶̠͑̍s̷̝̭̰̳̖̣͉̈́̌̐́̈́̒͂̚t̴̩̦̫̟̲̘̆̔̑̅͘̕͠͝͠ ̶̜̺͚̆̈ͅb̸̰͕͔͈̤̾̉͒̂̎ͅl̵̳͚̘̯̀̎o̵̯͝ǵ̴̨̛͍̞͙̲̦̗̖͍̂̈́͆͝'), false); + }); + + it('isSlug rejects slugs with combining marks in the beginning of a text or a word', function () { + // note that this might break some text editors, so it might need to be removed + assert.equal(validator.isSlug('ผีในวัฒนธรรมไทย'), true); + assert.equal(validator.isSlug('\u0303\u0301\u0302ผีในวัฒนธรรมไทย'), false); + assert.equal(validator.isSlug('ghost-\u0301\u0302blog'), false); + }); + + it('isSlug accepts foreign characters, in this case Japanese', function () { + assert.equal(validator.isSlug('に間違いがないか_再度確認してください_再読み込みしてください'), true); + }); + + it('isSlug rejects non-letter characters, like emojis', function () { + assert.equal(validator.isSlug('im-so-happy-today-🙃'), false); }); it('custom validators enforce string input', function () {