readthedocs / readthedocs.org

The source code that powers readthedocs.org
https://readthedocs.org/
MIT License
7.99k stars 3.58k forks source link

Use django forms for query string parameters validation #8955

Open stsewd opened 2 years ago

stsewd commented 2 years ago

Some of our views that receive user input and interact with the db can fail on some invalid input, like when passing null characters (%00) (probably these types of requests are from some bots trying to exploit a vulnerability in our app). Currently, Postgres will raise an exception, which adds noise to sentry.

It is also hard to know what parameters a view expects (currently we document some of them in the view's docstring), and for search we are even using a named tuple to be able to pass the data around https://github.com/readthedocs/readthedocs.org/blob/987b09f0f32606b50d4dfaf7cdbb112c171fe9f1/readthedocs/search/views.py#L26-L36

All these problems can be solved by using a django form to validate the parameters, which basically means

form = Form(request.GET)
if not form.is_valid():
    return Response(400)
form.cleaned_data['foo']
...

I don't see much around using forms on get requests on the django docs, but I've seen other frameworks doing this to have more structured query parameters.

If you don't like this, we can also just validate all query params with https://docs.djangoproject.com/en/dev/ref/validators/#prohibitnullcharactersvalidator before passing those values to the get handler.

humitos commented 2 years ago

I saw this issue in: