Avoid use of iconv to get rid of unicode

[come-nc]

Using iconv for translit depends upon server configuration, locale, and PHP version.
Using htmlentities instead to have a consistent behavior independent of configuration.

In theory using iconv for translit is better because it can do smart things like turing ä to ae or ß to ss, but in practice that only works if you know which locale to set before calling iconv, which we cannot know for usernames.

I did not remove the iconv dependency check as I am not sure if this is our only use of iconv?

Also, I found this in Symfony, which could be used instead of having our own code for this: https://symfony.com/doc/current/components/string.html#slugger

[come-nc]

Also, I found this in Symfony, which could be used instead of having our own code for this: https://symfony.com/doc/current/components/string.html#slugger

This appears to be Symfony >= 5 only and we use 4 for now.

[MichaIng]

Generally I like getting rid of dependencies when they are used in a single case only, which is not even used on every Nextcloud instance (LDAP) and where an own solution doesn't involve tons of code.

Checking the server repository code, it seems to be indeed the only case: https://github.com/nextcloud/server/search?q=iconv
Not sure whether apps may depend on it, but not the core apps at least, and for external apps it is pretty common anyway that additional software needs to be installed on the server, so not a blocker IMHO.

I ran a grep -r iconv . as well and there is much more code around checking for the module, printing its version, installing it for tests etc than the few lines you added to remove unsupported characters:

# grep -r iconv .
./build/package-lock.json: "iconv-lite": "0.4.24",
./build/package-lock.json: "iconv-lite": {
./build/package-lock.json: "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.4.24.tgz",
./build/package-lock.json: "iconv-lite": "0.4.24",
./.github/workflows/update-psalm-baseline.yml: extensions: ctype,curl,dom,fileinfo,gd,iconv,intl,json,mbstring,openssl,pdo_sqlite,posix,sqlite,xml,zip
./.github/workflows/s3-external.yml: extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, zip, gd
./.github/workflows/s3-external.yml: extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, zip, gd
./.github/workflows/oci.yml: extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, oci8
./apps/user_ldap/lib/Access.php: $transliterated = @iconv('UTF-8', 'ASCII//TRANSLIT', $name);
./apps/user_ldap/lib/Access.php: // depending on system config iconv can work or not
./apps/user_ldap/tests/AccessTest.php: $translitExpected = @iconv('UTF-8', 'ASCII//TRANSLIT', 'fränk') ? 'frank' : 'frnk';
./lib/private/App/PlatformRepository.php: case 'iconv':
./lib/private/legacy/OC_Util.php: 'iconv' => 'iconv',
./package-lock.json: "iconv-lite": "^0.4.24",
./package-lock.json: "iconv-lite": {
./package-lock.json: "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.4.24.tgz",
./package-lock.json: "iconv-lite": "0.4.24"

Furthermore, iconv brings own issues, aside of this particular one that it may or may not work based on systems locale:

• 47a10bd
• eedbebd
• The disabled test for fränk.

Also I don't think that there will be any new kinds of special characters included regularly so that this code should not require constant maintenance, and with preg_replace('#&[^;]+;#', '', $name); we have a failsafe removal, in case.

So basically it looks like removing iconv will reduce maintenance effort and not increase it + the solution here allows more consistency across Nextcloud instances and CI tests and is more failsafe as well.

Leaving this as opinion, but I won't approve as I think two guys with more insights should evaluate implications I might overlook.

[MichaIng]

/backport to stable23

[MichaIng]

/backport to stable22

[MichaIng]

/backport to stable21

[backportbot-nextcloud]

The backport to stable22 failed. Please do this backport manually.

[backportbot-nextcloud]

The backport to stable23 failed. Please do this backport manually.

[backportbot-nextcloud]

The backport to stable21 failed. Please do this backport manually.

[MichaIng]

PR up to consequently remove the module from dependencies and tests: #29958