Trashbin truncate filename unicode - #62698
Conversation
|
/backport to stable34 |
|
/backport to stable33 |
|
/backport to stable32 |
ShGKme
left a comment
There was a problem hiding this comment.
To remove the middle instead of "after the middle"
|
|
||
| $charsToRemove = $length - $maxLength + 1; | ||
| $charLength = mb_strlen($trashFilename); | ||
| $start = mb_substr($trashFilename, 0, floor(($charLength / 2) - $charsToRemove)); | ||
| $end = mb_substr($trashFilename, floor($charLength / 2)); | ||
| return $start . '_' . $end; |
There was a problem hiding this comment.
| $charsToRemove = $length - $maxLength + 1; | |
| $charLength = mb_strlen($trashFilename); | |
| $start = mb_substr($trashFilename, 0, floor(($charLength / 2) - $charsToRemove)); | |
| $end = mb_substr($trashFilename, floor($charLength / 2)); | |
| return $start . '_' . $end; | |
| $half = intdiv($maxLength - 1, 2); // One char for _ | |
| return mb_substr($trashFilename, 0, $half) | |
| . '_' | |
| . mb_substr($trashFilename, -$half); |
There was a problem hiding this comment.
That doesn't work with multibyte codepoints, mb_substr($foo, 0, $length) can have a byte length > $length.
By using mb_strlen as the starting point for determining the mb_substr codepoint length, we take that into account.
There was a problem hiding this comment.
True, but we can still remove the middle instead of from/to the middle.
6f8fdab to
3f4310f
Compare
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
3f4310f to
b9d7662
Compare
| $charsToRemove = $length - $maxLength + 1; | ||
| $charLength = mb_strlen($trashFilename); | ||
| $start = mb_substr($trashFilename, 0, intdiv($charLength, 2) - $charsToRemove); |
There was a problem hiding this comment.
The original filename is bound to the same 500 byte limit, so we can't get into that situation.
$length = 500 (can even be longer due to '.d' . $timestamp)
$maxLength = 250
$charsToRemove = 500 - 250 + 1 = 251
intdiv($charLength, 2) - $charsToRemove = 500 / 2 - 251 = -1
Start is a suffix instead of a prefix?
There was a problem hiding this comment.
It seems very indirect to rely on the 500 limit here - this is not visible from the code.
There was a problem hiding this comment.
Why not remove in the middle instead of using the middle as the boundary?
Then we don't need to worry about a half being too big.
There was a problem hiding this comment.
I made a mistake in an earlier comment, the $filename has a max-length of 250
Summary
The old logic can cut right in the middle of multi-byte unicode codepoints.
TODO
Checklist
3. to review, feature component)stable32)AI (if applicable)