From 04429898ebebd12ebed23cdad318fb9232be30a4 Mon Sep 17 00:00:00 2001 From: Dev Aggarwal Date: Thu, 8 Aug 2024 04:09:20 +0530 Subject: [PATCH] Fix url routing to serve 404 page instead of 403 when static page not found - use blob.exists() to check if static file exists otherwise raise http exception 404 - avoid rendering a handle page with a sub-path (e.g. js/webflow.js) - Refactor `serve_static_file` to handle URLs more robustly - Replace custom exceptions with `HTTPException` in `render_recipe_page` and `url_shortener` functions - Extend `HandleAdmin` search fields with user fields and add filters --- handles/admin.py | 11 ++++++++++- routers/root.py | 15 ++++++++++----- routers/static_pages.py | 34 ++++++++++++++++++---------------- url_shortener/routers.py | 4 ++-- 4 files changed, 40 insertions(+), 24 deletions(-) diff --git a/handles/admin.py b/handles/admin.py index cf90e811f..1a86567ee 100644 --- a/handles/admin.py +++ b/handles/admin.py @@ -1,8 +1,17 @@ from django.contrib import admin +from app_users.admin import AppUserAdmin from .models import Handle @admin.register(Handle) class HandleAdmin(admin.ModelAdmin): - search_fields = ["name", "redirect_url"] + search_fields = ["name", "redirect_url"] + [ + f"user__{field}" for field in AppUserAdmin.search_fields + ] + readonly_fields = ["user", "created_at", "updated_at"] + + list_filter = [ + ("user", admin.EmptyFieldListFilter), + ("redirect_url", admin.EmptyFieldListFilter), + ] diff --git a/routers/root.py b/routers/root.py index 33763b539..d353bddd4 100644 --- a/routers/root.py +++ b/routers/root.py @@ -579,9 +579,9 @@ def chat_lib_route(request: Request, integration_id: str, integration_name: str def recipe_or_handle_or_static( request: Request, page_slug=None, run_slug=None, example_id=None, path=None ): - path = furl(request.url).pathstr.lstrip("/") + parts = request.url.path.strip("/").split("/") - parts = path.strip("/").split("/") + # try to render a recipe page if len(parts) in {1, 2}: try: example_id = parts[1].split("-")[-1] or None @@ -591,12 +591,16 @@ def recipe_or_handle_or_static( return render_recipe_page(request, parts[0], RecipeTabs.run, example_id) except RecipePageNotFound: pass + + # try to render a handle page + if len(parts) == 1: try: return render_handle_page(request, parts[0]) except Handle.DoesNotExist: pass - return serve_static_file(request, path) + # try to serve a static file + return serve_static_file(request) def render_handle_page(request: Request, name: str): @@ -614,8 +618,9 @@ def render_handle_page(request: Request, name: str): raise HTTPException(status_code=404) -class RecipePageNotFound(Exception): - pass +class RecipePageNotFound(HTTPException): + def __init__(self) -> None: + super().__init__(status_code=404) def render_recipe_page( diff --git a/routers/static_pages.py b/routers/static_pages.py index 3969dbf8c..d6e2301ae 100644 --- a/routers/static_pages.py +++ b/routers/static_pages.py @@ -4,12 +4,13 @@ import gooey_gui as gui import requests +from fastapi import HTTPException from starlette.requests import Request from starlette.responses import ( - Response, RedirectResponse, HTMLResponse, PlainTextResponse, + Response, ) from starlette.status import HTTP_308_PERMANENT_REDIRECT, HTTP_401_UNAUTHORIZED @@ -24,20 +25,20 @@ app = CustomAPIRouter() -def serve_static_file(request: Request, path: str): +def serve_static_file(request: Request) -> Response | None: bucket = gcs_bucket() - if path.endswith("/"): - # relative css/js paths dont work with trailing slashes - return RedirectResponse( - os.path.join("/", path.strip("/")), status_code=HTTP_308_PERMANENT_REDIRECT - ) - - path = path or "index" - gcs_path = os.path.join(settings.GS_STATIC_PATH, path) + relpath = request.url.path.strip("/") or "index" + gcs_path = os.path.join(settings.GS_STATIC_PATH, relpath) # if the path has no extension, try to serve a .html file - if not os.path.splitext(gcs_path)[1]: + if not os.path.splitext(relpath)[1]: + # relative css/js paths in html won't work if a trailing slash is present in the url + if request.url.path.lstrip("/").endswith("/"): + return RedirectResponse( + os.path.join("/", relpath), + status_code=HTTP_308_PERMANENT_REDIRECT, + ) html_url = bucket.blob(gcs_path + ".html").public_url r = requests.get(html_url) if r.ok: @@ -51,12 +52,13 @@ def serve_static_file(request: Request, path: str): ) return HTMLResponse(html, status_code=r.status_code) - url = bucket.blob(gcs_path).public_url - r = requests.head(url) - if r.ok: - return RedirectResponse(url, status_code=HTTP_308_PERMANENT_REDIRECT) + blob = bucket.blob(gcs_path) + if blob.exists(): + return RedirectResponse( + blob.public_url, status_code=HTTP_308_PERMANENT_REDIRECT + ) - return Response(status_code=r.status_code) + raise HTTPException(status_code=404) @gui.route(app, "/internal/webflow-upload/") diff --git a/url_shortener/routers.py b/url_shortener/routers.py index dfa2c25cc..29496a2ad 100644 --- a/url_shortener/routers.py +++ b/url_shortener/routers.py @@ -1,5 +1,5 @@ from django.db.models import F -from fastapi import Request +from fastapi import Request, HTTPException from fastapi.responses import RedirectResponse from fastapi.responses import Response @@ -15,7 +15,7 @@ def url_shortener(hashid: str, request: Request): try: surl = ShortenedURL.objects.get_by_hashid(hashid) except ShortenedURL.DoesNotExist: - return Response(status_code=404) + raise HTTPException(status_code=404) # ensure that the url is not disabled and has not exceeded max clicks if surl.disabled or (surl.max_clicks and surl.clicks >= surl.max_clicks): return Response(status_code=410, content="This link has expired")