kevin1024 / vcrpy

Automatically mock your HTTP interactions to simplify and speed up testing
MIT License
2.72k stars 388 forks source link

aiohttp: `json` payload ignored when `data` keyword is set to `None` instead of being absent. #623

Open leorochael opened 3 years ago

leorochael commented 3 years ago

In vcr/stubs/aiohttp_stubs.py:vcr_request(), the new_request() coroutine checks for data or jsonwith this:

        data = kwargs.get("data", kwargs.get("json"))

But this line assumes that json can only be sent if data is absent. But data could be present and set to None, along with a json parameter that is not None.

In the case of a None in data and a payload in json, the json payload is ignored, and the cassete is recorded as:

interactions:
- request:
    body: null

The equivalent logic in aiohttp itself is tolerant of a None being present in data (as it must, since None is the default value of both arguments):

        if data is not None and json is not None:
            raise ValueError(
                "data and json parameters can not be used at the same time"
            )
        elif json is not None:
            data = payload.JsonPayload(json, dumps=self._json_serialize)

I recommend the logic in vcr/stubs/aiohttp_stubs.py:vcr_request() be changed to match aiohttp.client:

        data = kwargs.get("data")
        if data is None:
            data = kwargs.get("json")
        elif kwargs.get("json") is not None:
            raise ValueError(
                "data and json parameters can not be used at the same time"
            )
djlambert commented 1 year ago

The json needs to be dumped back to a string otherwise an exception is thrown in the matcher. So:

        if data is None:
            data = json.dumps(kwargs.get("json"))
        elif kwargs.get("json") is not None:
            raise ValueError(
                "data and json parameters can not be used at the same time"
            )

I had hoped this bug was resolved with all the other aiohttp fixes. I can provide additional details, need to fix the tests in my project first.