socket: accept a filesystem-encoded hostname in sethostname - #8437
Merged
youknowone merged 1 commit intoAug 3, 2026
Conversation
`socket.sethostname` took `PyUtf8StrRef`, so it rejected `bytes` outright and refused a `str` carrying a surrogate escape. `socketmodule.c socket_sethostname` accepts a bytes object directly, falls back to `PyUnicode_FSConverter` for anything else, and hands the syscall the resulting buffer and its length — the name is never required to be UTF-8. Take `FsPath`, the converter `if_nametoindex` in this same module already uses, and pass its bytes down. `host_env::socket::sethostname` correspondingly takes `&[u8]` and builds the `OsStr` from them; `nix::unistd::sethostname` accepts `AsRef<OsStr>` and passes pointer and length to the syscall, so nothing on the path needs a NUL terminator or valid UTF-8. `Lib/test/test_socket.py test_sethostname` covers this: it calls `socket.sethostname(b'bar')` and asserts the hostname changed. The test is skipped unless run as root, which is why the gap went unnoticed. Assisted-by: Claude
Contributor
📝 WalkthroughWalkthroughThe socket API now accepts filesystem-encoded hostname bytes. The standard library passes ChangesHostname encoding
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates RustPython’s socket.sethostname to accept filesystem-encoded hostnames (including bytes and str values containing surrogate escapes), aligning more closely with CPython’s socket_sethostname behavior by passing raw bytes down to the OS.
Changes:
- Change
_socket.sethostnameto acceptFsPathinstead ofPyUtf8StrRef, enablingbytesand surrogate-containingstrinputs. - Update
host_env::socket::sethostnameto take a&[u8]and build anOsStrfrom bytes before callingnix::unistd::sethostname.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
crates/stdlib/src/socket.rs |
Switches sethostname to accept FsPath and forwards bytes to the host syscall wrapper. |
crates/host_env/src/socket.rs |
Changes host wrapper to accept raw bytes and pass them to nix::unistd::sethostname via OsStr::from_bytes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2328
to
2330
| fn sethostname(hostname: FsPath) -> std::io::Result<()> { | ||
| host_socket::sethostname(hostname.as_bytes()) | ||
| } |
Comment on lines
+2328
to
+2329
| fn sethostname(hostname: FsPath) -> std::io::Result<()> { | ||
| host_socket::sethostname(hostname.as_bytes()) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/stdlib/src/socket.rs`:
- Around line 2328-2329: Update sethostname to obtain the hostname bytes through
hostname.as_os_str(vm)? before calling host_socket::sethostname, ensuring
filesystem encoding handles surrogate strings. Add a root-gated regression test
covering a surrogate-string hostname in addition to the existing ASCII string
test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 345e6549-abd0-4042-82e9-7a13484ee10f
📒 Files selected for processing (2)
crates/host_env/src/socket.rscrates/stdlib/src/socket.rs
Comment on lines
+2328
to
+2329
| fn sethostname(hostname: FsPath) -> std::io::Result<()> { | ||
| host_socket::sethostname(hostname.as_bytes()) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
git ls-files | rg '(^|/)socket\.rs$|fspath\.rs$|fs\.rs$|path\.rs$|sethostname|host_socket|host_env' || true
echo "== socket.rs outline around sethostname =="
if [ -f crates/stdlib/src/socket.rs ]; then
wc -l crates/stdlib/src/socket.rs
rg -n "sethostname|FsPath|as_bytes|as_os_str|#\\[pyfunction\\]" crates/stdlib/src/socket.rs
sed -n '2290,2345p' crates/stdlib/src/socket.rs
fi
echo "== fspath implementation outline/search =="
for f in $(git ls-files | rg 'fspath\.rs$'); do
echo "--- $f"
wc -l "$f"
rg -n "struct FsPath|enum Fspath|impl .*FsPath|as_bytes|as_os_str|fsencode|surrog" "$f"
sed -n '1,260p' "$f"
done
echo "== search sethostname in codebase =="
rg -n "fn sethostname|host_socket::sethostname|host_env::sethostname|PyUnicode_FSConverter|sethostname" .Repository: RustPython/RustPython
Length of output: 12842
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== host_env socket sethostname =="
sed -n '1,90p' crates/host_env/src/socket.rs
echo "== fsencode implementation search =="
rg -n "fn fsencode|fsencode\\(" crates/vm/src crates/host_env/src | head -80
for f in $(git ls-files | rg 'crates/(vm|host_env)/src/.*'); do
if rg -q "fn fsencode|fsencode\\(" "$f"; then
echo "--- $f"
rg -n -A 40 -B 5 "fn fsencode|fsencode\\(" "$f"
fi
done
echo "== existing socket hostname tests =="
sed -n '1130,1180p' Lib/test/test_socket.py 2>/dev/null || true
echo "== sethostname call context in stdlib =="
sed -n '2200,2335p' crates/stdlib/src/socket.rs
echo "== import/return type context for sethostname =="
sed -n '1,35p' crates/stdlib/src/socket.rsRepository: RustPython/RustPython
Length of output: 20484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== encode_text implementation candidates =="
rg -n "encode_text|fs_encode_errors|codec_registry|FS_ERRORS|surrogate" crates/vm/src crates/host_env/src | head -120
echo "== codec registry outline/search =="
rg -n "struct CodecRegistry|fn encode_text|encode_text" crates/vm/src | head -80
for f in $(git ls-files | rg 'crates/vm/src/.*'); do
if rg -q "encode_text\\(" "$f"; then
echo "--- $f"
rg -n -A 80 -B 5 "encode_text\\(|struct CodecRegistry|codec_registry" "$f"
fi
done
echo "== fs decode/encode tests =="
rg -n "fsencode|fsdecode|surrogates|\\udc[a-f0-9][0-9a-f]{2}|\\\\udcf" crates/stdlib Lib test crates/vm | head -160 | sed -n '1,180p'Repository: RustPython/RustPython
Length of output: 36215
Use filesystem encoding before forwarding hostname.
FsPath::as_bytes() returns raw string bytes for FsPath::Str, and host_socket::sethostname forwards those bytes directly to sethostname. A surrogate string such as "\udcff" therefore bypasses vm.fsencode; use hostname.as_os_str(vm)? and pass its Unix OsStr bytes to host_socket::sethostname. Add a root-gated regression test for the surrogate-string branch; the current string test only covers ASCII.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/stdlib/src/socket.rs` around lines 2328 - 2329, Update sethostname to
obtain the hostname bytes through hostname.as_os_str(vm)? before calling
host_socket::sethostname, ensuring filesystem encoding handles surrogate
strings. Add a root-gated regression test covering a surrogate-string hostname
in addition to the existing ASCII string test.
Source: MCP tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
socket.sethostnametookPyUtf8StrRef, so it rejectedbytesoutright andrefused a
strcarrying a surrogate escape.socketmodule.c socket_sethostnameaccepts a bytes object directly ("S"),falls back to
PyUnicode_FSConverterfor anything else, and hands the syscallthe resulting buffer together with its length — the name is never required to be
UTF-8.
This takes
FsPath, the converterif_nametoindexin this same module alreadyuses, and passes its bytes down.
host_env::socket::sethostnamecorrespondinglytakes
&[u8]and builds theOsStrfrom them;nix::unistd::sethostnameacceptsAsRef<OsStr>and passes pointer and length to the syscall, so nothing on the pathneeds a NUL terminator or valid UTF-8.
Lib/test/test_socket.py test_sethostnamealready covers this — it callssocket.sethostname(b'bar')and asserts the hostname changed. The test is skippedunless run as root, which is why the gap went unnoticed.
Verification
Built both ways and called
sethostnameas a non-root user, where every acceptedargument reaches the syscall and comes back
EPERM. That makes the exception typethe discriminator:
PermissionErrormeans the argument was converted and reachedthe OS, anything else means it was rejected before that.
'bar'PermissionErrorPermissionErrorPermissionErrorb'bar'TypeError: Expected type 'str' but 'bytes' found.PermissionErrorPermissionError'x\udcff'UnicodeEncodeError: 'utf-8' codec can't encode character '\udcff'PermissionErrorPermissionErrorThe hostname itself is never changed by this check, so it is safe to run anywhere.
Summary by CodeRabbit