finesse the URL-decoding of the GP login args

Unsurprisingly, it's messier than I thought it was.  Some of them definitely
need to be URL-decoded, and some definitely shouldn't be.
https://gitlab.com/openconnect/openconnect/-/issues/147#note_429943037

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

diff --git a/auth-globalprotect.c b/auth-globalprotect.c
index 5185d4746543f8a7d9e18f7b8433b8a1687b2e04..ef17b2f9713620147c47ea16a1c25557bec1b17a 100644 (file)
--- a/auth-globalprotect.c
+++ b/auth-globalprotect.c
@@ -334,11 +334,13 @@ static int parse_login_xml(struct openconnect_info *vpninfo, xmlNode *xml_node,
                        if (value && (!value[0] || !strcmp(value, "(null)") || !strcmp(value, "-1"))) {
                                free(value);
                                value = NULL;
-                       } else {
-                               /* XX: The usage of URL encoding in the fields sent by GP servers here is
-                                * inconsistent, but in particular the value "%28empty_domain%29" keeps popping up
-                                * in places where the server expects "(empty_domain)" (like the stupidly redundant
-                                * logout operation). So we do this to be safe and to ensure logout succeeds.
+                       } else if (arg->save) {
+                               /* XX: Some of the fields returned here (e.g. portal-*cookie) should NOT be
+                                * URL-decoded in order to be reused correctly, but the ones which get saved
+                                * into "cookie" must be URL-decoded. They will be needed for the (stupidly
+                                * redundant) logout parameters. In particular the domain value "%28empty_domain%29"
+                                * appears frequently in the wild, and it needs to be decoded here for the logout
+                                * request to succeed.
                                 */
                                urldecode_inplace(value);
                        }