From e33dab371c0c3f9a368387702aa39e75a89b6c7a Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Sat, 4 Jan 2025 19:45:05 +0530 Subject: [PATCH] Use custom converter for catch-all index path #3045 fixed the 404 errors for non-existent urls but the implementation masks CORS issues with other endpoints. The catch-all index_pages endpoint , by default, has provide_automatic_options enabled and as a result it matches OPTIONS requests for non-existent urls or any other endpoints that have CORS incompletely configured. This makes it harder to debug CORS issues. To avoid this instead of throwing a 404 after matching, we need to throw the error during matching itself. Hence, use a custom path converter that matches all paths except those starting with the api prefix. --- listenbrainz/webserver/__init__.py | 3 +++ listenbrainz/webserver/converters.py | 10 ++++++++++ listenbrainz/webserver/views/index.py | 6 ++---- 3 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 listenbrainz/webserver/converters.py diff --git a/listenbrainz/webserver/__init__.py b/listenbrainz/webserver/__init__.py index 789bc63b01..762ec95830 100644 --- a/listenbrainz/webserver/__init__.py +++ b/listenbrainz/webserver/__init__.py @@ -13,6 +13,7 @@ from listenbrainz import db from listenbrainz.db import create_test_database_connect_strings, timescale, donation from listenbrainz.db.timescale import create_test_timescale_connect_strings +from listenbrainz.webserver.converters import NotApiPathConverter API_PREFIX = '/1' @@ -108,6 +109,8 @@ def create_app(debug=None): logger = logging.getLogger('listenbrainz') logger.setLevel(logging.DEBUG) + app.url_map.converters["not_api_path"] = NotApiPathConverter + # initialize Flask-DebugToolbar if the debug option is True if app.debug and app.config['SECRET_KEY']: app.init_debug_toolbar() diff --git a/listenbrainz/webserver/converters.py b/listenbrainz/webserver/converters.py new file mode 100644 index 0000000000..bda821d954 --- /dev/null +++ b/listenbrainz/webserver/converters.py @@ -0,0 +1,10 @@ +from werkzeug.routing import PathConverter, ValidationError + + +class NotApiPathConverter(PathConverter): + + def to_python(self, path): + print(path) + if path.startswith('1/'): + raise ValidationError() + return path diff --git a/listenbrainz/webserver/views/index.py b/listenbrainz/webserver/views/index.py index a45032488b..5681c4d060 100644 --- a/listenbrainz/webserver/views/index.py +++ b/listenbrainz/webserver/views/index.py @@ -274,12 +274,10 @@ def _get_user_count(): @index_bp.route("/", defaults={'path': ''}) -@index_bp.route('//') +@index_bp.route('//') @web_listenstore_needed def index_pages(path): # this is a catch-all route, all unmatched urls match this route instead of raising a 404 # at least in the case the of API urls, we don't want this behavior. hence detect api urls - # and raise 404 errors manually - if path.startswith("1/"): - raise APINotFound(f"Page not found: {path}") + # in the custom NotApiConverter and raise 404 errors manually return render_template("index.html")