From 4ecbe9f176b6d29aa51bf7800d0495e1c949f5bc Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 11 Aug 2016 14:38:53 +0200
Subject: [PATCH 1/1] 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
(cherry picked from commit 62bdd278d69cc396479af9d05a0b776d5ad386bf)
---
 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);
-- 
2.7.4