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

Accept bytes for Unix socket paths. #640

Merged
merged 7 commits into from
Nov 24, 2023

Conversation

Fuyukai
Copy link
Contributor

@Fuyukai Fuyukai commented Nov 23, 2023

*nix paths are an arbitrary sequence of bytes.

@agronholm
Copy link
Owner

While this is almost an annotations only patch, it does replace the use of str(). Perhaps there should be a test that creates a socket from a bytes-based path that doesn't cleanly translate to ASCII?

@Fuyukai
Copy link
Contributor Author

Fuyukai commented Nov 23, 2023

What would that actually be testing? It's just passed straight through to the underlying socket.

@agronholm
Copy link
Owner

It would test that a bytes-based socket path would not result in UnicodeDecodeError, as it could without this patch.

@agronholm
Copy link
Owner

It's even clearer now that I tried:

>>> Path(b"/tmp/foo2")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.12/pathlib.py", line 1163, in __init__
    super().__init__(*args)
  File "/usr/lib64/python3.12/pathlib.py", line 373, in __init__
    raise TypeError(
TypeError: argument should be a str or an os.PathLike object where __fspath__ returns a str, not 'bytes'

@Fuyukai Fuyukai changed the title Accept bytes for connect_unix. Accept bytes for Unix socket paths. Nov 23, 2023
@Fuyukai
Copy link
Contributor Author

Fuyukai commented Nov 23, 2023

Also added bytes supports for the other methods and added tests for each distinct one.

I had to duplicate a bit of logic from Pathlib around stat()ing.

src/anyio/_core/_sockets.py Outdated Show resolved Hide resolved
src/anyio/_core/_sockets.py Outdated Show resolved Hide resolved
src/anyio/_core/_sockets.py Outdated Show resolved Hide resolved
tests/test_sockets.py Outdated Show resolved Hide resolved
tests/test_sockets.py Outdated Show resolved Hide resolved
tests/test_sockets.py Outdated Show resolved Hide resolved
@agronholm
Copy link
Owner

agronholm commented Nov 24, 2023

Also please add a changelog entry in docs/versionhistory.rst (under **UNRELEASED**, as there are no changes yet since the last release).

Copy link
Owner

@agronholm agronholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, thanks!

@agronholm agronholm merged commit cc18942 into agronholm:master Nov 24, 2023
14 checks passed
@Fuyukai Fuyukai deleted the use-bytes-typehint branch November 24, 2023 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants