Skip to content

Crash in WithDynamicViewSetMixin.get_request_fields when matching include[] substrings #364

@jamesbraza

Description

@jamesbraza

With Django Dynamic REST version https://github.com/AltSchool/dynamic-rest/tree/v2.1.7, there is the possibility for a crash based on two include[] fields matching the same substring. This can be reproduced in the test suite's test_api:

    def test_include_matching_substring(self) -> None:
        self.client.get(
            '/users/'
            '?include[]=location.cats'
            '&include[]=location.cats.home'
        )

This will lead to a crash per TypeError("'bool' object does not support item assignment"):

Traceback (most recent call last):
  File "/Users/user/code/dynamic-rest/tests/test_api.py", line 810, in test_foo
    self.client.get(url)
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/test.py", line 289, in get
    response = super().get(path, data=data, **extra)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/test.py", line 206, in get
    return self.generic('GET', path, **r)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/test.py", line 234, in generic
    return super().generic(
           ^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/django/test/client.py", line 541, in generic
    return self.request(**r)
           ^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/test.py", line 286, in request
    return super().request(**kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/test.py", line 238, in request
    request = super().request(**kwargs)
              ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/django/test/client.py", line 810, in request
    self.check_exception(response)
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/django/test/client.py", line 663, in check_exception
    raise exc_value
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 56, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/django/views/decorators/csrf.py", line 55, in wrapped_view
    return view_func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/viewsets.py", line 125, in view
    return self.dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/views.py", line 509, in dispatch
    response = self.handle_exception(exc)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/views.py", line 469, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
    raise exc
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/tests/viewsets.py", line 54, in list
    return super(UserViewSet, self).list(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/mixins.py", line 38, in list
    queryset = self.filter_queryset(self.get_queryset())
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/venv/lib/python3.11/site-packages/rest_framework/generics.py", line 150, in filter_queryset
    queryset = backend().filter_queryset(self.request, queryset, self)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/dynamic_rest/filters.py", line 236, in filter_queryset
    return self._build_queryset(
           ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/dynamic_rest/filters.py", line 522, in _build_queryset
    serializer = self.view.get_serializer()
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/dynamic_rest/viewsets.py", line 317, in get_serializer
    kwargs['request_fields'] = self.get_request_fields()
                               ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/code/dynamic-rest/dynamic_rest/viewsets.py", line 256, in get_request_fields
    current_fields[segment] = include
    ~~~~~~~~~~~~~~^^^^^^^^^
TypeError: 'bool' object does not support item assignment

I think this should either:

  • Generalize WithDynamicViewSetMixin.get_request_fields's logic for parsing include[] and exclude[] to allow substrings
  • Or the TypeError get caught and rejected as an invalid request

A short term workaround is using a Django middleware:

# middleware.py

import traceback
from collections.abc import Callable

from django.http import HttpRequest, HttpResponse, HttpResponseBadRequest


class DuplicateIncludeExcludeFieldSubstringsMiddleware:
    """
    Safely handle duplicate include[] or exclude[] field substrings in query params.

    SEE: https://github.com/AltSchool/dynamic-rest/issues/364
    """

    def __init__(self, get_response: Callable[[HttpRequest], HttpResponse]):
        self.get_response = get_response

    def __call__(self, request: HttpRequest) -> HttpResponse:
        return self.get_response(request)

    def process_exception(self, request: HttpRequest, exception: Exception) -> HttpResponse | None:
        if traceback.extract_tb(exception.__traceback__)[-1].line == "current_fields[segment] = include":
            return HttpResponseBadRequest(
                f"Invalid request with path '{request.get_full_path()}' per message '{exception}'."
            )
        return None  # Indicates to not silence the exception
# settings.py

MIDDLEWARE = [
    ...,
    "cepheus.middleware.DuplicateIncludeExcludeFieldSubstringsMiddleware",
]

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions