Check in a simple pure-python based Markdown previewer.

tools/md_browser/md_browser.py

Index: tools/md_browser/md_browser.py
diff --git a/tools/md_browser/md_browser.py b/tools/md_browser/md_browser.py
new file mode 100644
index 0000000000000000000000000000000000000000..58571f0ef75cd39f92561c9202b0e6c8070f6cb3
--- /dev/null
+++ b/tools/md_browser/md_browser.py
@@ -0,0 +1,101 @@
+# Copyright 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Simple Markdown browser for a Git checkout."""
+
+import SimpleHTTPServer
+import SocketServer
+import argparse
+import codecs
+import os
+import socket
+import sys
+
+
+SRC_DIR = os.path.dirname(os.path.dirname(
+    os.path.dirname(os.path.abspath(__file__))))
+sys.path.append(os.path.join(SRC_DIR, 'third_party', 'Python-Markdown'))
+import markdown
+
+
+def main(argv):
+    parser = argparse.ArgumentParser(prog='md_browser')

[scottmg, 2015/09/22 04:58:01]

+4 like an animal! :)

[Dirk Pranke, 2015/09/22 21:16:27]

will fix. stupid chromium coding standards :).

+    parser.add_argument('-p', '--port', type=int, default=8080,
+                        help='port to run on (default = %(default)s)')
+    args = parser.parse_args(argv)
+
+    try:
+        s = Server(args.port, SRC_DIR)
+        s.serve_forever()
+        s.shutdown()
+        return 0
+    except KeyboardInterrupt:
+        return 130
+
+
+class Server(SocketServer.TCPServer):
+    def __init__(self, port, top_level):
+        SocketServer.TCPServer.__init__(self, ('0.0.0.0', port), Handler)
+        self.port = port
+        self.top_level = top_level
+
+    def server_bind(self):
+        self.socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+        self.socket.bind(self.server_address)
+
+
+class Handler(SimpleHTTPServer.SimpleHTTPRequestHandler):
+    def do_GET(self):
+        full_path = os.path.abspath(os.path.join(self.server.top_level,
+                                                 self.path[1:]))

[scottmg, 2015/09/22 04:58:01]

I guess it doesn't matter for this usage, but you could maybe check for '..'s in
the path.

[nodir, 2015/09/22 16:38:18]

os.path.abspath returns a normalized path, so the startswith check below should
be enough

[scottmg, 2015/09/22 17:32:46]

(I meant that by entering localhost:8080/../../../../../../../../autoexec.bat
that you could read things that you sort of shouldn't be able to. But it also
probably doesn't matter here.)

[Dirk Pranke, 2015/09/22 21:16:27]

Right, but Nodir's saying (correctly, I believe) that abspath() will normalize
joined path, at which point the check on line 52 will stop you from looking at
files outside of the tree.

[scottmg, 2015/09/22 21:26:39]

Oh, you're right. Sorry.

+        if not full_path.startswith(self.server.top_level):
+            self._do_unknown()
+        elif self.path == '/doc.css':
+            self._write_file('doc.css')
+        elif not os.path.exists(full_path):
+            self._do_not_found()
+        elif self.path.endswith('.md'):

[nodir, 2015/09/22 16:38:18]

self.path.lower()... JIC

[Dirk Pranke, 2015/09/22 21:16:27]

Done, though I worry about what other sorts of weirdness this might let get
through.

+            self._do_md()
+        else:
+            self._do_unknown()
+
+    def _do_md(self):
+        extensions = [
+            'markdown.extensions.fenced_code',
+            'markdown.extensions.tables',
+            'markdown.extensions.toc',
+        ]
+
+        contents = self._read(self.path[1:])

[scottmg, 2015/09/22 04:58:01]

it seems like this is already stripping the leading /, but then _read also
checks for a leading '/'?

[Dirk Pranke, 2015/09/22 21:16:27]

Made things more consistent.

+        md_fragment = markdown.markdown(contents,
+                                        extensions=extensions,
+                                        output_format='html4').encode('utf-8')
+        try:
+            self._write_file('header.html')
+            self.wfile.write(md_fragment)
+            self._write_file('footer.html')
+        except:
+            raise
+
+    def _do_not_found(self):
+        self.wfile.write('<html><body>%s not found</body></html>' % self.path)
+
+    def _do_unknown(self):
+        self.wfile.write('<html><body>I do not know how to serve %s.</body>'
+                         '</html>' % self.path)
+
+    def _read(self, path):
+        if not path.startswith('/'):

[nodir, 2015/09/22 16:38:18]

this if is unnecessary: os.path.join('/foo', '/bar') == '/bar'

[Dirk Pranke, 2015/09/22 21:16:27]

true, but the code was confusing regardless. Made things more consistent here as
well.

+            path = os.path.join(self.server.top_level, path)
+        return codecs.open(path, mode='r', encoding='utf-8').read()

[nodir, 2015/09/22 16:38:18]

mode='r' is unnecessary, it is default

[nodir, 2015/09/22 16:38:18]

This seems to leave the fd open?
Shouldn't you

with codes.open(....) as f:
  return f.read()

?

[Dirk Pranke, 2015/09/22 21:16:27]

Done, and good catch on the fd.

+
+    def _write_file(self, path):
+        assert os.sep not in path
+        contents = self._read(os.path.join(SRC_DIR, 'tools', 'md_browser',

[nodir, 2015/09/22 16:38:18]

nit: I think moving the line break to the position after self._read( would be
more readable

[Dirk Pranke, 2015/09/22 21:16:27]

Once I switch to 2-space indents the whole thing fits on one line.

+                                           path))
+        self.wfile.write(contents.encode('utf-8'))
+
+
+if __name__ == '__main__':
+    sys.exit(main(sys.argv[1:]))