Summary

Prevent PUT requests to list resources.

Review Request #13284 — Created Sept. 22, 2023 and submitted Sept. 29, 2023, 3:33 p.m.

Information

Owner

david

Repository

Djblets

Branch

release-4.x

Bugs

Depends On

Reviewers

Groups

djblets

People

Description

We've had a long-standing bug in the API where making a PUT request to a
list resource would generally cause a crash (depending on the particular
implementation of the resource's update method, most of which end up
trying to call get_object without the necessary object key in the
kwargs).

This change makes it so that attempting to PUT to a list resource will
fail with an HTTP 405. We had a similar implementation for POST to item
resources, so this implementation is based on that.

Testing Done


Did a PUT to a list resource and saw that I got back an HTTP 405

  (method not allowed) instead of a 500 and HTML error content.
Ran unit tests.
Updated Review Board unit tests to test this functionality for all

  list resources.

Commits

Summary	ID
Prevent PUT requests to list resources. We've had a long-standing bug in the API where making a PUT request to a list resource would generally cause a crash (depending on the particular implementation of the resource's `update` method, most of which end up trying to call `get_object` without the necessary object key in the kwargs). This change makes it so that attempting to PUT to a list resource will fail with an HTTP 405. We had a similar implementation for POST to item resources, so this implementation is based on that. Testing Done: - Did a PUT to a list resource and saw that I got back an HTTP 405 (method not allowed) instead of a 500 and HTML error content. - Ran unit tests. - Updated Review Board unit tests to test this functionality for all list resources.	763b2063e8b6a7754e0f5ada1f8841e4ff5d7b70

Summary

Prevent PUT requests to list resources.

We've had a long-standing bug in the API where making a PUT request to a list resource would generally cause a crash (depending on the particular implementation of the resource's `update` method, most of which end up trying to call `get_object` without the necessary object key in the kwargs). This change makes it so that attempting to PUT to a list resource will fail with an HTTP 405. We had a similar implementation for POST to item resources, so this implementation is based on that. Testing Done: - Did a PUT to a list resource and saw that I got back an HTTP 405 (method not allowed) instead of a 500 and HTML error content. - Ran unit tests. - Updated Review Board unit tests to test this functionality for all list resources.

763b2063e8b6a7754e0f5ada1f8841e4ff5d7b70

Issues

Description	From	Last Updated
Can you update Djblets unit tests for these conditions?	chipx86	Sept. 28, 2023, 1:08 p.m.
local variable 'response' is assigned to but never used Column: 9 Error code: F841	reviewbot	Sept. 28, 2023, 2:23 p.m.
statement ends with a semicolon Column: 51 Error code: E703	reviewbot	Sept. 28, 2023, 2:23 p.m.
local variable 'response' is assigned to but never used Column: 9 Error code: F841	reviewbot	Sept. 28, 2023, 2:23 p.m.
local variable 'response' is assigned to but never used Column: 9 Error code: F841	reviewbot	Sept. 28, 2023, 2:23 p.m.

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues

Can you update Djblets unit tests for these conditions?

david Sept. 28, 2023, 12:14 p.m.

I don't really see anywhere appropriate in the djblets test suite for this. It's covered (quite) thoroughly by the Review Board tests, but we don't seem to actually have any infrastructure in djblets for this right now.

chipx86 Sept. 28, 2023, 12:16 p.m.

We don't have much test infrastructure, but we can construct a subclass and call e.g. .put() on it, see the result.

Commits:

	Summary	ID
	Prevent PUT requests to list resources. We've had a long-standing bug in the API where making a PUT request to a list resource would generally cause a crash (depending on the particular implementation of the resource's `update` method, most of which end up trying to call `get_object` without the necessary object key in the kwargs). This change makes it so that attempting to PUT to a list resource will fail with an HTTP 405. We had a similar implementation for POST to item resources, so this implementation is based on that. Testing Done: - Did a PUT to a list resource and saw that I got back an HTTP 405 (method not allowed) instead of a 500 and HTML error content. - Ran unit tests. - Updated Review Board unit tests to test this functionality for all list resources.	939063d8310394f5643cb1eb51bb12c47d8dbccb
	Prevent PUT requests to list resources. We've had a long-standing bug in the API where making a PUT request to a list resource would generally cause a crash (depending on the particular implementation of the resource's `update` method, most of which end up trying to call `get_object` without the necessary object key in the kwargs). This change makes it so that attempting to PUT to a list resource will fail with an HTTP 405. We had a similar implementation for POST to item resources, so this implementation is based on that. Testing Done: - Did a PUT to a list resource and saw that I got back an HTTP 405 (method not allowed) instead of a 500 and HTML error content. - Ran unit tests. - Updated Review Board unit tests to test this functionality for all list resources.	09a5ffa13718d882590a2a5d49134cb90fb06b1e

Diff:

Revision 2 (+66 -4)

Show changes

	djblets/webapi/resources/base.py
	djblets/webapi/tests/test_webapiresource.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Add more tests.

Commits:

	Summary	ID
	Prevent PUT requests to list resources. We've had a long-standing bug in the API where making a PUT request to a list resource would generally cause a crash (depending on the particular implementation of the resource's `update` method, most of which end up trying to call `get_object` without the necessary object key in the kwargs). This change makes it so that attempting to PUT to a list resource will fail with an HTTP 405. We had a similar implementation for POST to item resources, so this implementation is based on that. Testing Done: - Did a PUT to a list resource and saw that I got back an HTTP 405 (method not allowed) instead of a 500 and HTML error content. - Ran unit tests. - Updated Review Board unit tests to test this functionality for all list resources.	09a5ffa13718d882590a2a5d49134cb90fb06b1e
	Prevent PUT requests to list resources. We've had a long-standing bug in the API where making a PUT request to a list resource would generally cause a crash (depending on the particular implementation of the resource's `update` method, most of which end up trying to call `get_object` without the necessary object key in the kwargs). This change makes it so that attempting to PUT to a list resource will fail with an HTTP 405. We had a similar implementation for POST to item resources, so this implementation is based on that. Testing Done: - Did a PUT to a list resource and saw that I got back an HTTP 405 (method not allowed) instead of a 500 and HTML error content. - Ran unit tests. - Updated Review Board unit tests to test this functionality for all list resources.	a8dfc1014521f4f578057a146bf8de70326cdd82

Diff:

Revision 3 (+164 -4)

Show changes

	djblets/webapi/resources/base.py
	djblets/webapi/tests/test_webapiresource.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/webapi/tests/test_webapiresource.py (Diff revision 3)
The issue has been dropped. Show all issues
```
local variable 'response' is assigned to but never used

Column: 9
Error code: F841
```
djblets/webapi/tests/test_webapiresource.py (Diff revision 3)
The issue has been dropped. Show all issues
```
statement ends with a semicolon

Column: 51
Error code: E703
```
djblets/webapi/tests/test_webapiresource.py (Diff revision 3)
The issue has been dropped. Show all issues
```
local variable 'response' is assigned to but never used

Column: 9
Error code: F841
```
djblets/webapi/tests/test_webapiresource.py (Diff revision 3)
The issue has been dropped. Show all issues
```
local variable 'response' is assigned to but never used

Column: 9
Error code: F841
```

Commits:

	Summary	ID
	Prevent PUT requests to list resources. We've had a long-standing bug in the API where making a PUT request to a list resource would generally cause a crash (depending on the particular implementation of the resource's `update` method, most of which end up trying to call `get_object` without the necessary object key in the kwargs). This change makes it so that attempting to PUT to a list resource will fail with an HTTP 405. We had a similar implementation for POST to item resources, so this implementation is based on that. Testing Done: - Did a PUT to a list resource and saw that I got back an HTTP 405 (method not allowed) instead of a 500 and HTML error content. - Ran unit tests. - Updated Review Board unit tests to test this functionality for all list resources.	a8dfc1014521f4f578057a146bf8de70326cdd82
	Prevent PUT requests to list resources. We've had a long-standing bug in the API where making a PUT request to a list resource would generally cause a crash (depending on the particular implementation of the resource's `update` method, most of which end up trying to call `get_object` without the necessary object key in the kwargs). This change makes it so that attempting to PUT to a list resource will fail with an HTTP 405. We had a similar implementation for POST to item resources, so this implementation is based on that. Testing Done: - Did a PUT to a list resource and saw that I got back an HTTP 405 (method not allowed) instead of a 500 and HTML error content. - Ran unit tests. - Updated Review Board unit tests to test this functionality for all list resources.	763b2063e8b6a7754e0f5ada1f8841e4ff5d7b70

Diff:

Revision 4 (+164 -4)

Show changes

	djblets/webapi/resources/base.py
	djblets/webapi/tests/test_webapiresource.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-4.x (9d5cbeb)