From 74ab58167ea338f0c114814ccdbd80032d886606 Mon Sep 17 00:00:00 2001 From: Martin Zimmermann Date: Fri, 1 Nov 2013 12:28:37 +0100 Subject: [PATCH] replace abort(code) with proper exceptions and refactor request dispatch --- isso/__init__.py | 70 ++++++++++++++++++++---------------------- isso/utils/__init__.py | 13 ++++++++ isso/views/comment.py | 39 +++++++++-------------- 3 files changed, 62 insertions(+), 60 deletions(-) diff --git a/isso/__init__.py b/isso/__init__.py index acebb19..229797d 100644 --- a/isso/__init__.py +++ b/isso/__init__.py @@ -32,7 +32,6 @@ dist = pkg_resources.get_distribution("isso") import sys import os -import socket import logging from os.path import dirname, join @@ -47,8 +46,7 @@ import misaka from itsdangerous import URLSafeTimedSerializer from werkzeug.routing import Map, Rule -from werkzeug.wrappers import Response, Request -from werkzeug.exceptions import HTTPException, NotFound, MethodNotAllowed +from werkzeug.exceptions import HTTPException, InternalServerError from werkzeug.wsgi import SharedDataMiddleware from werkzeug.serving import run_simple @@ -56,7 +54,7 @@ from werkzeug.contrib.fixers import ProxyFix from isso import db, migrate, views, wsgi from isso.core import ThreadedMixin, uWSGIMixin, Config -from isso.utils import parse, http +from isso.utils import parse, http, JSONRequest from isso.views import comment logging.getLogger('werkzeug').setLevel(logging.ERROR) @@ -67,25 +65,23 @@ logging.basicConfig( logger = logging.getLogger("isso") -rules = Map([ - Rule('/new', methods=['POST'], endpoint=views.comment.new), - - Rule('/id/', methods=['GET', 'PUT', 'DELETE'], endpoint=views.comment.single), - Rule('/id//like', methods=['POST'], endpoint=views.comment.like), - Rule('/id//dislike', methods=['POST'], endpoint=views.comment.dislike), - - Rule('/', methods=['GET'], endpoint=views.comment.fetch), - Rule('/count', methods=['GET'], endpoint=views.comment.count), - Rule('/delete/', endpoint=views.comment.delete), - Rule('/activate/', endpoint=views.comment.activate), - - Rule('/check-ip', endpoint=views.comment.checkip) -]) - - class Isso(object): salt = b"Eech7co8Ohloopo9Ol6baimi" + urls = Map([ + Rule('/new', methods=['POST'], endpoint=views.comment.new), + + Rule('/id/', methods=['GET', 'PUT', 'DELETE'], endpoint=views.comment.single), + Rule('/id//like', methods=['POST'], endpoint=views.comment.like), + Rule('/id//dislike', methods=['POST'], endpoint=views.comment.dislike), + + Rule('/', methods=['GET'], endpoint=views.comment.fetch), + Rule('/count', methods=['GET'], endpoint=views.comment.count), + Rule('/delete/', endpoint=views.comment.delete), + Rule('/activate/', endpoint=views.comment.activate), + + Rule('/check-ip', endpoint=views.comment.checkip) + ]) def __init__(self, conf): @@ -102,30 +98,27 @@ class Isso(object): return self.signer.loads(obj, max_age=max_age or self.conf.getint('general', 'max-age')) def markdown(self, text): - return misaka.html(text, extensions=misaka.EXT_STRIKETHROUGH \ - | misaka.EXT_SUPERSCRIPT | misaka.EXT_AUTOLINK \ + return misaka.html(text, extensions=misaka.EXT_STRIKETHROUGH + | misaka.EXT_SUPERSCRIPT | misaka.EXT_AUTOLINK | misaka.HTML_SKIP_HTML | misaka.HTML_SKIP_IMAGES | misaka.HTML_SAFELINK) - def dispatch(self, request, start_response): - adapter = rules.bind_to_environ(request.environ) + def dispatch(self, request): + adapter = Isso.urls.bind_to_environ(request.environ) try: handler, values = adapter.match() - return handler(self, request.environ, request, **values) - except NotFound: - return Response('Not Found', 404) - except MethodNotAllowed: - return Response("Yup.", 200) except HTTPException as e: return e + else: + try: + response = handler(self, request.environ, request, **values) + except HTTPException as e: + return e + except Exception: + logger.exception("%s %s", request.method, request.environ["PATH_INFO"]) + return InternalServerError() - def wsgi_app(self, environ, start_response): - - response = self.dispatch(Request(environ), start_response) - - # add CORS header - if hasattr(response, 'headers') and 'HTTP_ORIGN' in environ: for host in self.conf.getiter('general', 'host'): - if environ["HTTP_ORIGIN"] == host.rstrip("/"): + if request.environ.get("HTTP_ORIGIN", None) == host.rstrip("/"): origin = host.rstrip("/") break else: @@ -137,6 +130,11 @@ class Isso(object): hdrs["Access-Control-Allow-Credentials"] = "true" hdrs["Access-Control-Allow-Methods"] = "GET, POST, PUT, DELETE" + return response + + def wsgi_app(self, environ, start_response): + + response = self.dispatch(JSONRequest(environ)) return response(environ, start_response) def __call__(self, environ, start_response): diff --git a/isso/utils/__init__.py b/isso/utils/__init__.py index 12216e7..09af650 100644 --- a/isso/utils/__init__.py +++ b/isso/utils/__init__.py @@ -2,11 +2,15 @@ from __future__ import division +import json import random import hashlib from string import ascii_letters, digits +from werkzeug.wrappers import Request +from werkzeug.exceptions import BadRequest + import ipaddress @@ -83,3 +87,12 @@ class Bloomfilter: def __len__(self): return self.elements + + +class JSONRequest(Request): + + def get_json(self): + try: + return json.loads(self.get_data().decode('utf-8')) + except ValueError: + raise BadRequest('Unable to read JSON request') diff --git a/isso/views/comment.py b/isso/views/comment.py index 3bcbf56..939948d 100644 --- a/isso/views/comment.py +++ b/isso/views/comment.py @@ -10,7 +10,7 @@ import sqlite3 from itsdangerous import SignatureExpired, BadSignature from werkzeug.wrappers import Response -from werkzeug.exceptions import abort, BadRequest +from werkzeug.exceptions import BadRequest, Forbidden, NotFound from isso.compat import text_type as str @@ -49,19 +49,16 @@ class requires: @requires(str, 'uri') def new(app, environ, request, uri): - try: - data = json.loads(request.get_data().decode('utf-8')) - except ValueError: - return Response("No JSON object could be decoded", 400) + data = request.get_json() for field in set(data.keys()) - set(['text', 'author', 'website', 'email', 'parent']): data.pop(field) if not data.get("text"): - return Response("No text given.", 400) + raise BadRequest("no text given") if "id" in data and not isinstance(data["id"], int): - return Response("Parent ID must be an integer.") + raise BadRequest("parent id must be an integer") for field in ("author", "email"): if data.get(field): @@ -88,11 +85,8 @@ def new(app, environ, request, uri): try: with app.lock: rv = app.db.comments.add(uri, data) - except sqlite3.Error: - logging.exception('uncaught SQLite3 exception') - abort(400) except db.IssoDBException: - abort(403) + raise Forbidden host = list(app.conf.getiter('general', 'host'))[0].rstrip("/") href = host + uri + "#isso-%i" % rv["id"] @@ -130,7 +124,7 @@ def single(app, environ, request, id): if request.method == 'GET': rv = app.db.comments.get(id) if rv is None: - abort(404) + raise NotFound for key in set(rv.keys()) - FIELDS: rv.pop(key) @@ -146,23 +140,20 @@ def single(app, environ, request, id): try: rv = app.unsign(request.cookies.get('admin', '')) except (SignatureExpired, BadSignature): - abort(403) + raise Forbidden if rv[0] != id: - abort(403) + raise Forbidden # verify checksum, mallory might skip cookie deletion when he deletes a comment if rv[1] != hashlib.md5(app.db.comments.get(id)["text"].encode('utf-8')).hexdigest(): - abort(403) + raise Forbidden if request.method == 'PUT': - try: - data = json.loads(request.get_data().decode('utf-8')) - except ValueError: - return Response("No JSON object could be decoded", 400) + data = request.get_json() if data.get("text") is not None and len(data['text']) < 3: - return Response("No text given.", 400) + raise BadRequest("no text given") for key in set(data.keys()) - set(["text", "author", "website"]): data.pop(key) @@ -203,7 +194,7 @@ def fetch(app, environ, request, uri): rv = list(app.db.comments.fetch(uri)) if not rv: - abort(404) + raise NotFound for item in rv: @@ -237,7 +228,7 @@ def count(app, environ, request, uri): rv = app.db.comments.count(uri)[0] if rv == 0: - abort(404) + raise NotFound return Response(json.dumps(rv), 200, content_type='application/json') @@ -247,7 +238,7 @@ def activate(app, environ, request, auth): try: id = app.unsign(auth, max_age=2**32) except (BadSignature, SignatureExpired): - abort(403) + raise Forbidden with app.lock: app.db.comments.activate(id) @@ -260,7 +251,7 @@ def delete(app, environ, request, auth): try: id = app.unsign(auth, max_age=2**32) except (BadSignature, SignatureExpired): - abort(403) + raise Forbidden with app.lock: app.db.comments.delete(id)