New reminder, contact and posting pages, accepting a suggestion: a non-numeric id parameter is a server error #409

New issue

Closed

opened 2026-10-01 10:46:29 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:46:29 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading how the views take record ids from the query string and the form, and requesting each with a non-numeric value through the test client.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at applications/views.py:715-722 (ReminderCreateView.asked_application, which since #316 holds the reminder lookup; get_initial at :724-728 calls it, and so does ReminderFormMixin.fallback_url at :686-691 for the Cancel link and the address after saving) and applications/views.py:1291-1293 (SuggestionActionView.post); jobs/views.py, documents/views.py, _as_int and _tag have not moved, and the test client gives the same four answers.

Related to #235 (closed), which fixed this fault on the letter preview with _as_pk in documents/views.py:53-58; this issue adds the four views that fix did not reach. Each hands a raw request value to a primary-key lookup. Django raises ValueError for a primary key that is not a number and none of the four catches it, so a mistyped link or an altered form answers 500 and logs a traceback where an empty choice or a 404 was meant.

Where:
- applications/views.py:677-685, ReminderCreateView.get_initial: request.GET.get("application") into .filter(pk=application_id).
- jobs/views.py:694-702, ContactCreateView.get_initial: request.GET.get("company") into .filter(pk=company_id).
- jobs/views.py:898-906, PostingCreateView.get_initial: the same with ?company=.
- applications/views.py:1251-1253, SuggestionActionView.post: request.POST.get("application") into get_object_or_404(Application.objects.for_user(request.user), pk=chosen).
What happens: a signed-in person opens /applications/reminders/new/?application=abc, or the new contact or new posting page with ?company=abc (a truncated bookmark, a link pasted with a trailing character, ?application=12a). The query raises ValueError: Field 'id' expected a number but got 'abc' and the response is 500. The company page links to the new contact page with ?company= and the application page links to the new reminder page with ?application=, so these are links people follow and share. Posting the accept form of a suggestion (suggestions/<pk>/accept/) with application=x fails the same way. All four were reproduced with the test client.
Why it matters: a parameter that only preselects a field costs the person the whole page, and every such request writes an ERROR record with a traceback that the operator has to read and that hides real failures. It affects only the person's own session; the lookups are narrowed with for_user() and nothing leaks.
Fix: one shared parser. The codebase already has two private ones that differ: _as_pk in documents/views.py:53 returns None, and _as_int in applications/views.py:339 returns 0 (used by ApplicationBulkView._tag at line 313). Move _as_pk into core (for example core/params.py), replace both private helpers with it, and use it in the four places above. In ReminderCreateView.asked_application and the two get_initial methods in jobs/views.py, ignore a value that is not a number. In SuggestionActionView.post, answer 404 for one, as for an id that is not the person's.

Done when

A parametrised test (in tests/test_page_coverage.py, or a new tests/test_query_parameters.py) requests applications:reminder_create with ?application=abc, and jobs:contact_create and jobs:posting_create with ?company=abc, and expects 200 with nothing preselected; all three answer 500 today.
A test in tests/test_suggestions.py accepts a pending suggestion with application=x and expects 404; it answers 500 today.
_as_pk and _as_int no longer exist as private helpers in documents/views.py and applications/views.py; the letter preview test for #235 in tests/test_documents.py still passes.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading how the views take record ids from the query string and the form, and requesting each with a non-numeric value through the test client. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `applications/views.py:715-722` (`ReminderCreateView.asked_application`, which since #316 holds the reminder lookup; `get_initial` at `:724-728` calls it, and so does `ReminderFormMixin.fallback_url` at `:686-691` for the *Cancel* link and the address after saving) and `applications/views.py:1291-1293` (`SuggestionActionView.post`); `jobs/views.py`, `documents/views.py`, `_as_int` and `_tag` have not moved, and the test client gives the same four answers. Related to #235 (closed), which fixed this fault on the letter preview with `_as_pk` in `documents/views.py:53-58`; this issue adds the four views that fix did not reach. Each hands a raw request value to a primary-key lookup. Django raises `ValueError` for a primary key that is not a number and none of the four catches it, so a mistyped link or an altered form answers 500 and logs a traceback where an empty choice or a 404 was meant. - **Where:** - `applications/views.py:677-685`, `ReminderCreateView.get_initial`: `request.GET.get("application")` into `.filter(pk=application_id)`. - `jobs/views.py:694-702`, `ContactCreateView.get_initial`: `request.GET.get("company")` into `.filter(pk=company_id)`. - `jobs/views.py:898-906`, `PostingCreateView.get_initial`: the same with `?company=`. - `applications/views.py:1251-1253`, `SuggestionActionView.post`: `request.POST.get("application")` into `get_object_or_404(Application.objects.for_user(request.user), pk=chosen)`. - **What happens:** a signed-in person opens `/applications/reminders/new/?application=abc`, or the new contact or new posting page with `?company=abc` (a truncated bookmark, a link pasted with a trailing character, `?application=12a`). The query raises `ValueError: Field 'id' expected a number but got 'abc'` and the response is 500. The company page links to the new contact page with `?company=` and the application page links to the new reminder page with `?application=`, so these are links people follow and share. Posting the accept form of a suggestion (`suggestions/<pk>/accept/`) with `application=x` fails the same way. All four were reproduced with the test client. - **Why it matters:** a parameter that only preselects a field costs the person the whole page, and every such request writes an `ERROR` record with a traceback that the operator has to read and that hides real failures. It affects only the person's own session; the lookups are narrowed with `for_user()` and nothing leaks. - **Fix:** one shared parser. The codebase already has two private ones that differ: `_as_pk` in `documents/views.py:53` returns `None`, and `_as_int` in `applications/views.py:339` returns `0` (used by `ApplicationBulkView._tag` at line 313). Move `_as_pk` into `core` (for example `core/params.py`), replace both private helpers with it, and use it in the four places above. In `ReminderCreateView.asked_application` and the two `get_initial` methods in `jobs/views.py`, ignore a value that is not a number. In `SuggestionActionView.post`, answer 404 for one, as for an id that is not the person's. ## Done when - A parametrised test (in `tests/test_page_coverage.py`, or a new `tests/test_query_parameters.py`) requests `applications:reminder_create` with `?application=abc`, and `jobs:contact_create` and `jobs:posting_create` with `?company=abc`, and expects 200 with nothing preselected; all three answer 500 today. - A test in `tests/test_suggestions.py` accepts a pending suggestion with `application=x` and expects 404; it answers 500 today. - `_as_pk` and `_as_int` no longer exist as private helpers in `documents/views.py` and `applications/views.py`; the letter preview test for #235 in `tests/test_documents.py` still passes.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:46:29 +00:00

tiagoagueda added the

bug

tier

labels

2026-10-01 10:46:29 +00:00

tiagoagueda referenced this issue from a commit

2026-10-03 22:56:00 +00:00

Ignore a record id that is not a number instead of answering 500

tiagoagueda closed this issue

2026-10-03 22:56:00 +00:00