From 371f246c419942033b6f41a66f2b7d1fc3eda2e8 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Mon, 8 Jun 2020 18:08:46 +0200
Subject: [PATCH] Improved custom slug sluggification, allowing valid URL
 characters

---
 composer.json                                 | 11 +++++---
 .../src/Util/CocurSymfonySluggerBridge.php    | 26 +++++++++++++++++++
 .../Validation/ShortUrlMetaInputFilter.php    |  7 ++++-
 module/Core/test/Model/ShortUrlMetaTest.php   | 20 +++++++++++---
 4 files changed, 55 insertions(+), 9 deletions(-)
 create mode 100644 module/Core/src/Util/CocurSymfonySluggerBridge.php

diff --git a/composer.json b/composer.json
index 5d531a37..7ba81442 100644
--- a/composer.json
+++ b/composer.json
@@ -17,6 +17,7 @@
         "ext-pdo": "*",
         "akrabat/ip-address-middleware": "^1.0",
         "cakephp/chronos": "^1.2",
+        "cocur/slugify": "^4.0",
         "doctrine/cache": "^1.9",
         "doctrine/dbal": "^2.10",
         "doctrine/migrations": "^2.2",
@@ -53,11 +54,13 @@
         "shlinkio/shlink-event-dispatcher": "^1.4",
         "shlinkio/shlink-installer": "^5.0.0",
         "shlinkio/shlink-ip-geolocation": "^1.4",
-        "symfony/console": "^5.0",
-        "symfony/filesystem": "^5.0",
-        "symfony/lock": "^5.0",
+        "symfony/console": "^5.1",
+        "symfony/filesystem": "^5.1",
+        "symfony/lock": "^5.1",
         "symfony/mercure": "^0.3.0",
-        "symfony/process": "^5.0"
+        "symfony/process": "~5.0.9",
+        "symfony/string": "^5.1",
+        "symfony/translation-contracts": "^2.1"
     },
     "require-dev": {
         "devster/ubench": "^2.0",
diff --git a/module/Core/src/Util/CocurSymfonySluggerBridge.php b/module/Core/src/Util/CocurSymfonySluggerBridge.php
new file mode 100644
index 00000000..9415e47c
--- /dev/null
+++ b/module/Core/src/Util/CocurSymfonySluggerBridge.php
@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Util;
+
+use Cocur\Slugify\SlugifyInterface;
+use Symfony\Component\String\AbstractUnicodeString;
+use Symfony\Component\String\Slugger\SluggerInterface;
+
+use function Symfony\Component\String\s;
+
+class CocurSymfonySluggerBridge implements SluggerInterface
+{
+    private SlugifyInterface $slugger;
+
+    public function __construct(SlugifyInterface $slugger)
+    {
+        $this->slugger = $slugger;
+    }
+
+    public function slug(string $string, string $separator = '-', ?string $locale = null): AbstractUnicodeString
+    {
+        return s($this->slugger->slugify($string, $separator));
+    }
+}
diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
index 8fde5e98..4503117c 100644
--- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php
+++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
@@ -4,11 +4,13 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\Validation;
 
+use Cocur\Slugify\Slugify;
 use DateTime;
 use Laminas\InputFilter\Input;
 use Laminas\InputFilter\InputFilter;
 use Laminas\Validator;
 use Shlinkio\Shlink\Common\Validation;
+use Shlinkio\Shlink\Core\Util\CocurSymfonySluggerBridge;
 
 use const Shlinkio\Shlink\Core\MIN_SHORT_CODES_LENGTH;
 
@@ -46,7 +48,10 @@ class ShortUrlMetaInputFilter extends InputFilter
         // FIXME The only way to enforce the NotEmpty validator to be evaluated when the value is provided but it's
         //       empty, is by using the deprecated setContinueIfEmpty
         $customSlug = $this->createInput(self::CUSTOM_SLUG, false)->setContinueIfEmpty(true);
-        $customSlug->getFilterChain()->attach(new Validation\SluggerFilter());
+        $customSlug->getFilterChain()->attach(new Validation\SluggerFilter(new CocurSymfonySluggerBridge(new Slugify([
+            'regexp' => '/[^A-Za-z0-9._~]+/',
+            'lowercase' => false,
+        ]))));
         $customSlug->getValidatorChain()->attach(new Validator\NotEmpty([
             Validator\NotEmpty::STRING,
             Validator\NotEmpty::SPACE,
diff --git a/module/Core/test/Model/ShortUrlMetaTest.php b/module/Core/test/Model/ShortUrlMetaTest.php
index 7d0dd9b6..fe3d42fc 100644
--- a/module/Core/test/Model/ShortUrlMetaTest.php
+++ b/module/Core/test/Model/ShortUrlMetaTest.php
@@ -58,11 +58,14 @@ class ShortUrlMetaTest extends TestCase
         ]];
     }
 
-    /** @test */
-    public function properlyCreatedInstanceReturnsValues(): void
+    /**
+     * @test
+     * @dataProvider provideCustomSlugs
+     */
+    public function properlyCreatedInstanceReturnsValues(string $customSlug, string $expectedSlug): void
     {
         $meta = ShortUrlMeta::fromRawData(
-            ['validSince' => Chronos::parse('2015-01-01')->toAtomString(), 'customSlug' => 'foobar'],
+            ['validSince' => Chronos::parse('2015-01-01')->toAtomString(), 'customSlug' => $customSlug],
         );
 
         $this->assertTrue($meta->hasValidSince());
@@ -72,9 +75,18 @@ class ShortUrlMetaTest extends TestCase
         $this->assertNull($meta->getValidUntil());
 
         $this->assertTrue($meta->hasCustomSlug());
-        $this->assertEquals('foobar', $meta->getCustomSlug());
+        $this->assertEquals($expectedSlug, $meta->getCustomSlug());
 
         $this->assertFalse($meta->hasMaxVisits());
         $this->assertNull($meta->getMaxVisits());
     }
+
+    public function provideCustomSlugs(): iterable
+    {
+        yield ['foobar', 'foobar'];
+        yield ['foo bar', 'foo-bar'];
+        yield ['wp-admin.php', 'wp-admin.php'];
+        yield ['UPPER_lower', 'UPPER_lower'];
+        yield ['more~url_special.chars', 'more~url_special.chars'];
+    }
 }