+ add documentation for change strategy

Author: dlfoerster

change_implementation/concept_location.md

@@ -0,0 +1,93 @@
+# DRF Issue #6855 Concept Location
+
+## Problem Summary
+
+The browsable API can crash while rendering an extra action page when the response data comes from a serializer whose `instance` is not a normal model object for the current view's permission class.
+
+The failing path is in the browsable API renderer, not in view dispatch:
+
+1. The extra action returns `Response(serializer.data)`.
+2. `serializer.data` is a `ReturnDict` or `ReturnList` that keeps a backlink to the serializer.
+3. `BrowsableAPIRenderer` inspects that serializer during HTML rendering.
+4. While building placeholder UI for other HTTP methods, it calls `view.check_object_permissions(request, instance)`.
+5. `instance` may be a non-model object from the serializer, which can be incompatible with the permission class and crash with `AttributeError`.
+
+## Primary Source Locations
+
+### `rest_framework/renderers.py`
+
+This is the main issue location.
+
+- `BrowsableAPIRenderer.get_context()`
+  - Always asks for `put_form`, `post_form`, `delete_form`, and `options_form`.
+  - This means a normal `GET` render also evaluates synthetic UI for other methods.
+
+- `BrowsableAPIRenderer.get_rendered_html_form()`
+  - Pulls `serializer = getattr(data, 'serializer', None)`.
+  - Extracts `instance = getattr(serializer, 'instance', None)` when `many=False`.
+  - Calls `show_form_for_method()` before the current `DELETE` / `OPTIONS` bailout.
+  - This ordering is what exposes the bug.
+
+- `BrowsableAPIRenderer.show_form_for_method()`
+  - Calls `view.check_permissions(request)`.
+  - If `obj is not None`, also calls `view.check_object_permissions(request, obj)`.
+  - Only catches `APIException`, so an unexpected `AttributeError` from a permission class bubbles up and crashes rendering.
+
+## Supporting Locations
+
+### `rest_framework/utils/serializer_helpers.py`
+
+- `ReturnDict`
+- `ReturnList`
+
+These wrappers preserve `data.serializer`, which is why renderers can see the original serializer and its `.instance`.
+
+### `rest_framework/views.py`
+
+- `APIView.check_object_permissions()`
+
+This loops through permission classes and directly passes the provided object to `has_object_permission()`. It assumes the caller supplied the correct domain object.
+
+### `rest_framework/request.py`
+
+- `override_method`
+
+`get_rendered_html_form()` uses this context manager while probing alternate HTTP methods such as `OPTIONS` and `DELETE`.
+
+## UI/Template Locations
+
+### `rest_framework/templates/rest_framework/base.html`
+
+- `options_form`
+- `delete_form`
+
+These booleans control whether the browsable API shows the `OPTIONS` button and `DELETE` button/modal.
+
+### `rest_framework/templates/rest_framework/admin.html`
+
+- `delete_form`
+
+The admin renderer also consumes the truthiness of `delete_form`.
+
+## Existing Test Locations To Extend
+
+### `tests/test_renderers.py`
+
+Contains `BrowsableAPIRendererTests`, which is the closest existing unit/integration coverage for renderer behavior and extra actions.
+
+### `tests/browsable_api/test_form_rendering.py`
+
+Already has regression coverage around browsable API form generation and serializer shapes, including `many=True`.
+
+### `tests/browsable_api/views.py`
+
+Contains an example object-permission class that accesses nested attributes on the object and is useful as a pattern for reproducing this class of failure.
+
+## Key Conceptual Insight
+
+The renderer is mixing two different concepts:
+
+- the object that was serialized for display
+- the object that should be used for permission checks when deciding whether to show action UI
+
+For issue #6855, the immediate crash is triggered by the synthetic `OPTIONS` UI path, where no object-level edit form is actually rendered, so reusing `serializer.instance` is not appropriate.

change_implementation/impact_analysis.md

@@ -0,0 +1,102 @@
+# DRF Issue #6855 Impact Analysis
+
+## Scope Of The Bug
+
+The bug affects HTML rendering through `BrowsableAPIRenderer`. It does not affect normal JSON responses or the primary view execution path.
+
+The issue appears when all of the following are true:
+
+1. A browsable API page is rendered.
+2. The response data came from `serializer.data`, so the renderer can access `data.serializer`.
+3. The serializer has `many=False`, so `serializer.instance` is used.
+4. The serializer instance is not a safe object for the view's `has_object_permission()` logic.
+5. The renderer probes another method, especially `OPTIONS`, during page construction.
+
+## Observable User Impact
+
+- The browsable API page crashes instead of rendering successfully.
+- The failure happens after the view has already produced a valid response payload.
+- API clients using non-HTML renderers are unaffected, so the bug can be invisible in automated API tests unless browsable rendering is exercised.
+
+## Why Extra Actions Are A Common Trigger
+
+Extra actions are more likely to serialize data that is not the same object type returned by `get_object()`. Examples include:
+
+- projection objects
+- service-layer result objects
+- denormalized DTO-like objects
+- serializer-backed views that do not call `get_object()`
+
+That makes `serializer.instance` an unreliable proxy for object-permission checks during browsable API rendering.
+
+## Method-Level Impact
+
+### `OPTIONS`
+
+This is the most important path for issue #6855.
+
+- `OPTIONS` is typically present in `view.allowed_methods`.
+- `get_context()` asks for `options_form` on normal page render.
+- `get_rendered_html_form()` currently runs permission checks before its `OPTIONS` early return.
+- No real serializer-bound form is produced for `OPTIONS`, so the object-level permission check is not necessary for rendering the placeholder button.
+
+### `DELETE`
+
+This method shares the same code shape, but the impact is different.
+
+- `DELETE` may or may not be allowed on the route.
+- `delete_form` is used to decide whether to show destructive UI.
+- Unlike `OPTIONS`, object-level permission checks may still be meaningful here because the button reflects an action against a resource.
+
+This means a broad "skip object checks for both `DELETE` and `OPTIONS`" change would be riskier than a focused `OPTIONS` fix.
+
+### `PUT` / `PATCH`
+
+These are not the direct cause of the reported crash for a GET-only extra action because they usually fail the `allowed_methods` check earlier. They remain important regression surface because they rely on `instance` to prepopulate edit forms.
+
+## Compatibility Risks
+
+### Low-risk changes
+
+- Preventing object-permission checks for the synthetic `OPTIONS` path.
+- Keeping existing permission behavior for edit and delete affordances.
+
+### Medium-risk changes
+
+- Replacing `serializer.instance` with `view.get_object()` inside the renderer.
+
+This could:
+
+- trigger additional database queries
+- fail on views that do not implement `get_object()`
+- introduce side effects in renderer code
+- blur the boundary between response rendering and view object retrieval
+
+### Higher-risk changes
+
+- Skipping object-permission checks for `DELETE`
+- Removing `delete_form` behavior entirely
+
+Either could change which users see destructive UI and would need stronger review.
+
+## Testing Impact
+
+Regression coverage should include:
+
+1. A browsable API request to an extra action that returns `serializer.data` for a non-model object.
+2. A permission class whose `has_object_permission()` would fail if handed that serializer instance.
+3. Confirmation that the HTML response renders successfully instead of crashing.
+4. Confirmation that existing delete/edit form behavior is not unintentionally broadened.
+
+Useful secondary coverage:
+
+- a direct renderer test for the `OPTIONS` path
+- confirmation that `many=True` behavior remains unchanged
+
+## Documentation Impact
+
+No public API documentation change is likely required if the fix is internal and behavior-preserving. A release note entry may still be appropriate because the change resolves a user-visible browsable API crash.
+
+## Recommended Impact Conclusion
+
+The smallest safe fix is to narrow the change to the synthetic `OPTIONS` form-generation path. That addresses the reported crash while minimizing risk to object-permission-sensitive UI such as delete and edit actions.

change_implementation/other_useful_info.md

@@ -0,0 +1,71 @@
+## Quick Overview of Important Classes
+
+Inside `rest_framework/`, common areas are:
+
+`serializers.py`: converts Python/Django objects to API data and validates input.
+`views.py`, `generics.py`, `viewsets.py`: request handling abstractions.
+`routers.py`: URL routing for viewsets.
+`permissions.py`, `authentication.py`, `throttling.py`: access control and API policies.
+`renderers.py`, `parsers.py`: output/input formats like JSON and the browsable API.
+`response.py`, `request.py`: DRF’s request/response wrappers.
+
+## Issue in Plain English
+
+Issue #6855 is a Browsable API bug. The actual API endpoint works, but the HTML page crashes while DRF is trying to decide which buttons/forms to show.
+
+The key flow is:
+
+- A view action returns `Response(serializer.data)`.
+- In browsable mode, DRF looks at `data.serializer`.
+- It pulls `serializer.instance`.
+- It uses that instance for permission checks while deciding whether to show forms/buttons.
+- For some extra actions, that serializer.instance is not the real model object the permission logic expects.
+- Result: DRF calls object-permission logic on the wrong kind of object and crashes.
+
+This is the critical code path:
+
+```python
+# renderers.py
+def show_form_for_method(self, view, method, request, obj):
+    """
+    Returns True if a form should be shown for this method.
+    """
+    if method not in view.allowed_methods:
+        return  # Not a valid method
+
+    try:
+        view.check_permissions(request)
+        if obj is not None:
+            view.check_object_permissions(request, obj)
+```
+
+And this is where the renderer derives the object from the serializer and then asks for forms for several methods:
+
+```python
+# renderers.py
+# See issue #2089 for refactoring this.
+serializer = getattr(data, 'serializer', None)
+if serializer and not getattr(serializer, 'many', False):
+    instance = getattr(serializer, 'instance', None)
+    if isinstance(instance, Page):
+        instance = None
+else:
+    instance = None
+
+with override_method(view, request, method) as request:
+    if not self.show_form_for_method(view, method, request, instance):
+        return
+
+    if method in ('DELETE', 'OPTIONS'):
+        return True  # Don't actually need to return a form
+```
+
+And the browsable page always probes multiple methods, even on a normal GET render:
+
+```python
+# renderers.py
+'put_form': self.get_rendered_html_form(data, view, 'PUT', request),
+'post_form': self.get_rendered_html_form(data, view, 'POST', request),
+'delete_form': self.get_rendered_html_form(data, view, 'DELETE', request),
+'options_form': self.get_rendered_html_form(data, view, 'OPTIONS', request),
+```

change_implementation/source_code_strategy.md

@@ -0,0 +1,93 @@
+# DRF Issue #6855 Source Code Strategy
+
+## Goal
+
+Fix the browsable API crash for extra actions that return serializer-backed data whose `serializer.instance` is not a valid object for the view's object-permission logic.
+
+## Recommended Strategy
+
+Implement a focused change in `rest_framework/renderers.py` so that the synthetic `OPTIONS` button path does not call `check_object_permissions()` on `serializer.instance`.
+
+## Why This Strategy
+
+It directly addresses the reported crash with the smallest behavioral change.
+
+It also preserves current behavior for methods where object-level permission checks are still meaningful:
+
+- `PUT`
+- `PATCH`
+- `DELETE`
+
+## Proposed Code Change
+
+### Primary change
+
+Refactor `BrowsableAPIRenderer.get_rendered_html_form()` so that `OPTIONS` is handled before `show_form_for_method(view, method, request, instance)` is called.
+
+Recommended behavior:
+
+1. Enter `override_method(view, request, method)` as today.
+2. If `method == 'OPTIONS'`:
+   - return early when the method is not allowed
+   - run `view.check_permissions(request)`
+   - do not run `view.check_object_permissions(request, instance)`
+   - return `True` so the template can render the `OPTIONS` button
+3. For all other methods, keep the existing `show_form_for_method()` flow.
+4. Keep the existing `DELETE` truthy return after permission gating.
+
+## Why Not Use `view.get_object()`
+
+Avoid changing the renderer to retrieve a new object from the view.
+
+Reasons:
+
+- not every relevant view implements `get_object()`
+- extra actions may intentionally not be tied to the routed object
+- renderer-time object lookup can introduce extra queries and side effects
+- it is a larger semantic change than needed for this bug
+
+## Why Not Skip Checks For Both `DELETE` And `OPTIONS`
+
+That would be simpler mechanically, but it risks changing existing UI authorization behavior for delete actions.
+
+`OPTIONS` is different because the renderer does not build an object-bound form for it. It only needs enough permission context to decide whether the method is generally accessible.
+
+## Suggested Test Plan
+
+### Regression test
+
+Add a renderer-focused regression test in `tests/test_renderers.py` near `BrowsableAPIRendererTests`.
+
+Suggested test shape:
+
+1. Define a `ViewSet` with browsable API enabled.
+2. Add a detail extra action that returns `Response(serializer.data)`.
+3. Use a serializer whose `instance` is a custom object without the attributes expected by object permissions.
+4. Attach a permission class whose `has_object_permission()` would raise `AttributeError` if called with that custom object.
+5. Request the extra action with `HTTP_ACCEPT='text/html'`.
+6. Assert that the response renders successfully with status `200`.
+
+### Guardrail test
+
+Add a small test confirming that delete UI still respects the existing permission gate, or at minimum ensure no existing delete-related renderer test regresses when the new test is added.
+
+### Optional unit test
+
+If maintainers prefer tighter isolation, add a direct unit test around `get_rendered_html_form(..., method='OPTIONS', ...)` to assert the method returns truthy without consulting object permissions.
+
+## Implementation Notes
+
+- Keep the fix local to `BrowsableAPIRenderer`; avoid changing core permission APIs.
+- Do not broaden exception handling to swallow arbitrary `AttributeError`. That would hide real bugs instead of correcting the invalid permission-check input.
+- Preserve the current `many=True` behavior, where `instance` is already treated as `None`.
+
+## Validation Steps
+
+1. Run the new regression test.
+2. Run existing browsable API renderer tests.
+3. Verify no change in non-HTML renderer behavior.
+4. Manually confirm that an authenticated browsable API extra action page now renders instead of crashing.
+
+## Fallback Option
+
+If reviewers want a more centralized fix, a secondary approach is to teach `show_form_for_method()` to skip object-permission checks only for `OPTIONS`. That is still acceptable, but the `get_rendered_html_form()`-level change keeps the special case closest to the synthetic-form behavior that causes the bug.