Cloud-CV / Cloud-CV/EvalAI

`is_public is True` never matches form-encoded requests, skipping when_made_public and the one-public-submission rule - apps/jobs/views.py:509

Open
#5,227 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.