[network-manager-openvpn] fix recent change in behavior for "comp-lzo=no" setting
- From: Thomas Haller <thaller src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [network-manager-openvpn] fix recent change in behavior for "comp-lzo=no" setting
- Date: Thu, 11 Aug 2016 14:10:30 +0000 (UTC)
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]