tools: modernize nfd-status-http-server and improve error handling

Change-Id: I5715c35f79d7dcd8dbefed328ebb625e9150a846

tools/nfd-status-http-server.py

@@ -1,8 +1,6 @@
 #!/usr/bin/env python3
-# -*- Mode: python; py-indent-offset: 4; indent-tabs-mode: nil; coding: utf-8; -*-
-
 """
-Copyright (c) 2014-2018,  Regents of the University of California,
+Copyright (c) 2014-2023,  Regents of the University of California,
                           Arizona Board of Regents,
                           Colorado State University,
                           University Pierre & Marie Curie, Sorbonne University,
@@ -25,88 +23,94 @@
 NFD, e.g., in COPYING.md file.  If not, see <http://www.gnu.org/licenses/>.
 """
 
-from http.server import HTTPServer, SimpleHTTPRequestHandler
-from socketserver import ThreadingMixIn
+from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer
 from urllib.parse import urlsplit
 import argparse, ipaddress, os, socket, subprocess
 
 
 class NfdStatusHandler(SimpleHTTPRequestHandler):
-    """ The handler class to handle requests """
+    """The handler class to handle HTTP requests."""
+
     def do_GET(self):
         path = urlsplit(self.path).path
         if path == "/":
-            self.__serveReport()
-        elif path == "/robots.txt" and self.server.allowRobots:
+            self.__serve_report()
+        elif path == "/robots.txt" and self.server.allow_robots:
             self.send_error(404)
         else:
             super().do_GET()
 
-    def __serveReport(self):
-        """ Obtain XML-formatted NFD status report and send it back as response body """
+    def __serve_report(self):
+        """Obtain XML-formatted NFD status report and send it back as response body."""
         try:
-            # enable universal_newlines to get output as string rather than byte sequence
-            output = subprocess.check_output(["nfdc", "status", "report", "xml"], universal_newlines=True)
+            proc = subprocess.run(
+                ["nfdc", "status", "report", "xml"], capture_output=True, check=True, text=True, timeout=10
+            )
+            output = proc.stdout
         except OSError as err:
-            super().log_message("error invoking nfdc: {}".format(err))
+            super().log_message(f"error invoking nfdc: {err}")
             self.send_error(500)
-        except subprocess.CalledProcessError as err:
-            super().log_message("error invoking nfdc: command exited with status {}".format(err.returncode))
-            self.send_error(504, "Cannot connect to NFD (code {})".format(err.returncode))
+        except subprocess.SubprocessError as err:
+            super().log_message(f"error invoking nfdc: {err}")
+            self.send_error(504, "Cannot connect to NFD")
         else:
             # add stylesheet processing instruction after the XML document type declaration
             # (yes, this is a ugly hack)
-            pos = output.index(">") + 1
-            xml = output[:pos]\
-                + '<?xml-stylesheet type="text/xsl" href="nfd-status.xsl"?>'\
-                + output[pos:]
-            self.send_response(200)
-            self.send_header("Content-Type", "text/xml; charset=UTF-8")
-            self.end_headers()
-            self.wfile.write(xml.encode())
+            if (pos := output.find(">") + 1) != 0:
+                xml = (
+                    output[:pos]
+                    + '<?xml-stylesheet type="text/xsl" href="nfd-status.xsl"?>'
+                    + output[pos:]
+                )
+                self.send_response(200)
+                self.send_header("Content-Type", "text/xml; charset=UTF-8")
+                self.end_headers()
+                self.wfile.write(xml.encode())
+            else:
+                super().log_message(f"malformed nfdc output: {output}")
+                self.send_error(500)
 
     # override
     def log_message(self, *args):
         if self.server.verbose:
             super().log_message(*args)
 
 
-class ThreadingHttpServer(ThreadingMixIn, HTTPServer):
-    """ Handle requests using threads """
-    def __init__(self, bindAddr, port, handler, allowRobots=False, verbose=False):
+class NfdStatusHttpServer(ThreadingHTTPServer):
+    def __init__(self, addr, port, *, robots=False, verbose=False):
         # socketserver.BaseServer defaults to AF_INET even if you provide an IPv6 address
         # see https://bugs.python.org/issue20215 and https://bugs.python.org/issue24209
-        if bindAddr.version == 6:
+        if addr.version == 6:
             self.address_family = socket.AF_INET6
-        self.allowRobots = allowRobots
+        self.allow_robots = robots
         self.verbose = verbose
-        super().__init__((str(bindAddr), port), handler)
+        super().__init__((str(addr), port), NfdStatusHandler)
 
 
 def main():
-    def ipAddress(arg):
-        """ Validate IP address """
+    def ip_address(arg, /):
+        """Validate IP address."""
         try:
             value = ipaddress.ip_address(arg)
         except ValueError:
-            raise argparse.ArgumentTypeError("{!r} is not a valid IP address".format(arg))
+            raise argparse.ArgumentTypeError(f"{arg!r} is not a valid IP address")
         return value
 
-    def portNumber(arg):
-        """ Validate port number """
+    def port_number(arg, /):
+        """Validate port number."""
         try:
             value = int(arg)
         except ValueError:
             value = -1
         if value < 0 or value > 65535:
-            raise argparse.ArgumentTypeError("{!r} is not a valid port number".format(arg))
+            raise argparse.ArgumentTypeError(f"{arg!r} is not a valid port number")
         return value
 
     parser = argparse.ArgumentParser(description="Serves NFD status page via HTTP")
     parser.add_argument("-V", "--version", action="version", version="@VERSION@")
-    parser.add_argument("-a", "--address", default="127.0.0.1", type=ipAddress, metavar="ADDR",
+    parser.add_argument("-a", "--address", default="127.0.0.1", type=ip_address, metavar="ADDR",
                         help="bind to this IP address (default: %(default)s)")
-    parser.add_argument("-p", "--port", default=8080, type=portNumber,
+    parser.add_argument("-p", "--port", default=8080, type=port_number,
                         help="bind to this port number (default: %(default)s)")
     parser.add_argument("-f", "--workdir", default="@DATAROOTDIR@/ndn", metavar="DIR",
                         help="server's working directory (default: %(default)s)")
@@ -118,20 +122,17 @@ def portNumber(arg):
 
     os.chdir(args.workdir)
 
-    httpd = ThreadingHttpServer(args.address, args.port, NfdStatusHandler,
-                                allowRobots=args.robots, verbose=args.verbose)
-
-    if httpd.address_family == socket.AF_INET6:
-        url = "http://[{}]:{}"
-    else:
-        url = "http://{}:{}"
-    print("Server started at", url.format(*httpd.server_address))
+    with NfdStatusHttpServer(args.address, args.port, robots=args.robots, verbose=args.verbose) as httpd:
+        if httpd.address_family == socket.AF_INET6:
+            url = "http://[{}]:{}"
+        else:
+            url = "http://{}:{}"
+        print("Server started at", url.format(*httpd.server_address))
 
-    try:
-        httpd.serve_forever()
-    except KeyboardInterrupt:
-        pass
-    httpd.server_close()
+        try:
+            httpd.serve_forever()
+        except KeyboardInterrupt:
+            pass
 
 
 if __name__ == "__main__":