`is_public is True` never matches form-encoded requests, skipping when_made_public and the one-public-submission rule - apps/jobs/views.py:509
- Dominant language
- Python
- Stars
- 2k
- Forks
- 984
- Avg merge
- 2h 54m
- Merged PRs (30d)
- 14
Description
`change_submission_data_and_visibility()` compares a request value with `is True`. Form-encoded clients send `"true"` as a **string**, so the check never matches: the submission still becomes public, but `when_made_public` is never stamped and the "only one public submission" rule is silently skipped.
## Offending code
https://github.com/Cloud-CV/EvalAI/blob/f8aff86a7e613c61fae3e4b0e5c0382fe3b52a25/apps/jobs/views.py#L507-L523
```python
try:
is_public = request.data["is_public"]
if is_public is True:
when_made_public = datetime.datetime.now()
request.data["when_made_public"] = when_made_public
submissions_already_public = Submission.objects.filter(...)
# Make the existing public submission private before making the new
# submission public
if (
challenge_phase.is_restricted_to_select_one_submission
and is_public
and submissions_already_public.count() == 1
):
```
`is_public` is taken straight from `request.data` with no coercion, then compared by **identity** against `True`.
## Why it is wrong
This is a `PATCH` endpoint with DRF's default parsers, which include `FormParser` and `MultiPartParser`:
```
>>> from rest_framework.settings import api_settings
>>> [p.__name__ for p in api_settings.DEFAULT_PARSER_CLASSES]
['JSONParser', 'FormParser', 'MultiPartParser']
```
Form-encoded and multipart bodies carry no type information, so every value arrives as a **string**. `"true" is True` is `False`, and so is `"True" is True` and `"1" is True`. Only a JSON body carrying a real boolean satisfies the check.
The damaging part is that the request does not fail. The serializer further down applies DRF's `BooleanField` to `is_public`, which happily coerces `"true"` to `True` and saves it — so the submission **does** become public, while the entire block guarded by `is True` was skipped:
| client | `is_public` | becomes public | `when_made_public` stamped | old public demoted |
|---|---|---|---|---|
| JSON body (web UI) | `True` (bool) | ✅ | ✅ | ✅ |
| form-encoded | `"true"` | ✅ | ❌ | ❌ |
| form-encoded | `"True"` | ✅ | ❌ | ❌ |
| form-encoded | `"1"` | ✅ | ❌ | ❌ |
The web UI is unaffected — `challengeCtrl.js` sends `parameters.data = {"is_public": submissionVisibility}` as JSON, a real boolean. This is reached by API and CLI clients.
## Relationship to #4830
#4830 reports that multiple submissions can be public at once in a phase with `is_restricted_to_select_one_submission=True`, and attributes it to a race condition; PR #4831 proposes `select_for_update()` locking at these same lines.
This is a **second, independent cause of the same broken state**, and it needs no concurrency at all — a single form-encoded PATCH reaches it deterministically. `is_public is True` does not appear anywhere in PR #4831's diff, so that locking fix leaves this path live. I mention it because a reviewer looking at #4830 may otherwise assume the locking change closes it entirely.
## Steps to reproduce
1. Create a challenge phase with `is_restricted_to_select_one_submission=True` and one public submission.
2. As a participant, `PATCH` a second submission at
`/api/jobs/challenge//challenge_phase//submission/`
sending **form-encoded** (`multipart/form-data` or `application/x-www-form-urlencoded`) data:
```
is_public=true
```
3. The response is HTTP 200 and the second submission is now public.
4. Query the phase: **two** submissions have `is_public=True`, and the new one's `when_made_public` is `NULL`.
Repeating step 2 with a JSON body (`{"is_public": true}`) demotes the first submission correctly, which shows the divergence.
## Expected behaviour
- `is_public` is coerced before being tested, so form-encoded and JSON clients behave identically.
- `when_made_public` is stamped whenever a submission is made public.
- The restrict-one-public branch runs for form-encoded clients too.
- A value that is not a recognised boolean returns HTTP 400 naming `is_public`.
---
I have a branch that coerces the value through DRF's `BooleanField` before the check and adds regression tests; I'll open a PR referencing this issue.
Contributor guide
Research direction
Start at apps/jobs/views.py:509 and inspect how DRF parses BooleanField values in request.data for the submission PATCH endpoint. Reproduce form-encoded and JSON requests; done means both stamp when_made_public and enforce the one-public-submission rule, invalid boolean values return 400, and regression coverage is added.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- django, python
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 42/100