Support non-utf8 file names

[gulrotkake]

If given a non-utf8 filename, such as

$ touch `echo -e "notutf8\xbf.dat"`

SftpSession::read_dir(path_to_directory) will fail to list the directory, as filename in DirEntry is of type string and try_get_string throws an error when encountering a non-utf8 string.

    fn try_get_string(&mut self) -> Result<String, Error> {
        let bytes = self.try_get_bytes()?;
        String::from_utf8(bytes).map_err(|_| Error::BadMessage("unable to parse str".to_owned()))
    }

A solution could be to use OsString or PathBuf, I'll try to write a PR, but I've barely started climbing the Rust learning curve :)

[TheBlindM]

I encountered the same problem

[AspectUnk]

Here it is described why we cannot use PathBuf. I can't say how appropriate it would be to use OsString. As a temporary solution we can use #39 because it allows to use .as_bytes() and leave further work to the end user but still this is not a panacea.

[gulrotkake]

Would just using a Vec<u8> instead of String be a way to go? I suppose it's not too different from #39, if as_bytes() returns the original array, except it would make it very explicit that filenames are just a vector of bytes, but with the trade-off that the API becomes less ergonomic 🤷 .

[AspectUnk]

Yes

Would just using a Vec<u8> instead of String be a way to go?

but

API becomes less ergonomic

So I would like to find a more universal solution because I don't even like #39 that much

[kanpov]

Yes

Would just using a Vec<u8> instead of String be a way to go?

but

API becomes less ergonomic

So I would like to find a more universal solution because I don't even like #39 that much

OsString would be an alternative then?

[kanpov]

I could make a PR replacing with OsString @AspectUnk, don't see the disadvantage

[kanpov]

@AspectUnk Made a most definitely working implementation with OsString in #44

[AspectUnk]

After conducting research, I can say that UTF-16 support is not provided for in the specification because there is inconsistency regarding the size of a string character.

OsString like PathBuf is somewhat system-dependent, because its serialization will produce different results on different systems. The inconsistency in the size of the string character leads to packet integrity and byte order violations.

Linux:

Windows:

@gulrotkake Replacing String with Vec<u8> also won't yield results because the packet defines the number of characters, not bytes.
@kanpov The reason why your tests passed is because of the absence of tests on different systems.

Some leave the choice of encoding strings in UTF-16 to the end user, but warn that the remote server must also support it. It is possible to add this as a feature, but it is an extremely specific solution, so leaving #39 would be more appropriate. I may take care of it, but not very soon.

If I'm wrong about anything let me know.

[kanpov]

So it's pretty much a choice between not having utf-8 or not supporting windows @AspectUnk ? I'd rather have the latter, but maybe there's another way

[AspectUnk]

So it's pretty much a choice between not having utf-8 or not supporting windows @AspectUnk ? I'd rather have the latter, but maybe there's another way

Perhaps I misunderstood you, but it already supports utf-8 by default and at the same time windows, and #39 solves all problems with characters unsuitable for parsing.

[AspectUnk]

Some leave the choice of encoding strings in UTF-16 to the end user, but warn that the remote server must also support it.

Btw, along with this we can add support for the unfinished version of specification v6 (page 16) since it has a solution for sending charset.

[TheBlindM]

When will the version be released?😂