hgweb: return meaningful HTTP status codes instead of nonsense

tests/test-hgwebdir

@@ -0,0 +1,55
+#!/bin/sh
+
+mkdir webdir
+cd webdir
+
+hg init a
+echo a > a/a
+hg --cwd a ci -Ama -d'1 0'
+
+hg init b
+echo b > b/b
+hg --cwd b ci -Amb -d'2 0'
+
+hg init c
+echo c > c/c
+hg --cwd c ci -Amc -d'3 0'
+root=`pwd`
+
+cd ..
+
+cat > paths.conf <<EOF
+[paths]
+a=$root/a
+b=$root/b
+EOF
+
+hg serve -p $HGPORT -d --pid-file=hg.pid --webdir-conf paths.conf \
+    -A access-paths.log -E error-paths.log
+cat hg.pid >> $DAEMON_PIDS
+
+echo % should give a 404 - file does not exist
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT '/a/file/tip/bork?style=raw'
+
+echo % should succeed
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT '/?style=raw'
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT '/a/file/tip/a?style=raw'
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT '/b/file/tip/b?style=raw'
+
+echo % should give a 404 - repo is not published
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT '/c/file/tip/c?style=raw'
+
+cat > collections.conf <<EOF
+[collections]
+$root=$root
+EOF
+
+hg serve -p $HGPORT1 -d --pid-file=hg.pid --webdir-conf collections.conf \
+    -A access-collections.log -E error-collections.log
+cat hg.pid >> $DAEMON_PIDS
+
+echo % should succeed
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT1 '/?style=raw'
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT1 '/a/file/tip/a?style=raw'
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT1 '/b/file/tip/b?style=raw'
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT1 '/c/file/tip/c?style=raw'

tests/test-hgwebdir.out

@@ -0,0 +1,43
+adding a
+adding b
+adding c
+% should give a 404 - file does not exist
+404 Not Found
+
+
+error: Path not found: bork/
+% should succeed
+200 Script output follows
+
+
+/a/
+/b/
+
+200 Script output follows
+
+a
+200 Script output follows
+
+b
+% should give a 404 - repo is not published
+404 Not Found
+
+
+error: repository c not found
+% should succeed
+200 Script output follows
+
+
+/a/
+/b/
+/c/
+
+200 Script output follows
+
+a
+200 Script output follows
+
+b
+200 Script output follows
+
+c

mercurial/hgweb/common.py

@@ -6,7 +6,17
 # This software may be used and distributed according to the terms
 # of the GNU General Public License, incorporated herein by reference.
 
-import os, mimetypes
+import errno, mimetypes, os
+
+class ErrorResponse(Exception):
+    def __init__(self, code, message=None):
+        Exception.__init__(self)
+        self.code = code
+        if message is None:
+            from httplib import responses
+            self.message = responses.get(code, 'Error')
+        else:
+            self.message = message
 
 def get_mtime(repo_path):
     store_path = os.path.join(repo_path, ".hg")
@@ -40,9 +50,13 def staticfile(directory, fname, req):
         req.header([('Content-type', ct),
                     ('Content-length', str(os.path.getsize(path)))])
         return file(path, 'rb').read()
-    except (TypeError, OSError):
-        # illegal fname or unreadable file
-        return ""
+    except TypeError:
+        raise ErrorResponse(500, 'illegal file name')
+    except OSError, err:
+        if err.errno == errno.ENOENT:
+            raise ErrorResponse(404)
+        else:
+            raise ErrorResponse(500, err.strerror)
 
 def style_map(templatepath, style):
     """Return path to mapfile for a given style.

mercurial/hgweb/hgweb_mod.py

@@ -6,13 +6,13
 # This software may be used and distributed according to the terms
 # of the GNU General Public License, incorporated herein by reference.
 
-import os, mimetypes, re, zlib, mimetools, cStringIO, sys
+import errno, os, mimetypes, re, zlib, mimetools, cStringIO, sys
 import tempfile, urllib, bz2
 from mercurial.node import *
 from mercurial.i18n import gettext as _
 from mercurial import mdiff, ui, hg, util, archival, streamclone, patch
 from mercurial import revlog, templater
-from common import get_mtime, staticfile, style_map, paritygen
+from common import ErrorResponse, get_mtime, staticfile, style_map, paritygen
 
 def _up(p):
     if p[0] != "/":
@@ -478,6 +478,9 class hgweb(object):
                 short = os.path.basename(remain)
                 files[short] = (f, n)
 
+        if not files:
+            raise ErrorResponse(404, 'Path not found: ' + path)
+
         def filelist(**map):
             fl = files.keys()
             fl.sort()
@@ -845,14 +848,20 class hgweb(object):
 
             cmd = req.form['cmd'][0]
 
-            method = getattr(self, 'do_' + cmd, None)
-            if method:
-                try:
-                    method(req)
-                except (hg.RepoError, revlog.RevlogError), inst:
-                    req.write(self.t("error", error=str(inst)))
-            else:
-                req.write(self.t("error", error='No such method: ' + cmd))
+            try:
+                method = getattr(self, 'do_' + cmd)
+                method(req)
+            except revlog.LookupError, err:
+                req.respond(404, self.t(
+                    'error', error='revision not found: %s' % err.name))
+            except (hg.RepoError, revlog.RevlogError), inst:
+                req.respond('500 Internal Server Error',
+                            self.t('error', error=str(inst)))
+            except ErrorResponse, inst:
+                req.respond(inst.code, self.t('error', error=inst.message))
+            except AttributeError:
+                req.respond(400,
+                            self.t('error', error='No such method: ' + cmd))
         finally:
             self.t = None
 
@@ -1038,7 +1047,8 class hgweb(object):
             self.archive(req, req.form['node'][0], type_)
             return
 
-        req.write(self.t("error"))
+        req.respond(400, self.t('error',
+                                error='Unsupported archive type: %s' % type_))
 
     def do_static(self, req):
         fname = req.form['file'][0]
@@ -1047,8 +1057,7 class hgweb(object):
         static = self.config("web", "static",
                              os.path.join(self.templatepath, "static"),
                              untrusted=False)
-        req.write(staticfile(static, fname, req)
-                  or self.t("error", error="%r not found" % fname))
+        req.write(staticfile(static, fname, req))
 
     def do_capabilities(self, req):
         caps = ['lookup', 'changegroupsubset']
@@ -1198,7 +1207,11 class hgweb(object):
                 else:
                     filename = ''
                 error = getattr(inst, 'strerror', 'Unknown error')
-                req.write('%s: %s\n' % (error, filename))
+                if inst.errno == errno.ENOENT:
+                    code = 404
+                else:
+                    code = 500
+                req.respond(code, '%s: %s\n' % (error, filename))
         finally:
             fp.close()
             os.unlink(tempname)

mercurial/hgweb/hgwebdir_mod.py

@@ -9,7 +9,7
 import os, mimetools, cStringIO
 from mercurial.i18n import gettext as _
 from mercurial import ui, hg, util, templater
-from common import get_mtime, staticfile, style_map, paritygen
+from common import ErrorResponse, get_mtime, staticfile, style_map, paritygen
 from hgweb_mod import hgweb
 
 # This is a stopgap
@@ -215,46 +215,47 class hgwebdir(object):
                            **dict(sort)))
 
         try:
-            virtual = req.env.get("PATH_INFO", "").strip('/')
-            if virtual.startswith('static/'):
-                static = os.path.join(templater.templatepath(), 'static')
-                fname = virtual[7:]
-                req.write(staticfile(static, fname, req) or
-                          tmpl('error', error='%r not found' % fname))
-            elif virtual:
-                repos = dict(self.repos)
-                while virtual:
-                    real = repos.get(virtual)
-                    if real:
-                        req.env['REPO_NAME'] = virtual
-                        try:
-                            repo = hg.repository(parentui, real)
-                            hgweb(repo).run_wsgi(req)
-                        except IOError, inst:
-                            req.write(tmpl("error", error=inst.strerror))
-                        except hg.RepoError, inst:
-                            req.write(tmpl("error", error=str(inst)))
-                        return
+            try:
+                virtual = req.env.get("PATH_INFO", "").strip('/')
+                if virtual.startswith('static/'):
+                    static = os.path.join(templater.templatepath(), 'static')
+                    fname = virtual[7:]
+                    req.write(staticfile(static, fname, req))
+                elif virtual:
+                    repos = dict(self.repos)
+                    while virtual:
+                        real = repos.get(virtual)
+                        if real:
+                            req.env['REPO_NAME'] = virtual
+                            try:
+                                repo = hg.repository(parentui, real)
+                                hgweb(repo).run_wsgi(req)
+                                return
+                            except IOError, inst:
+                                raise ErrorResponse(500, inst.strerror)
+                            except hg.RepoError, inst:
+                                raise ErrorResponse(500, str(inst))
 
-                    # browse subdirectories
-                    subdir = virtual + '/'
-                    if [r for r in repos if r.startswith(subdir)]:
-                        makeindex(req, subdir)
-                        return
+                        # browse subdirectories
+                        subdir = virtual + '/'
+                        if [r for r in repos if r.startswith(subdir)]:
+                            makeindex(req, subdir)
+                            return
+
+                        up = virtual.rfind('/')
+                        if up < 0:
+                            break
+                        virtual = virtual[:up]
 
-                    up = virtual.rfind('/')
-                    if up < 0:
-                        break
-                    virtual = virtual[:up]
-
-                req.write(tmpl("notfound", repo=virtual))
-            else:
-                if req.form.has_key('static'):
-                    static = os.path.join(templater.templatepath(), "static")
-                    fname = req.form['static'][0]
-                    req.write(staticfile(static, fname, req)
-                              or tmpl("error", error="%r not found" % fname))
+                    req.respond(404, tmpl("notfound", repo=virtual))
                 else:
-                    makeindex(req)
+                    if req.form.has_key('static'):
+                        static = os.path.join(templater.templatepath(), "static")
+                        fname = req.form['static'][0]
+                        req.write(staticfile(static, fname, req))
+                    else:
+                        makeindex(req)
+            except ErrorResponse, err:
+                req.respond(err.code, tmpl('error', error=err.message or ''))
         finally:
             tmpl = None

mercurial/hgweb/request.py

@@ -8,6 +8,7
 
 import socket, cgi, errno
 from mercurial.i18n import gettext as _
+from common import ErrorResponse
 
 class wsgiapplication(object):
     def __init__(self, destmaker):
@@ -42,25 +43,37 class _wsgirequest(object):
     def read(self, count=-1):
         return self.inp.read(count)
 
-    def write(self, *things):
+    def respond(self, status, *things):
         for thing in things:
             if hasattr(thing, "__iter__"):
                 for part in thing:
-                    self.write(part)
+                    self.respond(status, part)
             else:
                 thing = str(thing)
                 if self.server_write is None:
                     if not self.headers:
                         raise RuntimeError("request.write called before headers sent (%s)." % thing)
-                    self.server_write = self.start_response('200 Script output follows',
+                    code = None
+                    if isinstance(status, ErrorResponse):
+                        code = status.code
+                    elif isinstance(status, int):
+                        code = status
+                    if code:
+                        from httplib import responses
+                        status = '%d %s' % (
+                            code, responses.get(code, 'Error'))
+                    self.server_write = self.start_response(status,
                                                             self.headers)
                     self.start_response = None
-                    self.headers = None
+                    self.headers = []
                 try:
                     self.server_write(thing)
                 except socket.error, inst:
                     if inst[0] != errno.ECONNRESET:
                         raise
+        
+    def write(self, *things):
+        self.respond('200 Script output follows', *things)
 
     def writelines(self, lines):
         for line in lines:

tests/get-with-headers.py

@@ -14,3 +14,7 for h in headers:
         print "%s: %s" % (h, response.getheader(h))
 print
 sys.stdout.write(response.read())
+
+if 200 <= response.status <= 299:
+    sys.exit(0)
+sys.exit(1)

tests/test-hgweb

@@ -7,7 +7,25 echo foo > da/foo
 echo foo > foo
 hg ci -Ambase -d '0 0'
 hg serve -p $HGPORT -d --pid-file=hg.pid
+cat hg.pid >> $DAEMON_PIDS
 echo % manifest
 ("$TESTDIR/get-with-headers.py" localhost:$HGPORT '/file/tip/?style=raw')
 ("$TESTDIR/get-with-headers.py" localhost:$HGPORT '/file/tip/da?style=raw')
-kill `cat hg.pid`
+
+echo % plain file
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT '/file/tip/foo?style=raw'
+
+echo % should give a 404 - static file that does not exist
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT '/static/bogus'
+
+echo % should give a 404 - bad revision
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT '/file/spam/foo?style=raw'
+
+echo % should give a 400 - bad command
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT '/file/tip/foo?cmd=spam&style=raw'
+
+echo % should give a 404 - file does not exist
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT '/file/tip/bork?style=raw'
+
+echo % static file
+"$TESTDIR/get-with-headers.py" localhost:$HGPORT '/static/style-gitweb.css'

tests/test-hgweb.out

@@ -14,3 +14,123 200 Script output follows
 -rw-r--r-- 4 foo
 
 
+% plain file
+200 Script output follows
+
+foo
+% should give a 404 - static file that does not exist
+404 Not Found
+
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
+<html>
+<head>
+<link rel="icon" href="/static/hgicon.png" type="image/png">
+<meta name="robots" content="index, nofollow" />
+<link rel="stylesheet" href="/static/style.css" type="text/css" />
+
+<title>Mercurial Error</title>
+</head>
+<body>
+
+<h2>Mercurial Error</h2>
+
+<p>
+An error occured while processing your request:
+</p>
+<p>
+Not Found
+</p>
+
+
+<div class="logo">
+powered by<br/>
+<a href="http://www.selenic.com/mercurial/">mercurial</a>
+</div>
+
+</body>
+</html>
+
+% should give a 404 - bad revision
+404 Not Found
+
+
+error: revision not found: spam
+% should give a 400 - bad command
+400 Bad Request
+
+
+error: No such method: spam
+% should give a 404 - file does not exist
+404 Not Found
+
+
+error: Path not found: bork/
+% static file
+200 Script output follows
+
+body { font-family: sans-serif; font-size: 12px; margin:0px; border:solid #d9d8d1; border-width:1px; margin:10px; }
+a { color:#0000cc; }
+a:hover, a:visited, a:active { color:#880000; }
+div.page_header { height:25px; padding:8px; font-size:18px; font-weight:bold; background-color:#d9d8d1; }
+div.page_header a:visited { color:#0000cc; }
+div.page_header a:hover { color:#880000; }
+div.page_nav { padding:8px; }
+div.page_nav a:visited { color:#0000cc; }
+div.page_path { padding:8px; border:solid #d9d8d1; border-width:0px 0px 1px}
+div.page_footer { padding:4px 8px; background-color: #d9d8d1; }
+div.page_footer_text { float:left; color:#555555; font-style:italic; }
+div.page_body { padding:8px; }
+div.title, a.title {
+	display:block; padding:6px 8px;
+	font-weight:bold; background-color:#edece6; text-decoration:none; color:#000000;
+}
+a.title:hover { background-color: #d9d8d1; }
+div.title_text { padding:6px 0px; border: solid #d9d8d1; border-width:0px 0px 1px; }
+div.log_body { padding:8px 8px 8px 150px; }
+.age { white-space:nowrap; }
+span.age { position:relative; float:left; width:142px; font-style:italic; }
+div.log_link {
+	padding:0px 8px;
+	font-size:10px; font-family:sans-serif; font-style:normal;
+	position:relative; float:left; width:136px;
+}
+div.list_head { padding:6px 8px 4px; border:solid #d9d8d1; border-width:1px 0px 0px; font-style:italic; }
+a.list { text-decoration:none; color:#000000; }
+a.list:hover { text-decoration:underline; color:#880000; }
+table { padding:8px 4px; }
+th { padding:2px 5px; font-size:12px; text-align:left; }
+tr.light:hover, .parity0:hover { background-color:#edece6; }
+tr.dark, .parity1 { background-color:#f6f6f0; }
+tr.dark:hover, .parity1:hover { background-color:#edece6; }
+td { padding:2px 5px; font-size:12px; vertical-align:top; }
+td.link { padding:2px 5px; font-family:sans-serif; font-size:10px; }
+div.pre { font-family:monospace; font-size:12px; white-space:pre; }
+div.diff_info { font-family:monospace; color:#000099; background-color:#edece6; font-style:italic; }
+div.index_include { border:solid #d9d8d1; border-width:0px 0px 1px; padding:12px 8px; }
+div.search { margin:4px 8px; position:absolute; top:56px; right:12px }
+.linenr { color:#999999; text-decoration:none }
+a.rss_logo {
+	float:right; padding:3px 0px; width:35px; line-height:10px;
+	border:1px solid; border-color:#fcc7a5 #7d3302 #3e1a01 #ff954e;
+	color:#ffffff; background-color:#ff6600;
+	font-weight:bold; font-family:sans-serif; font-size:10px;
+	text-align:center; text-decoration:none;
+}
+a.rss_logo:hover { background-color:#ee5500; }
+pre { margin: 0; }
+span.logtags span {
+	padding: 0px 4px;
+	font-size: 10px;
+	font-weight: normal;
+	border: 1px solid;
+	background-color: #ffaaff;
+	border-color: #ffccff #ff00ee #ff00ee #ffccff;
+}
+span.logtags span.tagtag {
+	background-color: #ffffaa;
+	border-color: #ffffcc #ffee00 #ffee00 #ffffcc;
+}
+span.logtags span.branchtag {
+	background-color: #aaffaa;
+	border-color: #ccffcc #00cc33 #00cc33 #ccffcc;
+}