Patch: Add URL config option

[Dmitrii Sutiagin]

Hi David,

Q: I'm a little confused by the motivation — why does adding that 
'--url' before the <server> argument really make anything easier?
A: The value here is the ability to define <server> in a config file. 
That's the sole purpose of the patch. I added a new argument "--url" for 
this. Specifying both --url and <server> does not make sense, but still 
allowed - did not want to put unnecessary restrictions and to keep
backward compatibility. Defining <server> in config allows to use syntax
like "openconnect --config=XXX" to run openconnect, which in turn is
useful for portability and for systemd units, allowing to set up
openconnect similarly to openvpn:
https://wiki.archlinux.org/index.php/OpenVPN#systemd_service_configuration

It makes sense to also ship a .service file similar to openvpn, but that
would be a separate commit (and I'd need to figure out where to put such 
file in the repo...).

Here is the patch once again:

############################################
commit 52e665b7bf8e8f08d78a36e84ff67c0ef5183ee1
Author: Dmitrii Sutiagin <f3flight at gmail.com>
Date:   Thu May 18 12:49:40 2017 -0700

     Add URL config option

     This new option allows putting <server> argument into a config file,
     which in turn allows cleaner setups of systemd units / automation.
     If --url is used then  argument can be omitted.
     If both --url and <server> are specified then <server> is used.

     Signed-off-by: Dmitrii Sutiagin <f3flight at gmail.com>

diff --git a/main.c b/main.c
index 7869f72..eab4f41 100644
--- a/main.c
+++ b/main.c
@@ -188,6 +188,7 @@ enum {
  	OPT_LOCAL_HOSTNAME,
  	OPT_PROTOCOL,
  	OPT_PASSTOS,
+	OPT_URL,
  };

  #ifdef __sun__
@@ -269,6 +270,7 @@ static const struct option long_options[] = {
  	OPTION("dump-http-traffic", 0, OPT_DUMP_HTTP),
  	OPTION("no-system-trust", 0, OPT_NO_SYSTEM_TRUST),
  	OPTION("protocol", 1, OPT_PROTOCOL),
+	OPTION("url", 1, OPT_URL),
  #ifdef OPENCONNECT_GNUTLS
  	OPTION("gnutls-debug", 1, OPT_GNUTLS_DEBUG),
  #endif
@@ -867,6 +869,7 @@ static void usage(void)
  	printf("      --resolve=HOST:IP           %s\n", _("Use IP when 
connecting to HOST"));
  	printf("      --os=STRING                 %s\n", _("OS type 
(linux,linux-64,win,...) to report"));
  	printf("      --dtls-local-port=PORT      %s\n", _("Set local port 
for DTLS datagrams"));
+	printf("      --url=URL                   %s\n", _("Set URL to connect 
to. If set, <server> argument can be omitted"));
  	print_supported_protocols_usage();

  	printf("\n");
@@ -1068,6 +1071,7 @@ int main(int argc, char **argv)
  	oc_token_mode_t token_mode = OC_TOKEN_MODE_NONE;
  	int reconnect_timeout = 300;
  	int ret;
+	char *url = NULL;
  #ifdef HAVE_NL_LANGINFO
  	char *charset;
  #endif
@@ -1431,6 +1435,9 @@ int main(int argc, char **argv)
  		case OPT_TIMESTAMP:
  			timestamp = 1;
  			break;
+		case OPT_URL:
+			url = keep_config_arg();
+			break;
  #ifdef OPENCONNECT_GNUTLS
  		case OPT_GNUTLS_DEBUG:
  			gnutls_global_set_log_level(atoi(config_arg));
@@ -1448,9 +1455,12 @@ int main(int argc, char **argv)
  	if (optind < argc - 1) {
  		fprintf(stderr, _("Too many arguments on command line\n"));
  		usage();
-	} else if (optind > argc - 1) {
-		fprintf(stderr, _("No server specified\n"));
-		usage();
+	} else if (optind == argc - 1) {
+		url = strdup(argv[optind]);
+	}
+	if (!(url && strlen(url))) {
+			fprintf(stderr, _("No URL or server specified\n"));
+			usage();
  	}

  	if (!vpninfo->sslkey)
@@ -1501,16 +1511,12 @@ int main(int argc, char **argv)
  	if (vpninfo->sslkey && do_passphrase_from_fsid)
  		openconnect_passphrase_from_fsid(vpninfo);

-	if (config_lookup_host(vpninfo, argv[optind]))
+	if (config_lookup_host(vpninfo, url))
  		exit(1);

  	if (!vpninfo->hostname) {
-		char *url = strdup(argv[optind]);
-
  		if (openconnect_parse_url(vpninfo, url))
  			exit(1);
-
-		free(url);
  	}

  	/* Historically, the path in the URL superseded the one in the
############################################

On 05/18/2017 01:34 PM, David Woodhouse wrote:
> On Thu, 2017-05-18 at 13:00 -0700, Dmitry Sutyagin wrote:
>> Hi everyone, I wanted to contribute with the following patch:
>>
>> ############################################
>> commit 52e665b7bf8e8f08d78a36e84ff67c0ef5183ee1
>> Author: Dmitrii Sutiagin <f3flight at gmail.com>
>> Date:   Thu May 18 12:49:40 2017 -0700
>>
>>      Add URL config option
>>
>>      This new option allows putting  argument into a config file,
>>      which in turn allows cleaner setups of systemd units / automation.
>>      If --url is used then  argument can be omitted.
>>      If both --url and  are specified then  is used.
>>
>>      Signed-off-by: Dmitrii Sutiagin <f3flight at gmail.com>
> 
> Hi Dmitry, thanks for the patch. Unfortunately it was whitespace-
> damaged so won't apply. Can you try sending it from a better mail
> client? You can send it to yourself first, then save it to a file and
> try to apply what you receive.
> 
> I'm a little confused by the motivation — why does adding that '--url'
> before the <server> argument really make anything easier?
>