#2687 - Include mimeo python module

modules/adblockplus/files/mimeo.py

 #!/usr/bin/env python3
 
 import argparse
-import logging
+import re
 import sys
+import threading
+import traceback
 
 from http.server import BaseHTTPRequestHandler, HTTPServer
 from string import Template
 
-PUNCTUATION = """!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~"""
-PROXY_TOKEN = 'http_'
+# The token used for the headers passed by nginx is: http_
+REGEX = r'\$http_\w+\b'
 DEFAULT_LOG = '$remote_addr - - [$time_local] "$request" $status $bytes_sent'
+_lock = threading.Lock()
 
 
 class Handler(BaseHTTPRequestHandler):
     def get_header_values(self):

[mathias, 2017/08/08 07:51:46]

Why all these string operations when a simple
`re.findall('\\$http_[a-z_0-9]+\\b', self.format)` should do the trick as well?

[Vasily Kuznetsov, 2017/08/08 11:30:51]

This particular regexp doesn't quite work, I think, because there's also
`${http_foo}` syntax, but you could do this with a regexp indeed. It would
probably be less code and more readable, so yeah, sounds like a good idea.

[f.lopez, 2017/08/09 19:50:47]

Done.

         values = {}
-        for var in self.log_format.split():
-            name = var.strip(PUNCTUATION)
-            if name.startswith(PROXY_TOKEN):
-                new_var = name[len(PROXY_TOKEN):].title()
-                values[name] = self.headers.get(new_var, '-')
+        headers = re.findall(REGEX, self.format)
+        for name in headers:
+            new_var = name[6:].replace('_', '-')
+            values[name[1:]] = self.headers.get(new_var, '-')
         return values
 
-    def log_info(self, args):

[mathias, 2017/08/08 07:51:46]

While the mimeo output looks like an HTTPd log, it is not logging from the
perspective of the mimeo script itself, but regular output. Hence this should
not be called log_info, but write_info or something. Analogous it should not use
the global resources from module logging, which are for logging from the script
perspective, but either a custom logger instance or just regular output.

[f.lopez, 2017/08/09 19:50:48]

Done.

-        message = Template(self.log_format)
-        logging.info(message.safe_substitute(args))
+    def send_simple_response(self, status, response=None):
+        self.send_response(status)
+        self.end_headers()
+        if response is None:
+            response = bytes(self.responses[status][0], 'UTF-8')
+        self.wfile.write(response)
+
+    def write_info(self, args):
+        message = Template(self.format).safe_substitute(args) + '\n'
+        with _lock:
+            self.output.write(message)
+            self.output.flush()
 
     def do_POST(self):
         status = 200
         content = bytes(self.response, 'UTF-8')
         values = {
             'remote_addr': self.address_string(),
             'time_local': self.log_date_time_string(),
             'request': self.requestline,
             'status': status,
             'bytes_sent': len(content),
-            'content_type': 'text/plain',
-            'content_length': len(content),
         }
         values.update(self.get_header_values())
-        self.log_info(values)
-        self.send_response(status)
-        self.send_header('Content-Type', values['content_type'])
-        self.send_header('Content-Length', values['content_length'])
-        self.end_headers()
-        self.wfile.write(content)
-        self.log_request(200)

[mathias, 2017/08/08 07:51:46]

Shouldn't this use the status variable from above instead of the magic number?

[f.lopez, 2017/08/09 19:50:47]

Done.

+        try:
+            self.write_info(values)
+            self.send_simple_response(status, content)
+        except:
+            traceback.print_exc(file=sys.stderr)
+            self.send_simple_response(500)
 
 
 if __name__ == '__main__':
     parser = argparse.ArgumentParser()
     parser.add_argument('--port', action='store',
                         default=8000, type=int,
                         nargs='?',
                         help='Port to use [default: 8000]')
     parser.add_argument('--response', action='store',
                         type=str, nargs='?', default='OK',
                         help='The response send to the client')
-    parser.add_argument('--log_format', action='store',
+    parser.add_argument('--format', action='store',
                         type=str, nargs='?',
                         default=DEFAULT_LOG,
                         help='Format of the log ouput')
-    parser.add_argument('log_file', action='store',
-                        type=str, nargs='?',
+    parser.add_argument('output', action='store',
+                        type=str, nargs='?', default='-',
                         help='The file where the logs will be written')
     args = parser.parse_args()
-    handler_class = Handler
-    server_class = HTTPServer

[mathias, 2017/08/08 07:51:45]

Why the aliasing, additional variables?

[f.lopez, 2017/08/09 19:50:47]

Done.

-    setattr(handler_class, 'log_format', args.log_format)

[mathias, 2017/08/08 07:51:45]

Analogous to log_info/write_info abobve, this property should be called
write_format or just format or similar. 

[f.lopez, 2017/08/09 19:50:47]

Done.

-    setattr(handler_class, 'response', args.response)

[mathias, 2017/08/08 07:51:45]

Why invocations of setattr() instead of assignments?

[f.lopez, 2017/08/09 19:50:48]

The HTTPServer (server_class) receives a server address and a
RequestHandlerClass, the Handler is not initialized since it only needs the
class not the object, and the `args` are out of the scope of the class, so the
only way I found to send the attributes to the class without initializing an
object was with setattr

[Vasily Kuznetsov, 2017/08/14 10:59:55]

I'm not sure what you mean here. Setting attributes on classes works fine and
produces the same effect as `setattr`. Here's an example from a Python REPL:

    >>> class A: pass
    ...
    >>> A.foo = 'bar'
    >>> A.foo
    'bar'
    >>> a = A()
    >>> a.foo
    'bar'

Or am I still missing something?

-    server_address = ('', args.port)
-    httpd = server_class(server_address, handler_class)
-    if args.log_file is None:
-        logging.basicConfig(stream=sys.stdout, level=logging.INFO)
+    if args.output and args.output != '-':
+        fh = open(args.output, 'a')
     else:
-        logging.basicConfig(filename=args.log_file, level=logging.INFO)
-    httpd.serve_forever()
+        fh = open(sys.stdout.fileno(), 'w', closefd=False)
+    try:
+        Handler.output = fh
+        Handler.format = args.format
+        Handler.response = args.response
+        server_address = ('', args.port)
+        httpd = HTTPServer(server_address, Handler)
+        httpd.serve_forever()
+    finally:
+        fh.close()