Still more UID-swapped file I/O, and general cleanup of UI file I/O.

GUI no longer uses (an additional layer of) temporary files between
itself and zookeeper.  Zookeeper file read methods now take file-like
objects.  rpkic now opens input files as the real UID, then reverts
back to the effective UID before handing the resulting file off to the
zookeeper.

This caught several more instances of rpkic file I/O that were not
doing the real/effective UID swap properly while loading CSV and VCard
files.

As far as I can tell from testing, this didn't break anything that
worked before.  Whether it fixed all the file I/O problems remains to
be seen.

svn path=/branches/tk705/; revision=6339

4 files changed, 59 insertions, 84 deletions

diff --git a/rpki/csv_utils.py b/rpki/csv_utils.py
index 2864693c..5fa498a1 100644
--- a/rpki/csv_utils.py
+++ b/rpki/csv_utils.py
@@ -46,12 +46,18 @@ class csv_reader(object):
         assert min_columns is None or isinstance(min_columns, int)
         if columns is not None and min_columns is None:
             min_columns = columns
-        self.filename = filename
         self.columns = columns
         self.min_columns = min_columns
         self.comment_characters = comment_characters
-        self.file = open(filename, "r")
-
+        if isinstance(filename, (str, unicode)):
+            # Name of a file to open
+            self.filename = filename
+            self.file = open(filename, "r")
+        else:
+            # File-like object, already opened
+            self.filename = None
+            self.file = filename
+            
     def __iter__(self):
         line_number = 0
         for line in self.file:
diff --git a/rpki/gui/app/views.py b/rpki/gui/app/views.py
index cd2f0e16..1c954765 100644
--- a/rpki/gui/app/views.py
+++ b/rpki/gui/app/views.py
@@ -23,7 +23,6 @@ __version__ = '$Id$'
 
 import os
 import os.path
-from tempfile import NamedTemporaryFile
 import cStringIO
 import csv
 import logging
@@ -137,10 +136,6 @@ def generic_import(request, queryset, configure, form_class=None,
     if request.method == 'POST':
         form = form_class(request.POST, request.FILES)
         if form.is_valid():
-            tmpf = NamedTemporaryFile(prefix='import', suffix='.xml',
-                                      delete=False)
-            tmpf.write(form.cleaned_data['xml'].read())
-            tmpf.close()
             z = Zookeeper(handle=conf.handle)
             handle = form.cleaned_data.get('handle')
             # CharField uses an empty string for the empty value, rather than
@@ -151,7 +146,7 @@ def generic_import(request, queryset, configure, form_class=None,
             try:
                 # configure_repository returns None, so can't use tuple expansion
                 # here.  Unpack the tuple below if post_import_redirect is None.
-                r = configure(z, tmpf.name, handle)
+                r = configure(z, form.cleaned_data['xml'], handle)
             except lxml.etree.XMLSyntaxError as e:
                 logger.exception('caught XMLSyntaxError while parsing uploaded file')
                 messages.error(
@@ -168,8 +163,6 @@ def generic_import(request, queryset, configure, form_class=None,
                     url = queryset.get(issuer=conf,
                                        handle=handle).get_absolute_url()
                 return http.HttpResponseRedirect(url)
-            finally:
-                os.remove(tmpf.name)
     else:
         form = form_class()
 
@@ -333,13 +326,10 @@ def import_asns(request):
     if request.method == 'POST':
         form = forms.ImportCSVForm(request.POST, request.FILES)
         if form.is_valid():
-            f = NamedTemporaryFile(prefix='asns', suffix='.csv', delete=False)
-            f.write(request.FILES['csv'].read())
-            f.close()
             z = Zookeeper(handle=conf.handle, disable_signal_handlers=True)
             try:
                 z.load_asns(
-                    f.name,
+                    request.FILES['csv'],
                     ignore_missing_children=form.cleaned_data['ignore_missing_children']
                 )
             except rpki.irdb.models.Child.DoesNotExist:
@@ -354,8 +344,6 @@ def import_asns(request):
                 z.run_rpkid_now()
                 messages.success(request, 'Successfully imported AS delgations from CSV file.')
                 return redirect(dashboard)
-            finally:
-                os.unlink(f.name)
     else:
         form = forms.ImportCSVForm()
     return render(request, 'app/import_resource_form.html', {
@@ -382,13 +370,10 @@ def import_prefixes(request):
     if request.method == 'POST':
         form = forms.ImportCSVForm(request.POST, request.FILES)
         if form.is_valid():
-            f = NamedTemporaryFile(prefix='prefixes', suffix='.csv', delete=False)
-            f.write(request.FILES['csv'].read())
-            f.close()
             z = Zookeeper(handle=conf.handle, disable_signal_handlers=True)
             try:
                 z.load_prefixes(
-                    f.name,
+                    request.FILES['csv'],
                     ignore_missing_children=form.cleaned_data['ignore_missing_children']
                 )
             except rpki.irdb.models.Child.DoesNotExist:
@@ -400,8 +385,6 @@ def import_prefixes(request):
                 z.run_rpkid_now()
                 messages.success(request, 'Successfully imported AS delgations from CSV file.')
                 return redirect(dashboard)
-            finally:
-                os.unlink(f.name)
     else:
         form = forms.ImportCSVForm()
     return render(request, 'app/import_resource_form.html', {
@@ -904,14 +887,10 @@ def roa_import(request):
     if request.method == 'POST':
         form = forms.ImportCSVForm(request.POST, request.FILES)
         if form.is_valid():
-            import tempfile
-            tmp = tempfile.NamedTemporaryFile(suffix='.csv', prefix='roas', delete=False)
-            tmp.write(request.FILES['csv'].read())
-            tmp.close()
             z = Zookeeper(handle=request.session['handle'],
                           disable_signal_handlers=True)
             try:
-                z.load_roa_requests(tmp.name)
+                z.load_roa_requests(request.FILES['csv'])
             except rpki.csv_utils.BadCSVSyntax as e:
                 messages.error(request,
                                'CSV has bad syntax: %s' % (e,))
@@ -919,8 +898,6 @@ def roa_import(request):
                 z.run_rpkid_now()
                 messages.success(request, 'Successfully imported ROAs.')
                 return redirect(dashboard)
-            finally:
-                os.unlink(tmp.name)
     else:
         form = forms.ImportCSVForm()
     return render(request, 'app/import_resource_form.html', {
@@ -1452,14 +1429,9 @@ class RouterImportView(FormView):
 
     def form_valid(self, form):
         conf = get_conf(self.request.user, self.request.session['handle'])
-        tmpf = NamedTemporaryFile(prefix='import', suffix='.xml',
-                                  delete=False)
-        tmpf.write(form.cleaned_data['xml'].read())
-        tmpf.close()
         z = Zookeeper(handle=conf.handle, disable_signal_handlers=True)
-        z.add_router_certificate_request(tmpf.name)
+        z.add_router_certificate_request(form.cleaned_data['xml'])
         z.run_rpkid_now()
-        os.remove(tmpf.name)
         return super(RouterImportView, self).form_valid(form)
 
     def get_context_data(self, **kwargs):
diff --git a/rpki/irdb/zookeeper.py b/rpki/irdb/zookeeper.py
index bca56926..0ce91800 100644
--- a/rpki/irdb/zookeeper.py
+++ b/rpki/irdb/zookeeper.py
@@ -147,30 +147,19 @@ class PEM_writer(object):
         self.wrote.add(filename)
 
 
-def etree_read(filename_or_etree_wrapper, schema = rpki.relaxng.oob_setup):
+def etree_read(xml_file, schema = rpki.relaxng.oob_setup):
     """
-    Read an etree from a file, verifying then stripping XML namespace
-    cruft.
+    Read an etree from a file-like object, verifying it against a schema.
 
     As a convenience, we also accept an etree_wrapper object in place
     of a filename, in which case we deepcopy the etree directly from
     the etree_wrapper and there's no need for a file.
-
-    As a further convenience, we also accept an Element object,
-    in which case we just validate and return it.
-
-    This function's behavior has changed over time, and the code which
-    calls it is overdue for refactoring, but the relevant code in
-    rpki.gui.app.views is a bit complex, so that yak will have to take
-    a number and wait for its shave, today we have a bug to fix.
     """
 
-    if isinstance(filename_or_etree_wrapper, etree_wrapper):
-        e = copy.deepcopy(filename_or_etree_wrapper.etree)
-    elif isinstance(filename_or_etree_wrapper, (str, unicode)):
-        e = ElementTree(file = filename_or_etree_wrapper).getroot()
+    if isinstance(xml_file, etree_wrapper):
+        e = copy.deepcopy(xml_file.etree)
     else:
-        e = filename_or_etree_wrapper
+        e = ElementTree(file = xml_file).getroot()
     schema.assertValid(e)
     return e
 
@@ -577,7 +566,7 @@ class Zookeeper(object):
 
 
     @django.db.transaction.atomic
-    def configure_child(self, filename, child_handle = None, valid_until = None):
+    def configure_child(self, xml_file, child_handle = None, valid_until = None):
         """
         Configure a new child of this RPKI entity, given the child's XML
         identity file as an input.  Extracts the child's data from the
@@ -587,7 +576,7 @@ class Zookeeper(object):
         data and up-down protocol service URI.
         """
 
-        x = etree_read(filename)
+        x = etree_read(xml_file)
 
         if x.tag != tag_oob_child_request:
             raise BadXMLMessage("Expected %s, got %s", tag_oob_child_request, x.tag)
@@ -669,7 +658,7 @@ class Zookeeper(object):
 
 
     @django.db.transaction.atomic
-    def configure_parent(self, filename, parent_handle = None):
+    def configure_parent(self, xml_file, parent_handle = None):
         """
         Configure a new parent of this RPKI entity, given the output of
         the parent's configure_child command as input.  Reads the parent's
@@ -681,7 +670,7 @@ class Zookeeper(object):
         the user wants to avail herself of the referral or offer.
         """
 
-        x = etree_read(filename)
+        x = etree_read(xml_file)
 
         if x.tag != tag_oob_parent_response:
             raise BadXMLMessage("Expected %s, got %s", tag_oob_parent_response, x.tag)
@@ -758,7 +747,7 @@ class Zookeeper(object):
 
 
     @django.db.transaction.atomic
-    def configure_publication_client(self, filename, sia_base = None, flat = False):
+    def configure_publication_client(self, xml_file, sia_base = None, flat = False):
         """
         Configure publication server to know about a new client, given the
         client's request-for-service message as input.  Reads the client's
@@ -769,7 +758,7 @@ class Zookeeper(object):
 
         # pylint: disable=E1124
 
-        x = etree_read(filename)
+        x = etree_read(xml_file)
 
         if x.tag != tag_oob_publisher_request:
             raise BadXMLMessage("Expected %s, got %s", tag_oob_publisher_request, x.tag)
@@ -876,7 +865,7 @@ class Zookeeper(object):
 
 
     @django.db.transaction.atomic
-    def configure_repository(self, filename, parent_handle = None):
+    def configure_repository(self, xml_file, parent_handle = None):
         """
         Configure a publication repository for this RPKI entity, given the
         repository's response to our request-for-service message as input.
@@ -885,7 +874,7 @@ class Zookeeper(object):
         corresponding parent data in our local database.
         """
 
-        x = etree_read(filename)
+        x = etree_read(xml_file)
 
         if x.tag != tag_oob_repository_response:
             raise BadXMLMessage("Expected %s, got %s", tag_oob_repository_response, x.tag)
@@ -974,7 +963,7 @@ class Zookeeper(object):
 
 
     @django.db.transaction.atomic
-    def load_prefixes(self, filename, ignore_missing_children = False):
+    def load_prefixes(self, csv_file, ignore_missing_children = False):
         """
         Whack IRDB to match prefixes.csv.
         """
@@ -982,7 +971,7 @@ class Zookeeper(object):
         grouped4 = {}
         grouped6 = {}
 
-        for handle, prefix in csv_reader(filename, columns = 2):
+        for handle, prefix in csv_reader(csv_file, columns = 2):
             grouped = grouped6 if ":" in prefix else grouped4
             if handle not in grouped:
                 grouped[handle] = []
@@ -1014,14 +1003,14 @@ class Zookeeper(object):
 
 
     @django.db.transaction.atomic
-    def load_asns(self, filename, ignore_missing_children = False):
+    def load_asns(self, csv_file, ignore_missing_children = False):
         """
         Whack IRDB to match asns.csv.
         """
 
         grouped = {}
 
-        for handle, asn in csv_reader(filename, columns = 2):
+        for handle, asn in csv_reader(csv_file, columns = 2):
             if handle not in grouped:
                 grouped[handle] = []
             grouped[handle].append(asn)
@@ -1049,7 +1038,7 @@ class Zookeeper(object):
 
 
     @django.db.transaction.atomic
-    def load_roa_requests(self, filename):
+    def load_roa_requests(self, csv_file):
         """
         Whack IRDB to match roa.csv.
         """
@@ -1057,7 +1046,7 @@ class Zookeeper(object):
         grouped = {}
 
         # format:  p/n-m asn group
-        for pnm, asn, group in csv_reader(filename, columns = 3):
+        for pnm, asn, group in csv_reader(csv_file, columns = 3):
             key = (asn, group)
             if key not in grouped:
                 grouped[key] = []
@@ -1090,7 +1079,7 @@ class Zookeeper(object):
 
 
     @django.db.transaction.atomic
-    def load_ghostbuster_requests(self, filename, parent = None):
+    def load_ghostbuster_requests(self, vcard_file, parent = None):
         """
         Whack IRDB to match ghostbusters.vcard.
 
@@ -1101,7 +1090,7 @@ class Zookeeper(object):
 
         vcard = []
 
-        for line in open(filename, "r"):
+        for line in vcard_file.read().splitlines(True):
             if not vcard and not line.upper().startswith("BEGIN:VCARD"):
                 continue
             vcard.append(line)
@@ -1706,7 +1695,7 @@ class Zookeeper(object):
 
 
     @django.db.transaction.atomic
-    def add_router_certificate_request(self, router_certificate_request_xml, valid_until = None):
+    def add_router_certificate_request(self, xml_file, valid_until = None):
         """
         Read XML file containing one or more router certificate requests,
         attempt to add request(s) to IRDB.
@@ -1717,7 +1706,7 @@ class Zookeeper(object):
         router-ID supplied in the XML.
         """
 
-        x = etree_read(router_certificate_request_xml, schema = rpki.relaxng.router_certificate)
+        x = etree_read(xml_file, schema = rpki.relaxng.router_certificate)
 
         for x in x.getiterator(tag_router_certificate_request):
 
diff --git a/rpki/rpkic.py b/rpki/rpkic.py
index 51a4d6d2..4cba846c 100644
--- a/rpki/rpkic.py
+++ b/rpki/rpkic.py
@@ -64,15 +64,14 @@ class swap_uids(object):
         return False
 
 
-def read_xml_swapped_uids(filename):
+def open_swapped_uids(*open_args):
     """
-    Read an XML file with UIDs swapped.
+    Open a file with UIDs swapped for the duration of the open() call.
     """
 
-    from lxml.etree import ElementTree
-
     with swap_uids():
-        return ElementTree(file = filename).getroot()
+        return open(*open_args)
+
 
 class main(Cmd):
 
@@ -378,7 +377,8 @@ class main(Cmd):
         up-down protocol service URI.
         """
 
-        r, child_handle = self.zoo.configure_child(read_xml_swapped_uids(args.child_xml), args.child_handle, args.valid_until)
+        with open_swapped_uids(args.child_xml) as f:
+            r, child_handle = self.zoo.configure_child(f, args.child_handle, args.valid_until)
         with swap_uids():
             r.save("%s.%s.parent-response.xml" % (self.zoo.handle, child_handle), sys.stdout)
         self.zoo.synchronize_ca()
@@ -425,7 +425,8 @@ class main(Cmd):
         synchronize here, run the synchronize command yourself.
         """
 
-        r, parent_handle = self.zoo.configure_parent(read_xml_swapped_uids(args.parent_xml), args.parent_handle)
+        with open_swapped_uids(args.parent_xml) as f:
+            r, parent_handle = self.zoo.configure_parent(f, args.parent_handle)
         with swap_uids():
             r.save("%s.%s.repository-request.xml" % (self.zoo.handle, parent_handle), sys.stdout)
 
@@ -496,7 +497,8 @@ class main(Cmd):
         message containing the repository's BPKI data and service URI.
         """
 
-        r, client_handle = self.zoo.configure_publication_client(read_xml_swapped_uids(args.client_xml), args.sia_base, args.flat)
+        with open_swapped_uids(args.client_xml) as f:
+            r, client_handle = self.zoo.configure_publication_client(f, args.sia_base, args.flat)
         with swap_uids():
             r.save("%s.repository-response.xml" % client_handle.replace("/", "."), sys.stdout)
         try:
@@ -537,7 +539,8 @@ class main(Cmd):
         corresponding parent data in our local database.
         """
 
-        self.zoo.configure_repository(read_xml_swapped_uids(args.repository_xml), args.parent_handle)
+        with open_swapped_uids(args.repository_xml) as f:
+            self.zoo.configure_repository(f, args.parent_handle)
         self.zoo.synchronize_ca()
 
 
@@ -610,7 +613,8 @@ class main(Cmd):
         Load prefixes into IRDB from CSV file.
         """
 
-        self.zoo.load_prefixes(args.prefixes_csv, True)
+        with open_swapped_uids(args.prefixes_csv) as f:
+            self.zoo.load_prefixes(f, True)
         if self.autosync:
             self.zoo.run_rpkid_now()
 
@@ -746,7 +750,8 @@ class main(Cmd):
         Load ASNs into IRDB from CSV file.
         """
 
-        self.zoo.load_asns(args.asns_csv, True)
+        with open_swapped_uids(args.asns_csv) as f:
+            self.zoo.load_asns(f, True)
         if self.autosync:
             self.zoo.run_rpkid_now()
 
@@ -758,7 +763,8 @@ class main(Cmd):
         Load ROA requests into IRDB from CSV file.
         """
 
-        self.zoo.load_roa_requests(args.roa_requests_csv)
+        with open_swapped_uids(args.roa_requests_csv) as f:
+            self.zoo.load_roa_requests(f)
         if self.autosync:
             self.zoo.run_rpkid_now()
 
@@ -770,7 +776,8 @@ class main(Cmd):
         Load Ghostbuster requests into IRDB from file.
         """
 
-        self.zoo.load_ghostbuster_requests(args.ghostbuster_requests)
+        with open_swapped_uids(args.ghostbuster_requests) as f:
+            self.zoo.load_ghostbuster_requests(f)
         if self.autosync:
             self.zoo.run_rpkid_now()
 
@@ -783,7 +790,8 @@ class main(Cmd):
         Load router certificate request(s) into IRDB from XML file.
         """
 
-        self.zoo.add_router_certificate_request(read_xml_swapped_uids(args.router_certificate_request_xml), args.valid_until)
+        with open_swapped_uids(args.router_certificate_request_xml) as f:
+            self.zoo.add_router_certificate_request(f, args.valid_until)
         if self.autosync:
             self.zoo.run_rpkid_now()