Skip to content

Commit

Permalink
Fix routing logic to show 404 page instead of 403 on bad urls
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
devxpy committed Aug 7, 2024
1 parent b42c1c4 commit 9ef80b3
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 46 deletions.
11 changes: 10 additions & 1 deletion handles/admin.py
Original file line number Diff line number Diff line change
@@ -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),
]
48 changes: 22 additions & 26 deletions routers/root.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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):
Expand All @@ -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(
Expand Down Expand Up @@ -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",
Expand Down
33 changes: 16 additions & 17 deletions routers/static_pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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:
Expand All @@ -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/")
Expand Down
4 changes: 4 additions & 0 deletions server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")


Expand Down
4 changes: 2 additions & 2 deletions url_shortener/routers.py
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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")
Expand Down

0 comments on commit 9ef80b3

Please sign in to comment.