🎨 Changed and improved the slug generation and validation

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
+    });
 };

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');
         });
     });
 });

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 }) {

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 () {