jawah / niquests

“Safest, Fastest, Easiest, and Most advanced” Python HTTP Client. Production Ready! Drop-in replacement for Requests. HTTP/1.1, HTTP/2, and HTTP/3 supported.
https://niquests.readthedocs.io/en/latest/
Apache License 2.0
775 stars 18 forks source link

Encoding None Values #118

Closed karosc closed 2 months ago

karosc commented 2 months ago

I was testing niquests to see if it is a drop-in replacement for requests for my project and found an issue. My project sometimes ends up posting data that has None values. Requests does not have an issue with this, but niquests throws a TypeError: 'NoneType' object is not iterable error.

The difference comes in how requests and niquests have their `_encode_params method. See the requests implementation and the niquests implementation

Expected Result

Expect the post complete when posting data with None values.

Actual Result

niquests throws a TypeError: 'NoneType' object is not iterable error.

Reproduction Steps

import niquests
import requests

url = "https://httpbin.org/post"
data = {'test_none': None}

# requests completes
resp = requests.post(url, data = data)
print(resp.json())

resp = niquests.post(url, data = data)
print(resp.json())

System Information

$ python -m niquests.help
{
  "charset_normalizer": {
    "version": "3.3.2"
  },
  "http1": {
    "h11": "0.14.0"
  },
  "http2": {
    "jh2": "5.0.3"
  },
  "http3": {
    "enabled": true,
    "qh3": "1.0.4"
  },
  "idna": {
    "version": "3.6"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.8.18"
  },
  "niquests": {
    "version": "3.6.2"
  },
  "ocsp": {
    "enabled": true
  },
  "platform": {
    "release": "6.5.0-28-generic",
    "system": "Linux"
  },
  "system_ssl": {
    "version": "300000d0"
  },
  "urllib3.future": {
    "cohabitation_version": null,
    "version": "2.7.906"
  },
  "wassima": {
    "certifi_fallback": false,
    "enabled": true,
    "version": "1.1.1"
  }
}
Ousret commented 2 months ago

Yes. It appears to be a bug. If you are willing to, I'd accept a PR for it.

  if iterable_vs is not None:
      for v in iterable_vs:
          if v is not None:
              result.append(
                  (
                      k.encode("utf-8") if isinstance(k, str) else k,
                      v.encode("utf-8") if isinstance(v, str) else v,
                  )
              )

Adding if iterable_vs is not None: seems to fix this.

regards,

karosc commented 2 months ago

I'd opt to integrate the more general solution that is in requests, which just throws the object into a list if it's not iterable.

      if isinstance(vs, (str, bytes, int, float, bool))  or not hasattr(vs, "__iter__"):
          # not officially supported, but some people maybe passing ints, float or bool.
          if isinstance(vs, (str, bytes)) is False:
              iterable_vs = [str(vs)]
          else:
              iterable_vs = [vs]
      else:
          iterable_vs = vs
      for v in iterable_vs:
          if v is not None:
              result.append(
                  (
                      k.encode("utf-8") if isinstance(k, str) else k,
                      v.encode("utf-8") if isinstance(v, str) else v,
                  )
              )  )

or we could do this to avoid another if statement:

   if isinstance(vs, (str, bytes, int, float, bool, type(None)):
       # not officially supported, but some people maybe passing ints, float or bool.
       if isinstance(vs, (str, bytes)) is False:
           iterable_vs = [str(vs)]
       else:
           iterable_vs = [vs]
   else:
       iterable_vs = vs
   for v in iterable_vs:
       if v is not None:
           result.append(
               (
                   k.encode("utf-8") if isinstance(k, str) else k,
                   v.encode("utf-8") if isinstance(v, str) else v,
               )
           )  )
Ousret commented 2 months ago

I'd opt to integrate the more general solution that is in requests

the thing is[...] requests isn't typed and allowed things that made mypy unhappy (as far as I remember).

I am open to suggestions as long as:

regards,

karosc commented 2 months ago

Thanks @Ousret, I have a fix with mypy passing. I can also implement some unit tests for this bit too. Are you interested in setting mypy to run in CI? Or are the pre-commit hooks sufficient for you?

Ousret commented 2 months ago

Or are the pre-commit hooks sufficient for you?

The pre-commit includes mypy. So yes, it is sufficient.

Ousret commented 2 months ago

It is live.

pip install niquests -U