Support for auto-update via proxy http

[teluq-pbrideau]

First, what a pain to document this bug! Sorry for the wall of text.

What is the problem

Composer is installed correctly, but never get updated when using an http proxy:

    class { 'php::composer':
      proxy_type   => 'http',
      proxy_server => 'http://myproxy.example.com:3128',
    }

I should define proxy_type as https for the update to work correctly, but is not really intuitive…

    class { 'php::composer':
      proxy_type   => 'https',
      proxy_server => 'http://myproxy.example.com:3128',
    }

Source of the problem

The original author of the php::composer::auto_update seems to have mix the usage of http_proxy and https_proxy environment variables.
The http/https part is not for the communication to the proxy server, but for the destination URL. The reference to these environment variables are often badly documented on the internet…

Per the manpage of curl(1):

  http_proxy [protocol://]<host>[:port]
         Sets the proxy server to use for HTTP.

  HTTPS_PROXY [protocol://]<host>[:port]
         Sets the proxy server to use for HTTPS.

Therefore, if only http_proxy is defined, a request to https://getcomposer.org (HTTPS) will not be done through the proxy, only http://getcomposer.org (HTTP) will be done through the proxy.

Both http_proxy and https_proxy should be defined if both requests to http://getcomposer.org and https://getcomposer.org want to be redirected through proxy, no matter if the proxy uses https or http as protocol.

If my proxy does not use HTTPS as protocol, environment should be defined as

http_proxy=http://myproxy.example.com:3128
https_proxy=http://myproxy.example.com:3128

If my proxy uses HTTPS as protocol, environment should be defined as

http_proxy=https://mytlsproxy.example.com:3128
https_proxy=https://mytlsproxy.example.com:3128

Archive module correctly defined

proxy_type in the archive module is used to defined the protocol of the proxy only if the proxy_server is not a valid URI

          uri = URI(options[:proxy_server])
          uri = URI("#{options[:proxy_type]}://#{options[:proxy_server]}") unless uri.scheme

https://github.com/voxpupuli/puppet-archive/blob/63474f6709e4c44b1b4c6852b32335f34df32877/lib/puppet_x/bodeco/util.rb#L78

therefore, the proxy_type parameter is correctly used in php::composer to define the proxy, only if you do not set proxy_server as full URI:

  archive { 'download composer':
    path         => $path,
    source       => $source,
    proxy_type   => $proxy_type,
    proxy_server => $proxy_server,
  }

So you should be able to call this module as both way:

    archive { 'example':
      path         => $path,
      source       => $source,
      proxy_type   => 'http',
      proxy_server => 'myproxy.example.com:3128',
    }

    archive { 'example':
      path         => $path,
      source       => $source,
      proxy_server => 'http://myproxy.example.com:3128',
    }

Auto-update

Auto-update does not use this proxy_type parameter the same way.

Here is how it is defined in php::composer::auto_update:

  if $proxy_type and $proxy_server {
    $env = ['HOME=/root', "${proxy_type}_proxy=${proxy_server}"]
  } else {
    $env = ['HOME=/root']
  }

Solutions

simple

The simple solution is to not use the proxy_type parameter at all.

  if $proxy_server {
    $env = [
      'HOME=/root',
      "http_proxy=${proxy_server}",
      "https_proxy=${proxy_server}",
    ]
  } else {
    $env = ['HOME=/root']
  }

clean

The cleanest solution in my opinion would be to detect if proxy_server is a valid URI the same way the archive module do, and throw an error to the user if not correctly defined:

  if $proxy_server {
    if proxy_server =~ /^https?:\/\// {
      $_proxy_server = $proxy_server
    } else {
      if $proxy_type {
        $_proxy_server = "${proxy_type}://${proxy_server}"
      } else {
        fail('proxy_type must be defined if proxy_server is not full URI (https://example.com)')
      }
    }
    $env = [
      'HOME=/root',
      "http_proxy=${_proxy_server}",
      "https_proxy=${_proxy_server}",
    ]
  } else {
    $env = ['HOME=/root']
  }

I will prepare a PR on this tomorrow, any feedback would be appreciated.