hgweb: raise ErrorResponses to communicate protocol errors

mercurial/hgweb/common.py

@@ -10,7 +10,9 b' import errno, mimetypes, os'
 
 HTTP_OK = 200
 HTTP_BAD_REQUEST = 400
+HTTP_UNAUTHORIZED = 401
 HTTP_NOT_FOUND = 404
+HTTP_METHOD_NOT_ALLOWED = 405
 HTTP_SERVER_ERROR = 500
 
 class ErrorResponse(Exception):

mercurial/hgweb/hgweb_mod.py

@@ -13,6 +13,7 b' from mercurial import mdiff, ui, hg, uti'
 from mercurial import revlog, templater, templatefilters
 from common import get_mtime, style_map, paritygen, countgen, ErrorResponse
 from common import HTTP_OK, HTTP_BAD_REQUEST, HTTP_NOT_FOUND, HTTP_SERVER_ERROR
+from common import HTTP_UNAUTHORIZED, HTTP_METHOD_NOT_ALLOWED
 from request import wsgirequest
 import webcommands, protocol, webutil
 
@@ -88,10 +89,16 b' class hgweb(object):'
 
         cmd = req.form.get('cmd', [''])[0]
         if cmd and cmd in protocol.__all__:
-            if cmd in perms and not self.check_perm(req, perms[cmd]):
-                return []
-            method = getattr(protocol, cmd)
-            return method(self.repo, req)
+            try:
+                if cmd in perms:
+                    self.check_perm(req, perms[cmd])
+                method = getattr(protocol, cmd)
+                return method(self.repo, req)
+            except ErrorResponse, inst:
+                req.respond(inst.code, protocol.HGTYPE)
+                if not inst.message:
+                    return []
+                return '0\n%s\n' % inst.message,
 
         # work with CGI variables to create coherent structure
         # use SCRIPT_NAME, PATH_INFO and QUERY_STRING as well as our REPO_NAME
@@ -344,35 +351,29 b' class hgweb(object):'
         '''Check permission for operation based on request data (including
         authentication info. Return true if op allowed, else false.'''
 
-        def error(status, message):
-            req.respond(status, protocol.HGTYPE)
-            req.write('0\n%s\n' % message)
-
-        if op == 'pull':
-            return self.allowpull
+        if op == 'pull' and not self.allowpull:
+            raise ErrorResponse(HTTP_OK, '')
+        elif op == 'pull':
+            return
 
         # enforce that you can only push using POST requests
         if req.env['REQUEST_METHOD'] != 'POST':
-            error('405 Method Not Allowed', 'push requires POST request')
-            return False
+            msg = 'push requires POST request'
+            raise ErrorResponse(HTTP_METHOD_NOT_ALLOWED, msg)
 
         # require ssl by default for pushing, auth info cannot be sniffed
         # and replayed
         scheme = req.env.get('wsgi.url_scheme')
         if self.configbool('web', 'push_ssl', True) and scheme != 'https':
-            error(HTTP_OK, 'ssl required')
-            return False
+            raise ErrorResponse(HTTP_OK, 'ssl required')
 
         user = req.env.get('REMOTE_USER')
 
         deny = self.configlist('web', 'deny_push')
         if deny and (not user or deny == ['*'] or user in deny):
-            error('401 Unauthorized', 'push not authorized')
-            return False
+            raise ErrorResponse(HTTP_UNAUTHORIZED, 'push not authorized')
 
         allow = self.configlist('web', 'allow_push')
         result = allow and (allow == ['*'] or user in allow)
         if not result:
-            error('401 Unauthorized', 'push not authorized')
-
-        return result
+            raise ErrorResponse(HTTP_UNAUTHORIZED, 'push not authorized')

mercurial/hgweb/protocol.py

@@ -108,7 +108,6 b' def capabilities(repo, req):'
 
 def unbundle(repo, req):
 
-    errorfmt = '0\n%s\n'
     proto = req.env.get('wsgi.url_scheme') or 'http'
     their_heads = req.form['heads'][0].split(' ')
 
@@ -123,10 +122,7 b' def unbundle(repo, req):'
             # drain incoming bundle, else client will not see
             # response when run outside cgi script
             pass
-        req.respond(HTTP_OK, HGTYPE)
-        return errorfmt % 'unsynced changes',
-
-    req.respond(HTTP_OK, HGTYPE)
+        raise ErrorResponse(HTTP_OK, 'unsynced changes')
 
     # do not lock repo until all changegroup data is
     # streamed. save to temporary file.
@@ -142,7 +138,7 b' def unbundle(repo, req):'
             lock = repo.lock()
             try:
                 if not check_heads():
-                    return errorfmt % 'unsynced changes',
+                    raise ErrorResponse(HTTP_OK, 'unsynced changes')
 
                 fp.seek(0)
                 header = fp.read(6)
@@ -168,11 +164,12 b' def unbundle(repo, req):'
                 finally:
                     val = sys.stdout.getvalue()
                     sys.stdout, sys.stderr = oldio
+                req.respond(HTTP_OK, HGTYPE)
                 return '%d\n%s' % (ret, val),
             finally:
                 del lock
         except ValueError, inst:
-            return errorfmt % inst,
+            raise ErrorResponse(HTTP_OK, inst)
         except (OSError, IOError), inst:
             filename = getattr(inst, 'filename', '')
             # Don't send our filesystem layout to the client
@@ -185,8 +182,7 b' def unbundle(repo, req):'
                 code = HTTP_NOT_FOUND
             else:
                 code = HTTP_SERVER_ERROR
-            req.respond(code)
-            return '0\n%s: %s\n' % (error, filename),
+            raise ErrorResponse(code, '%s: %s' % (error, filename))
     finally:
         fp.close()
         os.unlink(tempname)