Hi, On Wed, Apr 12, 2017 at 07:19:54AM -0400, Frediano Ziglio wrote: > > > > > From: Victor Toso <me@xxxxxxxxxxxxxx> > > > > Manual for G_FILE_ATTRIBUTE_STANDARD_NAME states: > > > The name is the on-disk filename which may not be in any known > > > encoding, and can thus not be generally displayed as is. > > > > Considering a file named "ěščřžýáíé", if we use > > G_FILE_ATTRIBUTE_STANDARD_NAME get the file name, we will have the > > following 72 char long string: > > "\xc4\x9b\xc5\xa1\xc4\x8d\xc5\x99\xc5\xbe\xc3\xbd\xc3\xa1\xc3\xad\xc3\xa9" > > > > this string is only 18 characters long, why 72 ? strlen(basename) == 72, there are 72 chars in above string. > > We should be use G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME instead which > > will give us the correct 18 long utf-8 string: "ěščřžýáíé" > > I think this solves the encoding as we'll transmit with a given encoding > (utf8). Yes > If the source filename is not correctly encoded this will give a > destination filename different from the source. That was the bug > As the protocol does not include an encoding utf-8 is a good choice > as ASCII compatible and in theory should be able to encode every > possible name set (beside invalid one of course). Yeah, In the commit [0] and related messages I did not find a specific encoding that should be used, maybe it is stated somewhere else... I agree that utf8 is a good choice. [0] https://cgit.freedesktop.org/spice/spice-protocol/commit/?id=a484ca8095556f7f75979f14 > As we deal with different OSes and encodings (ie utf8 in Linux and > utf16 in Windows) having a super set is probably the best solution. > A question could be: Is is better > basename = g_file_info_get_attribute_as_string(info, G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME); > or > basename = g_file_info_get_attribute_as_byte_string(info, G_FILE_ATTRIBUTE_STANDARD_NAME); > (as my previous consideration I think the first). Agreed > Maybe the name G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME is > confusing here as we just want the standard utf-8 encoded > name, not something just to display to the user. > A comment could help. I too thought it was confusing hence the bug :) I should have payed more attention to the documentation at the time I refactored this bits. [ before we were using g_file_get_basename() but I changed to g_file_info_get_attribute_as_string() ]. Besides the quote from the manual and the explanation of the issue, not sure what comment should I provide or what is lacking in explanation. toso > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1440206 > > > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx> > > --- > > src/channel-main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/channel-main.c b/src/channel-main.c > > index be7c852..c8de5f3 100644 > > --- a/src/channel-main.c > > +++ b/src/channel-main.c > > @@ -2846,7 +2846,7 @@ static void file_xfer_init_task_async_cb(GObject *obj, > > GAsyncResult *res, gpoint > > goto failed; > > > > channel = spice_file_transfer_task_get_channel(xfer_task); > > - basename = g_file_info_get_attribute_as_string(info, > > G_FILE_ATTRIBUTE_STANDARD_NAME); > > + basename = g_file_info_get_attribute_as_string(info, > > G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME); > > file_size = g_file_info_get_attribute_uint64(info, > > G_FILE_ATTRIBUTE_STANDARD_SIZE); > > > > xfer_op = data; > > I would be curious to see different OSes with even extra plane 0 > characters. > > Frediano
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel