client.py uses mutable default arguments, which is bad practice

[sleepydragon]

In django/test/client.py there are several methods that have data={} in their argument lists. The {} dictionary is a mutable type and it is not recommended to use mutable objects as default arguments. This is because each invocation of the method where data is not specified will use the same dict object. This enables completely unrelated method calls to affect each other.

The fix that I recommend is to instead use data=None in the argument list for the affected methods and then in the method body check for None and create a new dict if it is None. That way each method invocation gets their own, local dict object. I have attached a patch for review.

I'm fairly new to Django so it's possible that data=None is actually a meaningful value. If that is the case then this approach will need to be tweaked a bit.

[Simon Charette]

It looks like the dict creation should be moved at the RequestFactory level which also suffer from the same issues.

[Aloïs Guillopé]

This should do. Could you have a look? Is the test I added any use?

[Claude Paroz]

To make sense, a test should fail before applying the patch, which is not the case here. I'll commit an amended patch soon (without tests).

[Claude Paroz <claude@…>]

In 2a31d00933fd1d06ae3013f2c0eb8f4982fa7db6:

Fixed #21740 -- Stopped using mutable default arguments in test client

Thanks Denver Coneybeare for the report and initial patch, and
Atala for another patch.

[tony-zhu]

This fix actually changed the behavior of the test client. I have some tests about HTTP header handling, which I pass an empty string as the request content. So it is like:

self.client.post(url, '', content_type='application/json',
                                    **{
                                        'CUSTOM_HEADER': header_1,
                                    })

Before this change everything worked fine.

The request.body became "{}" after this change. That is because when data is an empty string

data or {}

returns {} instead of ""

[Claude Paroz]

​https://github.com/django/django/pull/3397 should fix the regression.

[Tim Graham]

@tony-zhu FYI, it's preferred to open a new ticket for new bugs after a ticket is released.

PR looks good me; it just needs release notes.

[Claude Paroz <claude@…>]

In f0bb3c98cc9e128cb0c1622be9eb41a26794c91f:

Fixed #21740 -- Allowed test client data to be an empty string

This fixes a regression introduced by 2a31d00933.
Thanks tony-zhu for the report.

[Claude Paroz <claude@…>]

In 53bc81dca3d785d0b399cacbf84cc660355895fc:

[1.7.x] Fixed #21740 -- Allowed test client data to be an empty string

This fixes a regression introduced by 2a31d00933.
Thanks tony-zhu for the report.
Backport of f0bb3c98cc from master.