Hi ----- Original Message ----- > 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 I am pretty sure we agreed on utf8, but I can't find a reference either (it's already quite old!) > > > 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() ]. interesting! I suspect it worked better before the change (in local encoding), but was still buggy in various cases ;). My concern with DISPLAY_NAME is that perhaps some files don't have a displayable name? > > 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 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel