From 9ef80b376ae771cc6f348a6e0dfedf977df4948c Mon Sep 17 00:00:00 2001 From: Dev Aggarwal Date: Thu, 8 Aug 2024 03:41:36 +0530 Subject: [PATCH] Fix routing logic to show 404 page instead of 403 on bad urls - Refactor routing to separate recipe and handle routes from static file serving. - Enhance the `RecipePageNotFound` exception to use HTTP 404 status code. - Update `serve_static_file` function to improve handling of paths without extensions. - Ensure static files are served when 404 or 405 exceptions occur instead of a catch-all route - Improve error handling in URL shortener to raise `HTTPException` instead of returning a response. - Extend `HandleAdmin` to include user search fields, read-only fields, and list filters. --- handles/admin.py | 11 ++++++++- routers/root.py | 48 ++++++++++++++++++---------------------- routers/static_pages.py | 33 ++++++++++++++------------- server.py | 4 ++++ url_shortener/routers.py | 4 ++-- 5 files changed, 54 insertions(+), 46 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..dec19b831 100644 --- a/routers/root.py +++ b/routers/root.py @@ -43,7 +43,6 @@ from daras_ai_v2.settings import templates from handles.models import Handle from routers.custom_api_router import CustomAPIRouter -from routers.static_pages import serve_static_file app = CustomAPIRouter() @@ -569,34 +568,30 @@ def chat_lib_route(request: Request, integration_id: str, integration_name: str ) +# this must be before the recipe_or_handle_route so that we can serve handles +@gui.route(app, "/{page_slug}/") +def recipe_or_handle_route(request: Request, page_slug: str): + try: + return render_recipe_page(request, page_slug, RecipeTabs.run, None) + except RecipePageNotFound: + pass + try: + return render_handle_page(request, page_slug) + except Handle.DoesNotExist: + pass + raise HTTPException(status_code=404) + + @gui.route( app, - "/{path:path}", - "/{page_slug}/", + "/{page_slug}/", # yes this is redundant, but helps to reverse urls "/{page_slug}/{run_slug}/", "/{page_slug}/{run_slug}-{example_id}/", ) -def recipe_or_handle_or_static( - request: Request, page_slug=None, run_slug=None, example_id=None, path=None +def recipe_page_route( + request: Request, page_slug: str, run_slug: str = None, example_id: str = None ): - path = furl(request.url).pathstr.lstrip("/") - - parts = path.strip("/").split("/") - if len(parts) in {1, 2}: - try: - example_id = parts[1].split("-")[-1] or None - except IndexError: - example_id = None - try: - return render_recipe_page(request, parts[0], RecipeTabs.run, example_id) - except RecipePageNotFound: - pass - try: - return render_handle_page(request, parts[0]) - except Handle.DoesNotExist: - pass - - return serve_static_file(request, path) + return render_recipe_page(request, page_slug, RecipeTabs.run, example_id) def render_handle_page(request: Request, name: str): @@ -614,8 +609,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( @@ -724,7 +720,7 @@ class RecipeTabs(TabData, Enum): run = TabData( title=f"{icons.run} Run", label="", - route=recipe_or_handle_or_static, + route=recipe_page_route, ) examples = TabData( title=f"{icons.example} Examples", diff --git a/routers/static_pages.py b/routers/static_pages.py index 3969dbf8c..2228908b6 100644 --- a/routers/static_pages.py +++ b/routers/static_pages.py @@ -6,10 +6,10 @@ import requests 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 +24,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 +51,11 @@ 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) - - return Response(status_code=r.status_code) + blob = bucket.blob(gcs_path) + if blob.exists(): + return RedirectResponse( + blob.public_url, status_code=HTTP_308_PERMANENT_REDIRECT + ) @gui.route(app, "/internal/webflow-upload/") diff --git a/server.py b/server.py index 4b4265f01..88d9f6dc3 100644 --- a/server.py +++ b/server.py @@ -18,6 +18,7 @@ from daras_ai_v2.pydantic_validation import convert_errors from daras_ai_v2.settings import templates from gooeysite import wsgi +from routers.static_pages import serve_static_file assert wsgi @@ -124,6 +125,9 @@ async def validation_exception_handler(request: Request, exc: RequestValidationE @app.exception_handler(HTTP_404_NOT_FOUND) @app.exception_handler(HTTP_405_METHOD_NOT_ALLOWED) async def not_found_exception_handler(request: Request, exc: HTTPException): + resp = serve_static_file(request) + if resp is not None: + return resp return await _exc_handler(request, exc, "errors/404.html") 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")