Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task done
kiraware opened this issue Jan 22, 2024 · 7 comments · Fixed by #9292
Closed
1 task done

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

kiraware opened this issue Jan 22, 2024 · 7 comments · Fixed by #9292
Labels

Comments

@kiraware
Copy link

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 encode/httpx#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

  • I agree to follow the aio-libs Code of Conduct
@kiraware kiraware added the bug label Jan 22, 2024
@Dreamsorcerer
Copy link
Member

Dreamsorcerer commented Jan 22, 2024

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=)).

@kiraware
Copy link
Author

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}

@Dreamsorcerer
Copy link
Member

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.

@kiraware
Copy link
Author

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.

@Dreamsorcerer
Copy link
Member

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.

@kiraware
Copy link
Author

kiraware commented Aug 6, 2024

Honestly, response with string "None" is unexpected for me. httpx now response with "" for None, see this encode/httpx#1539 for more detail.

@Dreamsorcerer
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants