Skip to content

chore: added API validation for numeric values#826

Draft
Stellatsuu wants to merge 7 commits intoDIRACGrid:mainfrom
Stellatsuu:endpoint_input_validation
Draft

chore: added API validation for numeric values#826
Stellatsuu wants to merge 7 commits intoDIRACGrid:mainfrom
Stellatsuu:endpoint_input_validation

Conversation

@Stellatsuu
Copy link
Contributor

Closes: #506

Changes:

  • added min-max validation in per_page query value /jobs/search
  • added min validation in page query value of /jobs/search
  • updated tests

Comments:
I can't reproduce this error from the issue, I think it has been fixed somewhere else:

In the job query for example pymysql.err.ProgrammingError, setting page number > 100

I don't really see other cases here for numeric values. Most numeric values are job_ids that are stored in DB and can't be validated from the FastAPI schemas, same for the page max value that can't be validated since it's based on DB, anyways, a page value out of bound will return a 416 error = no jobs found at this page value (might be why I can't reproduce the error above).

As for strings values, I did not find any relevant cases, most relevant ones does already have a validation (such as max_length and regex)

@Stellatsuu Stellatsuu self-assigned this Mar 10, 2026
Copy link
Contributor

@aldbr aldbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Stellatsuu!
About the client generation, are you using the same version locally than the one in the CI? (normally yes, but worth checking)

# Set the per_page parameter to 0
r = normal_user_client.post("/api/jobs/search", params={"page": 1, "per_page": 0})
assert r.status_code == 400, r.json()
assert r.json()["detail"][0]["msg"] == "Input should be greater than or equal to 1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for not testing the status code (less fragile than testing the content of the message)

Suggested change
assert r.json()["detail"][0]["msg"] == "Input should be greater than or equal to 1"
assert r.status_code == 422, r.json()

Comment on lines +174 to +178
raise HTTPException(
status_code=HTTPStatus.REQUESTED_RANGE_NOT_SATISFIABLE,
detail=HTTPStatus.REQUESTED_RANGE_NOT_SATISFIABLE.phrase,
headers={"Content-Range": f"jobs */{total}"},
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are changing the behavior of the API here:

  • it's a breaking change
  • the reason it was made like this are explained in the RFC 7233 linked above:

Note: Because servers are free to ignore Range, many
implementations will simply respond with the entire selected
representation in a 200 (OK) response. That is partly because
most clients are prepared to receive a 200 (OK) to complete the
task (albeit less efficiently) and partly because clients might
not stop making an invalid partial request until they have
received a complete representation. Thus, clients cannot depend
on receiving a 416 (Range Not Satisfiable) response even when it
is most appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Input validation is required to avoid 500 errors

2 participants