After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 703002 - screen sharing password should be limited to 8 chars
screen sharing password should be limited to 8 chars
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Sharing
git master
Other All
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-06-24 18:32 UTC by Jonh Wendell
Modified: 2013-06-26 14:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (1.10 KB, patch)
2013-06-24 18:33 UTC, Jonh Wendell
needs-work Details | Review
proposed patch, v2 (2.61 KB, patch)
2013-06-26 12:10 UTC, Jonh Wendell
needs-work Details | Review
proposed patch, v3 (2.63 KB, patch)
2013-06-26 13:41 UTC, Jonh Wendell
none Details | Review

Description Jonh Wendell 2013-06-24 18:32:08 UTC
vino (and vnc protocol) only recognizes the 8 first chars. also,
vinagre, gnome vnc client limits its entries to 8 chars as well.
Comment 1 Jonh Wendell 2013-06-24 18:33:18 UTC
Created attachment 247667 [details] [review]
proposed patch
Comment 2 Thomas Wood 2013-06-25 09:45:35 UTC
Review of attachment 247667 [details] [review]:

Patch looks fine to me, please push to master and gnome-3-8 branches.
Comment 3 Bastien Nocera 2013-06-25 09:56:24 UTC
(In reply to comment #0)
> vino (and vnc protocol) only recognizes the 8 first chars. also,
> vinagre, gnome vnc client limits its entries to 8 chars as well.

The first 8 bytes or the first 8 utf-8 characters?
Comment 4 Jonh Wendell 2013-06-25 12:15:55 UTC
8 bytes
Comment 5 Bastien Nocera 2013-06-25 12:41:03 UTC
Review of attachment 247667 [details] [review]:

::: panels/sharing/sharing.ui
@@ +1200,3 @@
                             <property name="invisible_char_set">True</property>
                             <property name="input_purpose">password</property>
+                            <property name="max_length">8</property>

This is the number of characters, not the number of bytes needed to fit those characters.
If non-ASCII characters are allowed as a password, then you'll need to restrict the maximum number of characters by checking the byte size.
Comment 6 Jonh Wendell 2013-06-26 12:10:22 UTC
Created attachment 247813 [details] [review]
proposed patch, v2

2nd try, this time blocks the input at 8 bytes, not 8 chars.

the plan is to apply this into 3.8 branch as well, so, no new strings.
Comment 7 Bastien Nocera 2013-06-26 12:26:39 UTC
Review of attachment 247813 [details] [review]:

::: panels/sharing/cc-sharing-panel.c
@@ +762,3 @@
+
+  available_size = MAX_PASSWORD_SIZE - l;
+  if (available_size == 0)

Looks like it doesn't handle me typing 7 bytes then trying to insert a 2 byte character (available_size would be negative).
Comment 8 Jonh Wendell 2013-06-26 12:35:54 UTC
"l" is the current size (in bytes) of the entry. in your example, 7.
available_size would be equal to 1

it would never be less than zero, because the size would never be great than MAX_PASSWORD_SIZE.

i've tried here various combinations and it worked fine.
Comment 9 Bastien Nocera 2013-06-26 12:52:39 UTC
Review of attachment 247813 [details] [review]:

::: panels/sharing/cc-sharing-panel.c
@@ +747,3 @@
+screen_sharing_password_insert_text_cb (GtkEditable *editable,
+                                        gchar       *new_text,
+                                        gint         new_text_length,

Say we have a 2-byte unicode character.

@@ +753,3 @@
+  int l, available_size;
+
+  l = gtk_entry_buffer_get_bytes (gtk_entry_get_buffer (GTK_ENTRY (editable)));

l = 7 bytes.

@@ +755,3 @@
+  l = gtk_entry_buffer_get_bytes (gtk_entry_get_buffer (GTK_ENTRY (editable)));
+
+  if (l + new_text_length <= MAX_PASSWORD_SIZE)

That condition fails.
7 + 2 ! <= 8
fails, and carries on.

@@ +761,3 @@
+  gtk_widget_error_bell (GTK_WIDGET (editable));
+
+  available_size = MAX_PASSWORD_SIZE - l;

8 - 7 = 1

@@ +768,3 @@
+                                   (gpointer) screen_sharing_password_insert_text_cb,
+                                   user_data);
+  gtk_editable_insert_text (editable, new_text, available_size, position);

And you'll be sending one byte out of the 2 byte of the character here.
Comment 10 Jonh Wendell 2013-06-26 13:41:54 UTC
Created attachment 247825 [details] [review]
proposed patch, v3

3rd try

this time I restrict the variable available_size to the value returned by g_utf8_strlen(), which avoids that break in the middle of a multi-byte char.
Comment 11 Jonh Wendell 2013-06-26 14:23:29 UTC
pushed into master and 3.8