Handle unicode filenames in FTP - #3064
Open
julianoes wants to merge 2 commits into
Open
Conversation
Emojis, Japanese, Cyrillic and combining accents in filenames are all just UTF-8 bytes to the FTP protocol, but nothing checked that they survive a round trip. These tests upload, download, list, rename and remove files and directories with such names, and check that a name which doesn't fit into the 239 byte payload is rejected instead of being cut in the middle of a code point.
A directory entry which doesn't fit into an empty payload, e.g. a file with
a 240 byte name, made the server respond with EOF at that offset. The client
took that as the end of the listing, so the long entry and everything after
it stayed invisible.
Send a skip entry ('S') for such an entry instead so the offset advances and
the rest of the directory is listed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Filenames with emojis and other non-ASCII characters were never exercised over MAVLink FTP, so this tries them out in
system_testsand fixes the one thing that turned out to be broken.Tests (
src/system_tests/ftp_unicode.cpp)🚁 flight-log 🛰️.bininside a directory calledflights 🛫InvalidParameterrather than being cut in the middle of a code point, while a name that just fits still worksEverything above already worked, since to the protocol these are just UTF-8 bytes and the client validates lengths before it copies.
Fix
The last test found a real bug, which is not unicode specific but much easier to hit with it (each emoji is 4 bytes): a directory entry that doesn't fit into an empty payload — e.g. a 240 byte filename, which is perfectly legal on ext4 — made the server break out of the loop with nothing written, so it responded with EOF. The client took that as the end of the listing, and the long entry and every entry after it stayed invisible.
The server now emits a skip entry (
'S') for such an entry, which the client already counts towards the offset, so the listing moves past it and the rest of the directory shows up.Not covered
On Windows,
std::filesystem::path's narrow conversions use the ANSI code page, so UTF-8 names would likely mangle there. The system tests only run on Linux and macOS (and hardcode/tmp), so this is untested and unchanged here.