Patch: Add URL config option

[Dmitry Sutyagin]

Any news on this?

2017-05-18 14:25 GMT-07:00 Dmitrii Sutiagin <f3flight at gmail.com>:
> 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?
>>
>