[Spice-devel] [spice-gtk v1] file-xfer: Fix bad filename encoding

[Victor Toso]

Hi,

On Wed, Apr 12, 2017 at 07:52:29AM -0400, Marc-André Lureau wrote:
> > 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!)

Ah, good!

> >
> > > 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?

Not sure. I don't see how as this we are dragging and drop it.. But it
isn't a problem to use g_file_get_basename() again, i tested and it
works. It would make sense to simple change the
g_file_info_get_attribute_as_string() call to
spice_file_transfer_task_get_filename().

Do you think it would be better?

>
> >
> > 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 at redhat.com>
> > > > ---
> > > >  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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170412/090a00de/attachment.sig>