requests icon indicating copy to clipboard operation
requests copied to clipboard

`cert` parameter does not accept a single `pathlib.Path` object as argument

Open ghost opened this issue 3 years ago • 7 comments

requests seems to have special handling for when the cert parameter is a single str, but does not extend the same special handling to pathlib.Path, meaning currently you either have to do cert=str(path) or cert=(path, path).

Expected Result

Path objects should be handled just like str.

Actual Result

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/nyuszika7h/test/.venv/lib/python3.9/site-packages/requests/api.py", line 75, in get
    return request('get', url, params=params, **kwargs)
  File "/home/nyuszika7h/test/.venv/lib/python3.9/site-packages/requests/api.py", line 61, in request
    return session.request(method=method, url=url, **kwargs)
  File "/home/nyuszika7h/test/.venv/lib/python3.9/site-packages/requests/sessions.py", line 542, in request
    resp = self.send(prep, **send_kwargs)
  File "/home/nyuszika7h/test/.venv/lib/python3.9/site-packages/requests/sessions.py", line 655, in send
    r = adapter.send(request, **kwargs)
  File "/home/nyuszika7h/test/.venv/lib/python3.9/site-packages/requests/adapters.py", line 416, in send
    self.cert_verify(conn, request.url, verify, cert)
  File "/home/nyuszika7h/test/.venv/lib/python3.9/site-packages/requests/adapters.py", line 243, in cert_verify
    conn.cert_file = cert[0]
TypeError: 'PosixPath' object is not subscriptable

Reproduction Steps

from pathlib import Path

import requests

requests.get('https://example.com', cert=Path('cert.pem'))

System Information

$ python -m requests.help
{
  "chardet": {
    "version": null
  },
  "charset_normalizer": {
    "version": "2.0.5"
  },
  "cryptography": {
    "version": ""
  },
  "idna": {
    "version": "3.2"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.9.2"
  },
  "platform": {
    "release": "5.10.0-8-amd64",
    "system": "Linux"
  },
  "pyOpenSSL": {
    "openssl_version": "",
    "version": null
  },
  "requests": {
    "version": "2.26.0"
  },
  "system_ssl": {
    "version": "101010bf"
  },
  "urllib3": {
    "version": "1.26.6"
  },
  "using_charset_normalizer": true,
  "using_pyopenssl": false
}

ghost avatar Sep 16 '21 16:09 ghost

Hi @nyuszika7h, this seems like a reasonable request but is going to be somewhat difficult to implement prior to dropping 2.7 support. We don't intend to add backports as a dependency, so we'll leave this a todo for when the project finally moves to only Python 3 support. Thanks!

nateprewitt avatar Sep 16 '21 16:09 nateprewitt

I don't think this is entirely reasonable as pathlib is garabage. Expecting it to just work in places clearly documented as accepting either str or Tuple[str, str] isn't reasonable.

sigmavirus24 avatar Sep 17 '21 11:09 sigmavirus24

How exactly is it "garbage"? It has existed since Python 3.4 and is becoming a standard, take it up with the Python maintainers, not me. If you're developing a standalone program you have every right to not use pathlib if you don't like it, but developers of a major library used by thousands of other developers should consider supporting it regardless of their personal views. It doesn't really add any undue maintenance burden either.

tuple[Path, Path] already works fine with requests today, and if type hints are a concern, it can be done with an union type which is the officially recommended way if you need backwards compatibility with str. (In an ideal world, maybe we'd only accept Path objects, but that's not quite feasible yet – backwards compatibility is a concern and I would at least like p'' literals in that case – so a simple type alias can be created instead to keep function signatures simple.)

0xallie avatar Sep 17 '21 11:09 0xallie

It makes sense that it would be a good idea to allow some sort of path-like object, even without specifying examples of path-like object types with type hinting. If you look at https://docs.python.org/3/glossary.html#term-path-like-object and https://docs.python.org/3/library/os.html#os.PathLike, it shouldn't be unreasonable to check if an object is path-like.

The docs do mention here that os.PathLike is new in version 3.6. So, to preserve backwards compatibility we can check for hasattr(cert, '__fspath__') rather than checking isinstance(cert, os.PathLike). I recommend doing a similar check for if the object is subscriptable hasattr(cert, '__getitem__'), to prevent subscriptable-related TypeErrors that could otherwise be handled.

We can replace...

https://github.com/psf/requests/blob/e8a9bd7415986278fe41736a3c224232b9f98c39/requests/adapters.py#L241-L247

With...

if cert:
    if not isinstance(cert, basestring):
        # a subscriptable object
        if hasattr(cert, '__getitem__'):
            conn.cert_file = cert[0]
            conn.key_file = cert[1]
        elif hasattr(cert, '__fspath__'):
            # a path-like object implements __fspath__
            # see https://docs.python.org/3/library/os.html#os.PathLike
            conn.cert_file = cert.__fspath__()
        else:
            conn.cert_file = str(cert)
    else:
        conn.cert_file = cert
        conn.key_file = None

Though I've never used pathlib yet, if it is being mentioned in Python's stdlib docs and is becoming more standard, it may be good to start looking into backwards-compatible ways to support it, like in the example above. We can handle more types this way, and if errors occur, they should be because the string provided was an invalid path to a certificate, rather than because the type provided wasn't acceptable.

If os.path is allowing path-like objects, Requests should, too

steveberdy avatar Sep 30 '21 22:09 steveberdy

While requests still supports Python 2, it's probably better to use getattr() rather than hasattr() as the latter catches all exceptions, not just AttributeError. But hopefully in the next major release that won't be a problem anymore as Python 2 is EOL.

0xallie avatar Oct 01 '21 14:10 0xallie

Hi all,

Just combing through issues in some of my favorite python libs looking for ways to contribute.

Seems y'all have dropped support for Python 2 in the requests library, so this might be a great thing to implement at this time, unless there's something I'm not understanding fully. I'm looking for issues good for a GitHub newbie to tackle. And, while I would love to help with this, it seems @steveberdy has a solution in the notes above, so I wouldn't want to steal thunder by patching in code they've written.

Sorry for any misunderstandings and hope this helps. Thanks.

brianpatman avatar Feb 01 '23 03:02 brianpatman

This issue is still ongoing with v2.28.2. I think I'll make a PR since this doesn't exactly break the rules for the indefinite feature-freeze.

steveberdy avatar Feb 01 '23 04:02 steveberdy