Replaced URLGrabber with requests (rhbz#1141245)

Because urlgrabber is being dropped by many significant software projects in
Fedora, Pykickstart can't keep depending on it once it is converted to Python 3
(rhbz#985310).

This commit replaces the dep with requests with SSL verification turned on by
default. This could pose trouble in Python 2 if proper underlying packages
aren't installed [1]. In case this becomes a problem, the SSL_VERIFY constant
in constants.py may be switched to False.

[1] https://github.com/kennethreitz/requests/issues/749#issuecomment-19187417

pykickstart.spec

@@ -15,11 +15,11 @@ Group: System Environment/Libraries
 BuildArch: noarch
 BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildRequires: python-devel, gettext, python-setuptools
-BuildRequires: python-urlgrabber
+BuildRequires: python-requests
 %if ! 0%{?rhel}
 BuildRequires: transifex-client
 %endif
-Requires: python, python-urlgrabber
+Requires: python, python-requests
 
 %description
 The pykickstart package is a python library for manipulating kickstart

pykickstart/constants.py

@@ -61,3 +61,5 @@
 GROUP_REQUIRED = 0
 GROUP_DEFAULT = 1
 GROUP_ALL = 2
+
+SSL_VERIFY = True

pykickstart/load.py

@@ -0,0 +1,88 @@
+
+import requests
+import shutil
+import tempfile
+
+from pykickstart.constants import SSL_VERIFY
+from pykickstart.errors import KickstartError
+from requests.exceptions import SSLError, RequestException
+
+_is_url = lambda location: '://' in location # RFC 3986
+
+def load_to_str(location):
+    '''Load a destination URL or file into a string.
+    Type of input is inferred automatically.
+
+    Arguments:
+    location -- URL or file name to load
+
+    Returns: string with contents
+    Raises: KickstartError on error reading'''
+
+    if _is_url(location):
+        return _load_url(location)
+    else:
+        return _load_file(location)
+
+
+def load_to_file(location, destination):
+    '''Load a destination URL or file into a file name.
+    Type of input is inferred automatically.
+
+    Arguments:
+    location -- URL or file name to load
+    destination -- destination file name to write to
+
+    Returns: file name with contents
+    Raises: KickstartError on error reading or writing'''
+
+    if _is_url(location):
+        contents =  _load_url(location)
+
+        # Write to file
+        try:
+            with open(destination, 'w') as fh:
+                fh.write(contents)
+        except IOError as e:
+            raise KickstartError('Error writing file: {e}'.format(e=str(e)))
+
+        return destination
+    else:
+        _copy_file(location, destination)
+        return destination
+
+
+
+def _load_url(location):
+    '''Load a location (URL or filename) and return contents as string'''
+
+    try:
+        request = requests.get(location, verify=SSL_VERIFY)
+    except SSLError as e:
+        raise KickstartError('Error securely accessing URL "{url}": {msg}'.format(
+            url=location, msg=e))
+    except RequestException as e:
+        raise KickstartError('Error accessing URL "{url}": {msg}'.format(
+            url=location, msg=e))
+
+    return request.content
+
+
+def _load_file(filename):
+    '''Load a file's contents and return them as a string'''
+
+    try:
+        with open(filename, 'r') as fh:
+            contents = fh.read()
+    except IOError as e:
+        raise KickstartError('Error opening file: {e}'.format(e=str(e)))
+
+    return contents
+
+def _copy_file(filename, destination):
+    '''Copy file to destination'''
+
+    try:
+        shutil.copyfile(filename, destination)
+    except OSError as e:
+        raise KickstartError('Error copying file: {e}'.format(str(e)))

pykickstart/parser.py

@@ -36,12 +36,11 @@
 import shlex
 import tempfile
 from optparse import OptionParser
-from urlgrabber import urlread
-import urlgrabber.grabber as grabber
 
 from pykickstart import constants, version
 from pykickstart.errors import KickstartError, KickstartParseError, KickstartValueError, formatErrorMsg
 from pykickstart.ko import KickstartObject
+from pykickstart.load import load_to_str
 from pykickstart.sections import PackageSection, PreScriptSection, PostScriptSection, TracebackScriptSection
 
 import gettext
@@ -85,25 +84,16 @@ def _preprocessStateMachine (lineIter):
             raise KickstartParseError(formatErrorMsg(lineno, msg=_("Illegal url for %%ksappend: %s") % ll))
 
         try:
-            url = grabber.urlopen(ksurl)
-        except grabber.URLGrabError, e:
-            raise KickstartError(formatErrorMsg(lineno, msg=_("Unable to open %%ksappend file: %s") % e.strerror))
-        else:
-            # Sanity check result.  Sometimes FTP doesn't catch a file
-            # is missing.
-            try:
-                if url.size < 1:
-                    raise KickstartError(formatErrorMsg(lineno, msg=_("Unable to open %%ksappend file")))
-            except:
-                raise KickstartError(formatErrorMsg(lineno, msg=_("Unable to open %%ksappend file")))
+            contents = load_to_str(ksurl)
+        except KickstartError as e:
+            raise KickstartError(formatErrorMsg(lineno, msg=_("Unable to open %%ksappend file: %s") % str(e)))
 
         # If that worked, write the remote file to the output kickstart
         # file in one burst.  Then close everything up to get ready to
         # read ahead in the input file.  This allows multiple %ksappend
         # lines to exist.
-        if url is not None:
-            os.write(outF, url.read())
-            url.close()
+        if contents is not None:
+            os.write(outF, contents)
 
     # All done - close the temp file and return its location.
     os.close(outF)
@@ -126,13 +116,11 @@ def preprocessKickstart (f):
         run.  Returns the location of the complete kickstart file.
     """
     try:
-        fh = grabber.urlopen(f)
-    except grabber.URLGrabError, e:
-        raise KickstartError(formatErrorMsg(0, msg=_("Unable to open input kickstart file: %s") % e.strerror))
+        contents = load_to_str(f)
+    except KickstartError as e:
+        raise KickstartError(formatErrorMsg(0, msg=_("Unable to open input kickstart file: %s") % str(e)))
 
-    rc = _preprocessStateMachine (iter(fh.readlines()))
-    fh.close()
-    return rc
+    return _preprocessStateMachine (iter(contents.splitlines(True)))
 
 class PutBackIterator(Iterator):
     def __init__(self, iterable):
@@ -726,9 +714,9 @@ def readKickstart(self, f, reset=True):
         self.currentdir[self._includeDepth] = cd
 
         try:
-            s = urlread(f)
-        except grabber.URLGrabError, e:
-            raise KickstartError(formatErrorMsg(0, msg=_("Unable to open input kickstart file: %s") % e.strerror))
+            s = load_to_str(f)
+        except KickstartError as e:
+            raise KickstartError(formatErrorMsg(0, msg=_("Unable to open input kickstart file: %s") % str(e)))
 
         self.readKickstartFromString(s, reset=False)
 

tools/ksvalidator

@@ -24,9 +24,9 @@ import os
 import sys
 import warnings
 import tempfile
-import urlgrabber
 import shutil
 from pykickstart.errors import KickstartError, KickstartParseError, KickstartValueError, KickstartVersionError
+from pykickstart.load import load_to_file
 from pykickstart.parser import KickstartParser, preprocessKickstart
 from pykickstart.version import DEVEL, makeVersion, versionMap
 
@@ -70,8 +70,8 @@ if opts.listversions:
 
 destdir = tempfile.mkdtemp("", "ksvalidator-tmp-", "/tmp")
 try:
-    f = urlgrabber.urlgrab(opts.ksfile, filename="%s/ks.cfg" % destdir)
-except urlgrabber.grabber.URLGrabError, e:
+    f = load_to_file(opts.ksfile, "%s/ks.cfg" % destdir)
+except KickstartError as e:
     print(_("Error reading %s:\n%s") % (opts.ksfile, e))
     cleanup(destdir)