DirectoryTree / LdapRecord

A fully-featured LDAP framework.
https://ldaprecord.com
MIT License
500 stars 44 forks source link

[Bug] Decoding problem of UTF-8 strings (used e.g. for serialization in session) #698

Closed lhaemmerle closed 5 months ago

lhaemmerle commented 5 months ago

Environment:

Describe the bug: We want to store LDAP user objects (extends LdapRecord\Models\Model) in the Laravel session (serialization). However, this causes the problem that LDAP users with names containing Umlaut characters (e.g. "René") are wrongly decoded when the session is loaded (=LDAP user unserialized) again. The UTF-8 string "René" is considered an 'ISO-8859-1' encoded string and converted to UTF-8, which then becomes "René".

It seems that the cause for this a bug in the function decodeValue(string $value) in HasAttributes.php: https://github.com/DirectoryTree/LdapRecord/blob/master/src/Models/Concerns/HasAttributes.php#L223

I suspect the condition in

if (MbString::isLoaded() && MbString::isUtf8($value)) {
      return mb_convert_encoding($value, 'UTF-8', 'ISO-8859-1');
}

Should rather be (notice the !):

if (MbString::isLoaded() && !MbString::isUtf8($value)) {
      return mb_convert_encoding($value, 'UTF-8', 'ISO-8859-1');
}

because it makes no sense to convert a string that is already utf-8.

stevebauman commented 5 months ago

Ah thanks @lhaemmerle! I'll patch this now and add a test to ensure this works properly.

stevebauman commented 5 months ago

Ok I've just released v3.5.1 with a patch for this issue. I've added test assertions to replicate this bug and ensure it's working properly.

Please run composer update and you're all set! 👍

Appreciate the detailed report 🙏

lhaemmerle commented 5 months ago

Great, thank your for the quick fix!