Fix urlopen usage to use open_url instead

Add a travis test for urlopen usage

.travis.yml

@@ -14,6 +14,8 @@ addons:
 install:
   - pip install tox PyYAML Jinja2 sphinx
 script:
+# urllib2's defaults are not secure enough for us
+- ./test/code-smell/replace-urlopen.sh .
 - if test x"$TOKENV" != x'py24' ; then  tox ; fi
 - if test x"$TOKENV" = x'py24' ; then python2.4 -V && python2.4 -m compileall -fq -x 'module_utils/(a10|rax|openstack|ec2|gce).py' lib/ansible/module_utils ; fi
   #- make -C docsite all

contrib/inventory/abiquo.py

@@ -45,26 +45,24 @@ import os
 import sys
 import time
 import ConfigParser
-import urllib2
-import base64
 
 try:
     import json
 except ImportError:
     import simplejson as json
 
+from ansible.module_utils.urls import open_url
+
 def api_get(link, config):
     try:
         if link == None:
-            request = urllib2.Request(config.get('api','uri')+config.get('api','login_path'))
-            request.add_header("Accept",config.get('api','login_type'))
+            url = config.get('api','uri') + config.get('api','login_path')
+            headers = {"Accept": config.get('api','login_type')}
         else:
-            request = urllib2.Request(link['href']+'?limit=0')
-            request.add_header("Accept",link['type'])
-        # Auth
-        base64string = base64.encodestring('%s:%s' % (config.get('auth','apiuser'),config.get('auth','apipass'))).replace('\n', '')
-        request.add_header("Authorization", "Basic %s" % base64string)
-        result = urllib2.urlopen(request)
+            url = link['href'] + '?limit=0'
+            headers = {"Accept": link['type']}
+        result = open_url(url, headers=headers, url_username=config.get('auth','apiuser').replace('\n', ''),
+                url_password=config.get('auth','apipass').replace('\n', ''))
         return json.loads(result.read())
     except:
         return None

contrib/inventory/collins.py

@@ -67,7 +67,6 @@ Tested against Ansible 1.8.2 and Collins 1.3.0.
 
 
 import argparse
-import base64
 import ConfigParser
 import logging
 import os
@@ -76,7 +75,6 @@ import sys
 from time import time
 import traceback
 import urllib
-import urllib2
 
 try:
     import json
@@ -85,6 +83,7 @@ except ImportError:
 
 from six import iteritems
 
+from ansible.module_utils.urls import open_url
 
 class CollinsDefaults(object):
     ASSETS_API_ENDPOINT = '%s/api/assets'
@@ -198,10 +197,11 @@ class CollinsInventory(object):
                 (CollinsDefaults.ASSETS_API_ENDPOINT % self.collins_host),
                 urllib.urlencode(query_parameters, doseq=True)
             )
-            request = urllib2.Request(query_url)
-            request.add_header('Authorization', self.basic_auth_header)
             try:
-                response = urllib2.urlopen(request, timeout=self.collins_timeout_secs)
+                response = open_url(query_url,
+                        timeout=self.collins_timeout_secs,
+                        url_username=self.collins_username,
+                        url_password=self.collins_password)
                 json_response = json.loads(response.read())
                 # Adds any assets found to the array of assets.
                 assets += json_response['data']['Data']
@@ -261,8 +261,6 @@ class CollinsInventory(object):
 
         log_path = config.get('collins', 'log_path')
         self.log_location = log_path + '/ansible-collins.log'
-        self.basic_auth_header = "Basic %s" % base64.encodestring(
-            '%s:%s' % (self.collins_username, self.collins_password))[:-1]
 
     def parse_cli_args(self):
         """ Command line argument processing """

contrib/inventory/openshift.py

@@ -28,7 +28,6 @@ version_added: None
 author: Michael Scherer
 '''
 
-import urllib2
 try:
     import json
 except ImportError:
@@ -39,6 +38,8 @@ import sys
 import ConfigParser
 import StringIO
 
+from ansible.module_utils.urls import open_url
+
 configparser = None
 
 
@@ -66,34 +67,21 @@ def get_config(env_var, config_var):
     return result
 
 
-def get_json_from_api(url):
-    req = urllib2.Request(url, None, {'Accept': 'application/json; version=1.5'})
-    response = urllib2.urlopen(req)
+def get_json_from_api(url, username, password):
+    headers = {'Accept': 'application/json; version=1.5'}
+    response = open_url(url, headers=headers, url_username=username, url_password=password)
     return json.loads(response.read())['data']
 
 
-def passwd_setup(top_level_url, username, password):
-    # create a password manager
-    password_mgr = urllib2.HTTPPasswordMgrWithDefaultRealm()
-    password_mgr.add_password(None, top_level_url, username, password)
-
-    handler = urllib2.HTTPBasicAuthHandler(password_mgr)
-    opener = urllib2.build_opener(handler)
-
-    urllib2.install_opener(opener)
-
-
 username = get_config('ANSIBLE_OPENSHIFT_USERNAME', 'default_rhlogin')
 password = get_config('ANSIBLE_OPENSHIFT_PASSWORD', 'password')
 broker_url = 'https://%s/broker/rest/' % get_config('ANSIBLE_OPENSHIFT_BROKER', 'libra_server')
 
 
-passwd_setup(broker_url, username, password)
-
-response = get_json_from_api(broker_url + '/domains')
+response = get_json_from_api(broker_url + '/domains', username, password)
 
 response = get_json_from_api("%s/domains/%s/applications" %
-                             (broker_url, response[0]['id']))
+                             (broker_url, response[0]['id']), username, password)
 
 result = {}
 for app in response:

contrib/inventory/proxmox.py

@@ -16,7 +16,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import urllib
-import urllib2
 try:
     import json
 except ImportError:
@@ -27,6 +26,7 @@ from optparse import OptionParser
 
 from six import iteritems
 
+from ansible.module_utils.urls import open_url
 
 class ProxmoxNodeList(list):
     def get_names(self):
@@ -86,7 +86,7 @@ class ProxmoxAPI(object):
             'password': self.options.password,
         })
 
-        data = json.load(urllib2.urlopen(request_path, request_params))
+        data = json.load(open_url(request_path, data=request_params))
 
         self.credentials = {
             'ticket': data['data']['ticket'],
@@ -94,11 +94,10 @@ class ProxmoxAPI(object):
         }
 
     def get(self, url, data=None):
-        opener = urllib2.build_opener()
-        opener.addheaders.append(('Cookie', 'PVEAuthCookie={}'.format(self.credentials['ticket'])))
-
         request_path = '{}{}'.format(self.options.url, url)
-        request = opener.open(request_path, data)
+
+        headers = {'Cookie': 'PVEAuthCookie={}'.format(self.credentials['ticket'])}
+        request = open_url(request_path, data=data, headers=headers)
 
         response = json.load(request)
         return response['data']

lib/ansible/galaxy/api.py

@@ -21,10 +21,11 @@
 #
 ########################################################################
 import json
-from urllib2 import urlopen, quote as urlquote, HTTPError
+from urllib2 import quote as urlquote, HTTPError
 from urlparse import urlparse
 
 from ansible.errors import AnsibleError, AnsibleOptionsError
+from ansible.module_utils.urls import open_url
 
 class GalaxyAPI(object):
     ''' This class is meant to be used as a API client for an Ansible Galaxy server '''
@@ -61,7 +62,7 @@ class GalaxyAPI(object):
         return 'v1'
 
         try:
-            data = json.load(urlopen(api_server))
+            data = json.load(open_url(api_server))
             return data.get("current_version", 'v1')
         except Exception as e:
             # TODO: report error
@@ -85,7 +86,7 @@ class GalaxyAPI(object):
         url = '%s/roles/?owner__username=%s&name=%s' % (self.baseurl, user_name, role_name)
         self.galaxy.display.vvvv("- %s" % (url))
         try:
-            data = json.load(urlopen(url))
+            data = json.load(open_url(url))
             if len(data["results"]) != 0:
                 return data["results"][0]
         except:
@@ -102,13 +103,13 @@ class GalaxyAPI(object):
 
         try:
             url = '%s/roles/%d/%s/?page_size=50' % (self.baseurl, int(role_id), related)
-            data = json.load(urlopen(url))
+            data = json.load(open_url(url))
             results = data['results']
             done = (data.get('next', None) == None)
             while not done:
                 url = '%s%s' % (self.baseurl, data['next'])
                 self.galaxy.display.display(url)
-                data = json.load(urlopen(url))
+                data = json.load(open_url(url))
                 results += data['results']
                 done = (data.get('next', None) == None)
             return results
@@ -122,7 +123,7 @@ class GalaxyAPI(object):
 
         try:
             url = '%s/%s/?page_size' % (self.baseurl, what)
-            data = json.load(urlopen(url))
+            data = json.load(open_url(url))
             if "results" in data:
                 results = data['results']
             else:
@@ -133,7 +134,7 @@ class GalaxyAPI(object):
             while not done:
                 url = '%s%s' % (self.baseurl, data['next'])
                 self.galaxy.display.display(url)
-                data = json.load(urlopen(url))
+                data = json.load(open_url(url))
                 results += data['results']
                 done = (data.get('next', None) == None)
             return results
@@ -165,7 +166,7 @@ class GalaxyAPI(object):
 
         self.galaxy.display.debug("Executing query: %s" % search_url)
         try:
-            data = json.load(urlopen(search_url))
+            data = json.load(open_url(search_url))
         except HTTPError as e:
             raise AnsibleError("Unsuccessful request to server: %s" % str(e))
 

lib/ansible/galaxy/role.py

@@ -26,10 +26,10 @@ import tarfile
 import tempfile
 import yaml
 from shutil import rmtree
-from urllib2 import urlopen
 
 from ansible import constants as C
 from ansible.errors import AnsibleError
+from ansilbe.module_utils.urls import open_url
 
 try:
     from __main__ import display
@@ -162,7 +162,7 @@ class GalaxyRole(object):
             display.display("- downloading role from %s" % archive_url)
 
             try:
-                url_file = urlopen(archive_url)
+                url_file = open_url(archive_url)
                 temp_file = tempfile.NamedTemporaryFile(delete=False)
                 data = url_file.read()
                 while data:

lib/ansible/plugins/callback/hipchat.py

@@ -17,7 +17,6 @@
 
 import os
 import urllib
-import urllib2
 
 try:
     import prettytable
@@ -26,6 +25,7 @@ except ImportError:
     HAS_PRETTYTABLE = False
 
 from ansible.plugins.callback import CallbackBase
+from ansible.module_utils.urls import open_url
 
 class CallbackModule(CallbackBase):
     """This is an example ansible callback plugin that sends status
@@ -82,7 +82,7 @@ class CallbackModule(CallbackBase):
 
         url = ('%s?auth_token=%s' % (self.msg_uri, self.token))
         try:
-            response = urllib2.urlopen(url, urllib.urlencode(params))
+            response = open_url(url, data=urllib.urlencode(params))
             return response.read()
         except:
             self.display.warning('Could not submit message to hipchat')

test/code-smell/replace-urlopen.sh

@@ -0,0 +1,12 @@
+#!/bin/sh
+
+BASEDIR=${1-"."}
+
+URLLIB_USERS=$(find "$BASEDIR" -name '*.py' -exec grep -H urlopen \{\} \;)
+URLLIB_USERS=$(echo "$URLLIB_USERS" | sed '/\(\n\|lib\/ansible\/module_utils\/urls.py\)/d')
+if test -n "$URLLIB_USERS" ; then
+  printf "$URLLIB_USERS"
+  exit 1
+else
+  exit 0
+fi