[Spice-devel] [PATCH spice-gtk 1/5] Block sending clipboard data > max-clipboard

[Marc-André Lureau]

On Thu, Nov 7, 2013 at 11:47 PM, Uri Lublin <uril at redhat.com> wrote:
> On 11/06/2013 11:25 PM, Marc-André Lureau wrote:
>>
>> From: Marc-André Lureau <marcandre.lureau at redhat.com>
>>
>> Attempt to send very large clipboard data may easy cause OOM
>> abort, either in  gdk - some patch are proposed to improve the situation,
>> or in spice-gtk itself.
>>
>> Let's have a property that blocks unreasonably big clipboard data from
>> being processed (by default 100mb). Users willing to send larger data
>> can use the send basic drag-drop send file instead, or tweak the
>> property value.
>
>
> Hi Marc-Andre,
>
> Makes sense to block huge clipboards by default.
>
> See one comment below.
>
>> diff --git a/gtk/channel-main.c b/gtk/channel-main.c
>> index 5c33e67..dbcaff8 100644
>> --- a/gtk/channel-main.c
>> +++ b/gtk/channel-main.c
>> diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c
>> index 71ed300..eab7e2f 100644
>> --- a/gtk/spice-gtk-session.c
>> +++ b/gtk/spice-gtk-session.c
>> @@ -748,16 +748,19 @@ static void clipboard_received_cb(GtkClipboard
>> *clipboard,
>>       gchar* name;
>>       GdkAtom atom;
>>       int selection;
>> +    int max_clipboard;
>>         selection = get_selection_from_clipboard(s, clipboard);
>>       g_return_if_fail(selection != -1);
>>   +    g_object_get(s->main, "max-clipboard", &max_clipboard, NULL);
>>       len = gtk_selection_data_get_length(selection_data);
>> -    if (len == -1) {
>> +    if (len == 0 || (max_clipboard != -1 && len > max_clipboard)) {
>> +        g_warning("discarded clipboard of size %d (max: %d)", len,
>> max_clipboard);
>> +        return;
>> +    } else if (len == -1) {
>>           SPICE_DEBUG("empty clipboard");
>>           len = 0;
>> -    } else if (len == 0) {
>> -        SPICE_DEBUG("TODO: what should be done here?");
>>       } else {
>>           atom = gtk_selection_data_get_data_type(selection_data);
>>           name = gdk_atom_name(atom);
>
>
> Why did you switch the logic of len==0 and len==-1.
> I think we can drop the clipboard for both cases.
> Is it possible to get such values (0 and -1) for len ?

-1 is for empty clipboard, or fail to retrieve clipboard data (for
various reasons).

0 could eventually happen, but when? an app claims to have clipboard
data, but provides 0 data? I don't know, I thought a warning made
sense, instead of silently sending 0 data, but perhaps the current
behaviour is more correct. I can split that in a separate patch, or
drop that little change. And perhaps we should always send empty
clipboard msg on error to the guest actually. Afaik, there is no other
way to tell to the guest app to stop waiting for clipboard data. Any
idea?


-- 
Marc-André Lureau