[network-manager-openvpn] fix recent change in behavior for "comp-lzo=no" setting



commit 62bdd278d69cc396479af9d05a0b776d5ad386bf
Author: Thomas Haller <thaller redhat com>
Date:   Thu Aug 11 14:38:53 2016 +0200

    fix recent change in behavior for "comp-lzo=no" setting
    
    openvpn supports 4 different ways for --comp-lzo:
      1) no --comp-lzo option
      2) --comp-lzo yes
      3) --comp-lzo [adaptive]
      4) --comp-lzo no
    
    Before commit 2ecf18c25a7bee7f0122d9d666a7e11cd8b55ea3, nm-openvpn
    only supported 1) and 2). Those were expressed in NM's connection
    by either omitting the comp-lzo setting or setting "comp-lzo=yes".
    
    Arguably due to a bug, old plasma-nm would configure connections with
    comp-lzo=no to mean 1), so after update of nm-openvpn to 2ecf18c2 those
    connections changed to mean 4), which broke some existing configurations.
    That was later attemted to be fixed in plasma-nm by commit [1], which
    however only affects new connections and cannot fix existing connections
    for users.
    
    Ultimatley, the "comp-lzo=no" setting is spoiled due to that.
    
    The fix is to add a new setting "comp-lzo=no-by-default" which shall
    have the meaning 4) and pass "--comp-lzo no" to openvpn. A connection
    with "comp-lzo=no" is again treated as 1).
    
    This fixes old connections that were created by old plasma-nm before commit
    [1] by restoring the old meaning.
    
    This however now breaks users of recent nm-openvpn which were deliberately setting
    "comp-lzo=no" to mean option 4), most notably users of recent plasma-nm (since
    commit [1]). Users of the properties plugin for nm-connection-editor/gnome-control-center
    are anyway unable to configure "comp-lzo=no" in the UI, so probably isn't a real
    issue for many users.
    
    plasma-nm bugs:
      [1] https://quickgit.kde.org/?p=plasma-nm.git&a=commit&h=31bcd5f2cffd1c19fbd10ab0f4172f2d82eff194
      https://bugs.kde.org/show_bug.cgi?id=365816
      https://bugzilla.redhat.com/show_bug.cgi?id=1365663
    
    https://bugzilla.gnome.org/show_bug.cgi?id=769177
    https://bugzilla.redhat.com/show_bug.cgi?id=1355688
    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=833166

 properties/auth-helpers.c  |    5 ++++-
 properties/import-export.c |   14 ++++++++++++--
 src/nm-openvpn-service.c   |   24 ++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 3 deletions(-)
---
diff --git a/properties/auth-helpers.c b/properties/auth-helpers.c
index 85985c8..7244df1 100644
--- a/properties/auth-helpers.c
+++ b/properties/auth-helpers.c
@@ -2010,7 +2010,10 @@ advanced_dialog_new_hash_from_dialog (GtkWidget *dialog, GError **error)
        if (gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (widget))) {
                /* we only have a checkbox, which we either map to "--comp-lzo yes" or
                 * no "--comp-lzo" flag. In the UI, we cannot express "--comp-lzo [adaptive]"
-                * or "--comp-lzo no". */
+                * or "--comp-lzo no".
+                *
+                * Note that "--comp-lzo no" must be encoded as "comp-lzo=no-by-default" (bgo#769177).
+                */
                g_hash_table_insert (hash, g_strdup (NM_OPENVPN_KEY_COMP_LZO), g_strdup ("yes"));
        }
 
diff --git a/properties/import-export.c b/properties/import-export.c
index 90b4631..ddb1cd3 100644
--- a/properties/import-export.c
+++ b/properties/import-export.c
@@ -951,13 +951,23 @@ do_import (const char *path, const char *contents, gsize contents_len, GError **
                }
 
                if (NM_IN_STRSET (params[0], NMV_OVPN_TAG_COMP_LZO)) {
+                       const char *v;
+
                        if (!args_params_check_nargs_minmax (params, 0, 1, &line_error))
                                goto handle_line_error;
-                       if (!NM_IN_STRSET (params[1], NULL, "no", "yes", "adaptive")) {
+
+                       v = params[1] ?: "adaptive";
+
+                       if (nm_streq (v, "no")) {
+                               /* old plasma-nm used to set "comp-lzo=no" to mean unset, thus it spoiled
+                                * to "no" option to be used in the connection. Workaround that, by instead
+                                * using "no-by-default" (bgo#769177). */
+                               v = "no-by-default";
+                       } else if (!NM_IN_STRSET (v, "yes", "adaptive")) {
                                line_error = g_strdup_printf (_("unsupported comp-lzo argument"));
                                goto handle_line_error;
                        }
-                       setting_vpn_add_data_item (s_vpn, NM_OPENVPN_KEY_COMP_LZO, params[1] ?: "adaptive");
+                       setting_vpn_add_data_item (s_vpn, NM_OPENVPN_KEY_COMP_LZO, v);
                        continue;
                }
 
diff --git a/src/nm-openvpn-service.c b/src/nm-openvpn-service.c
index e5a1aa9..9238a45 100644
--- a/src/nm-openvpn-service.c
+++ b/src/nm-openvpn-service.c
@@ -1250,6 +1250,30 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
        }
 
        tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_COMP_LZO);
+
+       /* openvpn understands 4 different modes for --comp-lzo, which have
+        * different meaning:
+        *  1) no --comp-lzo option
+        *  2) --comp-lzo yes
+        *  3) --comp-lzo [adaptive]
+        *  4) --comp-lzo no
+        *
+        * In the past, nm-openvpn only supported 1) and 2) by having no
+        * comp-lzo connection setting or "comp-lzo=yes", respectively.
+        *
+        * However, old plasma-nm would set "comp-lzo=no" in the connection
+        * to mean 1). Thus, "comp-lzo=no" is spoiled to mean 4) in order
+        * to preserve backward compatibily.
+        * We use instead a special value "no-by-default" to express "no".
+        *
+        * See bgo#769177
+        */
+       if (NM_IN_STRSET (tmp, "no")) {
+               /* means no --comp-lzo option. */
+               tmp = NULL;
+       } else if (NM_IN_STRSET (tmp, "no-by-default"))
+               tmp = "no";
+
        if (NM_IN_STRSET (tmp, "yes", "no", "adaptive")) {
                add_openvpn_arg (args, "--comp-lzo");
                add_openvpn_arg (args, tmp);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]