Fixed multibyte string encoding

[edgardmessias]

Q	A
Documentation	no
Bugfix	yes
BC Break	no
New Feature	no
RFC	no
QA	no

Description

The problem occurs when the filename is multibyte string and this needs continuation.
str_split don't work with multibyte strings, like chineses characteres.

Related to glpi-project/glpi#8495

[snapshotpl]

@edgardmessias please provide covered test case

[glensc]

I think it's a lot better to use only mb_str_split and add https://github.com/symfony/polyfill-mbstring polyfill dependency.

maintainers need to say their word on that.

[glensc]

also, I think you must specify $encoding parameter to mb_str_split, as the default comes from php.ini, is ISO8859-1 in older versions, changed to UTF-8 in PHP 5.6 but yet still configurable in php.ini settings. a library should not rely on such global behavior.

how does it even work for you? you say your encoding is "Chinese" (which one of them? Big5? ISO 2022-JP?. ...?), but then write code that assumes utf-8?

in the bug report, you complain that the existing code doesn't respect multibyte, but then you add a function that relies on autodetect or unknown default?

[edgardmessias]

Looking here the dependencies, the true/punycode require symfony/polyfill-mbstring

[glensc]

great, then you only need to ensure the proper version is used:

composer require symfony/polyfill-mbstring:^1.12.0

• add new PHP 7.4 functions symfony/polyfill#181, v1.12.0 was the first tag to include that PR

[edgardmessias]

@glensc , I made a full rework

[glensc]

Add test where filename is not quoted. as I'm sure quotes are optional:

-filename=\"$multibyteFilename\""
+filename=$multibyteFilename

are using single quotes also allowed there?
need to consult RFC, so far this is just suspiction.

[edgardmessias]

are using single quotes also allowed there?
need to consult RFC, so far this is just suspiction.

I think this is a discussion for another topic.

I solved the problems mentioned above

[glensc]

Some other notes.

1. do not force push commits when the review is in progress. add git fixup commits instead and rebase later. it's difficult to review if every push you make is overwriting previous changes, difficult to see new changes. use Draft status to indicate PR is not ready for merge. I recommend https://github.com/keis/git-fixup and https://github.com/MitMaro/git-interactive-rebase-tool to help you in the process.
2. avoid adding references in commit messages, like Related to glpi-project/glpi#8495. keep them in PR body instead. this avoids creating spam and bogus references on the other side. each force push creates noise there and it has no real value: GLPI 9.5.3 mailcollector drops some attached documents depending on the file name glpi-project/glpi#8495 (reference)

[glensc]

I think this is a discussion for another topic.

I'm not certain of that, as your new code assumes an exact match of =" (line 148), so if it's not quoted, parsing will fail? I have not tested how code behaved previously, only looked at your diff and saw that you expect quote following immediately after equal sign. you should also test with optional spacing: = "...", =\t"..." (tab), so it looks fragile.

[edgardmessias]

@glensc First, thanks for the tips.

About the quote, I not modified the parser function (lines 43-95), but the encoder function (getFieldValue 108-173)

The problem with getFieldValue is when the filename is multibyte string and this needs continuation field.

[glensc]

@edgardmessias alright, I had a really quick look on the diff only, so didn't look it closely it's a generator, not parser logic.

[Ocramius]

Some byte-level changes happened here: should be investigated (these two lines, specifically)

[edgardmessias]

Yes, because I edited the function to generate exactly size of 78 characters

[cedric-anne]

I reviewed changes and tests cases and all seems ok form me.

Indeed, now filenames that are not using multibytes strings are now splitted on lines that are 78 chars length (instead of fickle value based on ceil(0.6 * $maxValueLength) operation), and multibyte UTF-8 filenames are correctly handled.

[glensc]

LGTM