aiohttp icon indicating copy to clipboard operation
aiohttp copied to clipboard

Form-encoded data with None as the value gets passed as string "None"

Open kiraware opened this issue 1 year ago • 5 comments

Describe the bug

When i have form encoded data with None, by using requests the data is not sent to the server. But this is different for aiohttp, is sent "None" string. This happen also in httpx, take a look at this issue https://github.com/encode/httpx/issues/1500. Is this really the expected behavior?

To Reproduce

Code

import asyncio
from  aiohttp import ClientSession 

async def main():
  async with ClientSession() as session:
    async with session.post("https://httpbin.org/post", data={"foo": None}) as resp:
      print(await resp.json())

asyncio.run(main())

Output

{'args': {}, 'data': '', 'files': {}, 'form': {'foo': 'None'}, 'headers': {'Accept': '*/*', 'Accept-Encoding': 'gzip, deflate', 'Content-Length': '8', 'Content-Type': 'application/x-www-form-urlencoded', 'Host': 'httpbin.org', 'User-Agent': 'Python/3.12 aiohttp/3.9.1', 'X-Amzn-Trace-Id': 'Root=1-65ae0f19-5b556c225f8d6df5174e435d'}, 'json': None, 'origin': '110.137.36.152', 'url': 'https://httpbin.org/post'}

Take a look at the form, its contain None 'form': {'foo': 'None'}.

Expected behavior

Code

import requests

resp = requests.post("https://httpbin.org/post", data={"foo": None})
print(resp.json())

Output

{'args': {}, 'data': '', 'files': {}, 'form': {}, 'headers': {'Accept': '*/*', 'Accept-Encoding': 'gzip, deflate', 'Content-Type': 'application/x-www-form-urlencoded', 'Host': 'httpbin.org', 'Transfer-Encoding': 'chunked', 'User-Agent': 'python-requests/2.31.0', 'X-Amzn-Trace-Id': 'Root=1-65ae0f4c-569162ab7ff3089468c91391'}, 'json': None, 'origin': '110.137.36.152', 'url': 'https://httpbin.org/post'}

Take a look at the form, its empty 'form': {}.

Logs/tracebacks

N/A

Python Version

Python 3.12.1

aiohttp Version

name         : aiohttp
version      : 3.9.1
description  : Async http client/server framework (asyncio)

multidict Version

name         : multidict
version      : 6.0.4
description  : multidict implementation

yarl Version

name         : yarl
version      : 1.9.4
description  : Yet another URL library

OS

Windows 10

Related component

Client

Additional context

No response

Code of Conduct

  • [X] I agree to follow the aio-libs Code of Conduct

kiraware avatar Jan 22 '24 07:01 kiraware

This seems like an antipattern to me and I'd be more inclined to raise a TypeError. I don't have a strong opinion though, so if other maintainers think it's a good idea then I'm fine with it.

I would note there is a lack of type annotations here though, so those should probably be fixed regardless (ClientSession._request(data=) and FormData.__init__(fields=)).

Dreamsorcerer avatar Jan 22 '24 16:01 Dreamsorcerer

Some easy workaround i found is on stackoverflow, just remove dict element with None value.

Code snapshot

{k: v for k, v in data.items() if v is not None}

kiraware avatar Jan 23 '24 02:01 kiraware

Yes, exactly. If you don't want a parameter output, then don't pass a parameter at all. Passing None I would currently describe as undefined behaviour, as it's not an expected value.

Dreamsorcerer avatar Jan 23 '24 13:01 Dreamsorcerer

I would like to know if this behavior is documented and is it already included in test? If not, i suggest provide the documentation and add the test for this behavior, so the expected behavior is defined, which result as None string.

kiraware avatar Jan 24 '24 02:01 kiraware

Is that the expected behaviour though? As far as I can tell the method expects string values. Any other type should either raise a TypeError or, to save some performance, have undefined behaviour.

Dreamsorcerer avatar Jan 24 '24 15:01 Dreamsorcerer

Honestly, response with string "None" is unexpected for me. httpx now response with "" for None, see this https://github.com/encode/httpx/pull/1539 for more detail.

kiraware avatar Aug 06 '24 12:08 kiraware

Again, I'd learn towards a TypeError. It suggests the developer has done something wrong in their code. If you'd like to contribute such a change, I'm happy to review.

Most importantly, we need the type annotations sorted as mentioned in my initial comment. Then atleast type checkers will give an error when a developer has a None.

Dreamsorcerer avatar Aug 06 '24 12:08 Dreamsorcerer