fix(core): return 404 from marker_preview when ?id= is missing, non-int, or unknown#915
Open
SAY-5 wants to merge 1 commit intomemeLab:developfrom
Open
fix(core): return 404 from marker_preview when ?id= is missing, non-int, or unknown#915SAY-5 wants to merge 1 commit intomemeLab:developfrom
SAY-5 wants to merge 1 commit intomemeLab:developfrom
Conversation
…nt, or unknown
marker_preview fetched the marker with Marker.objects.get(id=marker_id)
where marker_id came straight off request.GET.get("id"). Three failure
modes all reach Django's default 500 handler:
1. ?id missing entirely: marker_id is None; the ORM issues a query
with id IS NULL which then raises DoesNotExist.
2. ?id=abc (or any non-numeric junk a crawler supplies): the DB
driver rejects the cast with ValueError / DataError.
3. ?id=99999 (deleted or never existed): DoesNotExist.
All three are 404s, not server errors. /marker/preview is unauthenticated
and reachable from links in the collection UI, so stale links + bots
reliably produce 500s in the error tracker.
Coerce marker_id to int first (Http404 on failure) and fetch via
get_object_or_404, matching the pattern already used by exhibit_detail
and related_content in the same module. get_object_or_404 and Http404
are already imported.
Fixes memeLab#847
Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
marker_previewinsrc/core/views/views.pycalls:marker_idis whatever came off the query string:None, a numeric string, or arbitrary junk.Marker.objects.get(id=marker_id)turns every non-happy-path into an HTTP 500 via Django's default handler:?idmissing →marker_id is None→ ORM issuesid IS NULL→DoesNotExist.?id=abc(or any non-numeric junk bots send) → DB driver rejects the integer cast →ValueError/DataError.?id=99999(deleted / never existed) →DoesNotExist.All three should be 404s, not server errors. The
/marker/previewendpoint is unauthenticated and reachable from marker thumbnails in the collection UI, so stale links and crawlers reliably log 500s in the error tracker, drowning real bugs.Fix
Same pattern already used for
exhibit_detailandrelated_contentin this module: coerce the id tointfirst (Http404on failure), then fetch viaget_object_or_404.get_object_or_404andHttp404are already imported fromdjango.shortcuts/django.httpat the top of the file; no new imports needed. Behaviour for a valid, existing marker id is unchanged.Fixes #847