From fe6b7a8d17844e8d086f47744f3776a5af96fd83 Mon Sep 17 00:00:00 2001 From: Javid Khan Date: Mon, 17 Aug 2026 19:01:47 +0530 Subject: [PATCH 1/5] keep a soft read/write marker's hostname within the reader grammar a host whose socket.gethostname() carries a space or a non-ascii byte made the writer publish a marker its own heartbeat could not parse, so the heartbeat stopped and a peer could evict the still-held marker; a non-ascii name made the ascii encode raise and the acquire fail. normalise the hostname where the marker is written so it always parses back. --- src/filelock/_soft_rw/_sync.py | 15 +++++++++++++- tests/soft_rw/test_soft_rw_sync.py | 33 ++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/filelock/_soft_rw/_sync.py b/src/filelock/_soft_rw/_sync.py index 136bd614..8263e934 100644 --- a/src/filelock/_soft_rw/_sync.py +++ b/src/filelock/_soft_rw/_sync.py @@ -891,6 +891,19 @@ def _break_stale_marker( # ruff:ignore[too-many-return-statements] # each retu return True +def _marker_hostname() -> str: + # _parse_marker_bytes accepts a hostname of 1..253 printable non-space ASCII bytes (the RFC 1123 grammar) and reads + # anything else as a malformed marker a peer may evict. socket.gethostname() is not bound to that grammar: a kernel + # hostname may carry a space or a non-ASCII byte, or run past 253 bytes. Writing it verbatim makes the holder + # publish a marker its own heartbeat cannot parse, so _refresh_marker stops the heartbeat on the first tick and a + # peer evicts the still-held marker as stale (the space case), or _atomic_create_marker aborts with + # UnicodeEncodeError before the marker exists (the non-ASCII case). The field is stored for diagnostics only; + # nothing here reads it back. Fold every out-of-grammar byte to '?' so the writer stays inside the reader's grammar + # and a lock is never lost to its own hostname. + hostname = "".join(char if "\x21" <= char <= "\x7e" else "?" for char in socket.gethostname()) + return hostname[:253] or "?" + + def _atomic_create_marker(name: str, token: str, *, dir_fd: int | None = None) -> None: # O_NOFOLLOW blocks the symlink-overwrite attack where an attacker pre-creates the marker path as a # symlink pointing at a victim file. Mode 0o600 keeps the token unreadable to other users. @@ -905,7 +918,7 @@ def _atomic_create_marker(name: str, token: str, *, dir_fd: int | None = None) - try: st = os.fstat(fd) identity = st.st_dev, st.st_ino - write_all(fd, f"{token}\n{os.getpid()}\n{socket.gethostname()}\n".encode("ascii")) + write_all(fd, f"{token}\n{os.getpid()}\n{_marker_hostname()}\n".encode("ascii")) except BaseException: os.close(fd) if identity is not None and _same_file(name, identity, dir_fd=dir_fd): diff --git a/tests/soft_rw/test_soft_rw_sync.py b/tests/soft_rw/test_soft_rw_sync.py index b5080679..8309bffb 100644 --- a/tests/soft_rw/test_soft_rw_sync.py +++ b/tests/soft_rw/test_soft_rw_sync.py @@ -770,6 +770,39 @@ def test_stale_malformed_marker_is_evicted(lock_file: str, content: bytes) -> No lock.close() +@pytest.mark.parametrize( + ("raw", "expected"), + [ + ("clean-host.example", "clean-host.example"), + ("host with space", "host?with?space"), + ("wörks", "w?rks"), + ("x" * 300, "x" * 253), + ("", "?"), + ], +) +def test_marker_hostname_stays_within_reader_grammar(raw: str, expected: str, mocker: MockerFixture) -> None: + mocker.patch.object(sync_mod.socket, "gethostname", return_value=raw) + hostname = sync_mod._marker_hostname() + assert hostname == expected + # the normalized hostname must parse cleanly where the raw one would be read as a malformed marker + assert sync_mod._parse_marker_bytes(f"{'0' * 32}\n4711\n{hostname}\n".encode("ascii")) is not None + + +@pytest.mark.parametrize("raw", ["host with space", "wörks"]) +def test_unusual_hostname_marker_is_self_refreshable(lock_file: str, raw: str, mocker: MockerFixture) -> None: + # A host whose socket.gethostname() falls outside the marker grammar (a space, a non-ASCII byte) must still + # publish a marker its own heartbeat can parse. Otherwise the space case stops the heartbeat and lets a peer + # evict a still-held lock, and the non-ASCII case aborts the acquire outright with UnicodeEncodeError. + mocker.patch.object(sync_mod.socket, "gethostname", return_value=raw) + lock = _make_lock(lock_file, heartbeat_interval=30, stale_threshold=90) + lock.acquire_write(timeout=2) + try: + assert lock._refresh_marker() is True + finally: + lock.release(force=True) + lock.close() + + def test_fifo_write_marker_does_not_block(lock_file: str) -> None: # pragma: needs fifo if sys.platform == "win32" or not CAPABILITIES["fifo"]: # pragma: win32 cover pytest.skip("os.mkfifo is unavailable") # the platform arm also narrows so ty resolves os.mkfifo below From 474eba90db0646190915569ccf6691fbf4f84174 Mon Sep 17 00:00:00 2001 From: Javid Khan Date: Mon, 17 Aug 2026 19:02:48 +0530 Subject: [PATCH 2/5] add news fragment for the marker hostname fix --- docs/changelog/709.bugfix.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 docs/changelog/709.bugfix.rst diff --git a/docs/changelog/709.bugfix.rst b/docs/changelog/709.bugfix.rst new file mode 100644 index 00000000..e30dda00 --- /dev/null +++ b/docs/changelog/709.bugfix.rst @@ -0,0 +1,4 @@ +``SoftReadWriteLock`` and ``AsyncSoftReadWriteLock`` now normalise the hostname written into a marker so a host whose +``socket.gethostname()`` carries a space or a non-ASCII byte no longer publishes a marker its own heartbeat parses as +malformed. Previously such a marker stopped the heartbeat on its first refresh and let a peer evict a still-held lock, +and a non-ASCII hostname failed the acquire outright with ``UnicodeEncodeError``. From 196f47fc4e9539e0ad2562bb8d8dec269798b4e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Sat, 22 Aug 2026 15:11:03 -0700 Subject: [PATCH 3/5] escape the hostname every marker publishes The soft read/write writer was not the only one bound by a grammar its own reader enforces. A hostname carrying a space also fails the StrictSoftFileLock claim validator, and one carrying a byte outside UTF-8 (which Python hands back as a surrogate) fails every writer with UnicodeEncodeError, the soft read/write state lock included. A newline forges an extra line in a protocol 1 marker, so its own holder no longer recognizes it. Escape in host_name() instead, where all four writers read the hostname, and drop the soft read/write copy. '?' is illegal in a hostname, so escaping it alongside every out-of-grammar byte as '?' leaves a real hostname untouched and keeps two hosts distinct, which owner_is_stale needs to refuse to probe a foreign PID. --- docs/changelog/709.bugfix.rst | 7 ++--- docs/how-to.rst | 4 +++ src/filelock/_identity.py | 25 +++++++++++++-- src/filelock/_soft_rw/_sync.py | 17 ++-------- tests/soft_rw/test_soft_rw_sync.py | 50 ++++++++++++++---------------- tests/test_host_name_grammar.py | 48 ++++++++++++++++++++++++++++ tests/test_process_identity.py | 20 ++++++++++-- 7 files changed, 120 insertions(+), 51 deletions(-) create mode 100644 tests/test_host_name_grammar.py diff --git a/docs/changelog/709.bugfix.rst b/docs/changelog/709.bugfix.rst index e30dda00..4af82458 100644 --- a/docs/changelog/709.bugfix.rst +++ b/docs/changelog/709.bugfix.rst @@ -1,4 +1,3 @@ -``SoftReadWriteLock`` and ``AsyncSoftReadWriteLock`` now normalise the hostname written into a marker so a host whose -``socket.gethostname()`` carries a space or a non-ASCII byte no longer publishes a marker its own heartbeat parses as -malformed. Previously such a marker stopped the heartbeat on its first refresh and let a peer evict a still-held lock, -and a non-ASCII hostname failed the acquire outright with ``UnicodeEncodeError``. +Every lock class now escapes the hostname it publishes, so a host whose ``socket.gethostname()`` carries a space, a +newline or a byte outside UTF-8 no longer writes a marker it reads back as malformed. Such a host used to lose a held +``SoftReadWriteLock`` read slot to a peer and could not take a write slot or a ``StrictSoftFileLock`` at all. diff --git a/docs/how-to.rst b/docs/how-to.rst index 76ee7fc0..c1f556c0 100644 --- a/docs/how-to.rst +++ b/docs/how-to.rst @@ -877,6 +877,10 @@ returns an :class:`OwnerRecord `: print(owner.lease_duration) # 30.0 print(owner.start) # process start token, or None where unavailable +Every record holds ``socket.gethostname()`` with each byte outside printable non-space ASCII, and each ``?``, +escaped as ``?``, since a marker line cannot carry a space, a newline, or a byte no codec encodes. A +conventional hostname reaches the record unchanged, so an escape names a host whose kernel hostname is not one. + :class:`StrictSoftFileLock ` does **not** publish this record and has no ``owner``: it derives from :class:`BaseFileLock `, keeps a permanent sentinel at the lock path, and stores one record per owner under ``work.lock.filelock/claims``. Read those through diff --git a/src/filelock/_identity.py b/src/filelock/_identity.py index fe1b03d4..dbc5ca18 100644 --- a/src/filelock/_identity.py +++ b/src/filelock/_identity.py @@ -7,10 +7,31 @@ from pathlib import Path from typing import Final +#: Every marker format in this package caps the hostname at the RFC 1123 limit and reads a longer one as malformed. +_HOST_NAME_LIMIT: Final[int] = 253 +#: The bytes those formats carry verbatim: printable non-space ASCII, less the ``?`` reserved for the escape. +_HOST_NAME_VERBATIM: Final[frozenset[int]] = frozenset(range(0x21, 0x7F)) - {ord("?")} + def host_name() -> str: - """The hostname recorded alongside an owner, so a marker written on another machine is never probed here.""" - return socket.gethostname() + """ + The hostname recorded alongside an owner, so a marker written on another machine is never probed here. + + Marker formats here hold the field to printable non-space ASCII, while ``socket.gethostname()`` reports whatever + the kernel stores: a space, a newline that forges an extra marker line, or a byte no codec encodes, which Python + hands back as a surrogate. A holder that publishes one raw writes a marker it cannot read back, and loses the lock + to the first peer that ages it out as malformed. ``?`` is illegal in a hostname, so escaping it along with every + out-of-grammar byte as ``?`` leaves a real name untouched and still keeps two hosts apart, which + :func:`owner_is_stale` relies on to refuse to probe a foreign PID. + """ + name = "" + for byte in socket.gethostname().encode("utf-8", "surrogateescape"): + piece = chr(byte) if byte in _HOST_NAME_VERBATIM else f"?{byte:02x}" + if len(name) + len(piece) > _HOST_NAME_LIMIT: + break + name += piece + # An escape is three characters and a kept byte is never '?', so a bare '?' can only mean an empty hostname. + return name or "?" def owner_is_stale(pid: int, hostname: str, start_token: int | None) -> bool: diff --git a/src/filelock/_soft_rw/_sync.py b/src/filelock/_soft_rw/_sync.py index 8263e934..9d8ba019 100644 --- a/src/filelock/_soft_rw/_sync.py +++ b/src/filelock/_soft_rw/_sync.py @@ -7,7 +7,6 @@ import os import re import secrets -import socket import stat import sys import threading @@ -30,6 +29,7 @@ _unregister_owned_descriptor, ) from filelock._error import Timeout +from filelock._identity import host_name from filelock._soft import SoftFileLock from filelock._util import ensure_directory_exists, touch, write_all @@ -891,19 +891,6 @@ def _break_stale_marker( # ruff:ignore[too-many-return-statements] # each retu return True -def _marker_hostname() -> str: - # _parse_marker_bytes accepts a hostname of 1..253 printable non-space ASCII bytes (the RFC 1123 grammar) and reads - # anything else as a malformed marker a peer may evict. socket.gethostname() is not bound to that grammar: a kernel - # hostname may carry a space or a non-ASCII byte, or run past 253 bytes. Writing it verbatim makes the holder - # publish a marker its own heartbeat cannot parse, so _refresh_marker stops the heartbeat on the first tick and a - # peer evicts the still-held marker as stale (the space case), or _atomic_create_marker aborts with - # UnicodeEncodeError before the marker exists (the non-ASCII case). The field is stored for diagnostics only; - # nothing here reads it back. Fold every out-of-grammar byte to '?' so the writer stays inside the reader's grammar - # and a lock is never lost to its own hostname. - hostname = "".join(char if "\x21" <= char <= "\x7e" else "?" for char in socket.gethostname()) - return hostname[:253] or "?" - - def _atomic_create_marker(name: str, token: str, *, dir_fd: int | None = None) -> None: # O_NOFOLLOW blocks the symlink-overwrite attack where an attacker pre-creates the marker path as a # symlink pointing at a victim file. Mode 0o600 keeps the token unreadable to other users. @@ -918,7 +905,7 @@ def _atomic_create_marker(name: str, token: str, *, dir_fd: int | None = None) - try: st = os.fstat(fd) identity = st.st_dev, st.st_ino - write_all(fd, f"{token}\n{os.getpid()}\n{_marker_hostname()}\n".encode("ascii")) + write_all(fd, f"{token}\n{os.getpid()}\n{host_name()}\n".encode("ascii")) except BaseException: os.close(fd) if identity is not None and _same_file(name, identity, dir_fd=dir_fd): diff --git a/tests/soft_rw/test_soft_rw_sync.py b/tests/soft_rw/test_soft_rw_sync.py index 8309bffb..6cbfea5c 100644 --- a/tests/soft_rw/test_soft_rw_sync.py +++ b/tests/soft_rw/test_soft_rw_sync.py @@ -770,37 +770,33 @@ def test_stale_malformed_marker_is_evicted(lock_file: str, content: bytes) -> No lock.close() +@pytest.mark.parametrize("mode", [pytest.param("write", id="write"), pytest.param("read", id="read")]) @pytest.mark.parametrize( - ("raw", "expected"), - [ - ("clean-host.example", "clean-host.example"), - ("host with space", "host?with?space"), - ("wörks", "w?rks"), - ("x" * 300, "x" * 253), - ("", "?"), - ], + "raw", + [pytest.param("host with space", id="space"), pytest.param("wörks", id="non-ascii")], ) -def test_marker_hostname_stays_within_reader_grammar(raw: str, expected: str, mocker: MockerFixture) -> None: - mocker.patch.object(sync_mod.socket, "gethostname", return_value=raw) - hostname = sync_mod._marker_hostname() - assert hostname == expected - # the normalized hostname must parse cleanly where the raw one would be read as a malformed marker - assert sync_mod._parse_marker_bytes(f"{'0' * 32}\n4711\n{hostname}\n".encode("ascii")) is not None - - -@pytest.mark.parametrize("raw", ["host with space", "wörks"]) -def test_unusual_hostname_marker_is_self_refreshable(lock_file: str, raw: str, mocker: MockerFixture) -> None: - # A host whose socket.gethostname() falls outside the marker grammar (a space, a non-ASCII byte) must still - # publish a marker its own heartbeat can parse. Otherwise the space case stops the heartbeat and lets a peer - # evict a still-held lock, and the non-ASCII case aborts the acquire outright with UnicodeEncodeError. - mocker.patch.object(sync_mod.socket, "gethostname", return_value=raw) - lock = _make_lock(lock_file, heartbeat_interval=30, stale_threshold=90) - lock.acquire_write(timeout=2) +@pytest.mark.timeout(10) +def test_out_of_grammar_hostname_keeps_the_slot_held( + lock_file: str, mocker: MockerFixture, raw: str, mode: Literal["read", "write"] +) -> None: + # A kernel hostname outside the marker grammar used to reach the marker verbatim, so a holder published a marker + # its own heartbeat read as malformed. A write slot then never claimed at all, and a read slot aged out under its + # live reader and fell to the next contender. + mocker.patch("filelock._identity.socket.gethostname", return_value=raw) + holder = _make_lock(lock_file) + acquire = holder.acquire_write if mode == "write" else holder.acquire_read + acquire(timeout=2) try: - assert lock._refresh_marker() is True + contender = _make_lock(lock_file) + try: + # longer than the stale threshold, so only a heartbeat that reads its own marker keeps the slot + with pytest.raises(Timeout): + contender.acquire_write(timeout=1) + finally: + contender.close() finally: - lock.release(force=True) - lock.close() + holder.release() + holder.close() def test_fifo_write_marker_does_not_block(lock_file: str) -> None: # pragma: needs fifo diff --git a/tests/test_host_name_grammar.py b/tests/test_host_name_grammar.py new file mode 100644 index 00000000..135a328d --- /dev/null +++ b/tests/test_host_name_grammar.py @@ -0,0 +1,48 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest + +from filelock import SoftFileLease, SoftFileLock, StrictSoftFileLock +from filelock._identity import host_name + +if TYPE_CHECKING: + from pathlib import Path + + from pytest_mock import MockerFixture + + +@pytest.fixture( + params=[ + pytest.param("host with space", id="space"), + pytest.param("host\nname", id="newline"), + pytest.param("wörks", id="non-ascii"), + pytest.param("b\udcffd", id="undecodable-byte"), + ] +) +def out_of_grammar_host(request: pytest.FixtureRequest, mocker: MockerFixture) -> None: + # Kernel hostnames that every marker format used to publish verbatim and then read back as malformed. The + # surrogate is how Python surfaces a hostname that is not valid UTF-8. + mocker.patch("filelock._identity.socket.gethostname", return_value=request.param) + + +@pytest.mark.usefixtures("out_of_grammar_host") +def test_soft_file_lock_recognizes_its_own_marker(tmp_path: Path) -> None: + with SoftFileLock(tmp_path / "resource.lock", timeout=2) as lock: + assert lock.is_lock_held_by_us is True + + +@pytest.mark.usefixtures("out_of_grammar_host") +def test_strict_soft_file_lock_reads_back_its_own_claim(tmp_path: Path) -> None: + with StrictSoftFileLock(tmp_path / "resource.lock", timeout=2) as lock: + assert {claim.hostname for claim in lock.claims} == {host_name()} + + +@pytest.mark.usefixtures("out_of_grammar_host") +def test_soft_file_lease_reads_back_its_own_owner(tmp_path: Path) -> None: + lease = SoftFileLease(str(tmp_path / "resource.lock"), timeout=2, lease_duration=30, heartbeat_interval=1) + with lease: + owner = lease.owner + assert owner is not None + assert owner.hostname == host_name() diff --git a/tests/test_process_identity.py b/tests/test_process_identity.py index 4db29ade..74744a0d 100644 --- a/tests/test_process_identity.py +++ b/tests/test_process_identity.py @@ -1,7 +1,6 @@ from __future__ import annotations import os -import socket import sys from errno import ENODEV, EPERM from typing import TYPE_CHECKING, Final @@ -21,8 +20,23 @@ ) -def test_host_name_matches_socket() -> None: - assert host_name() == socket.gethostname() +@pytest.mark.parametrize( + ("raw", "expected"), + [ + pytest.param("build-01.example.com", "build-01.example.com", id="plain"), + pytest.param("build 01", "build?2001", id="space"), + pytest.param("build\n01", "build?0a01", id="newline"), + pytest.param("wörks", "w?c3?b6rks", id="non-ascii"), + pytest.param("who?", "who?3f", id="escape-character"), + pytest.param("b\udcffd", "b?ffd", id="undecodable-byte"), + pytest.param("x" * 300, "x" * 253, id="over-long"), + pytest.param("ä" * 200, "?c3?a4" * 42, id="over-long-escaped"), + pytest.param("", "?", id="empty"), + ], +) +def test_host_name_escapes_out_of_grammar_bytes(raw: str, expected: str, mocker: MockerFixture) -> None: + mocker.patch("filelock._identity.socket.gethostname", return_value=raw) + assert host_name() == expected def test_process_alive_true_for_self() -> None: From 798fe90a11b3c9d6149f6f398ba7bd74a542bfec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Sat, 22 Aug 2026 15:19:32 -0700 Subject: [PATCH 4/5] skip the strict claim case where os.link is absent --- tests/test_host_name_grammar.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_host_name_grammar.py b/tests/test_host_name_grammar.py index 135a328d..09d3726e 100644 --- a/tests/test_host_name_grammar.py +++ b/tests/test_host_name_grammar.py @@ -33,6 +33,7 @@ def test_soft_file_lock_recognizes_its_own_marker(tmp_path: Path) -> None: assert lock.is_lock_held_by_us is True +@pytest.mark.requires_hard_links @pytest.mark.usefixtures("out_of_grammar_host") def test_strict_soft_file_lock_reads_back_its_own_claim(tmp_path: Path) -> None: with StrictSoftFileLock(tmp_path / "resource.lock", timeout=2) as lock: From 87a6b3ce10ffad0a35b61e8153604584d79805af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Sat, 22 Aug 2026 15:33:06 -0700 Subject: [PATCH 5/5] exclude the strict claim case where hard links are absent --- tests/test_host_name_grammar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_host_name_grammar.py b/tests/test_host_name_grammar.py index 09d3726e..ab91b0b9 100644 --- a/tests/test_host_name_grammar.py +++ b/tests/test_host_name_grammar.py @@ -35,7 +35,7 @@ def test_soft_file_lock_recognizes_its_own_marker(tmp_path: Path) -> None: @pytest.mark.requires_hard_links @pytest.mark.usefixtures("out_of_grammar_host") -def test_strict_soft_file_lock_reads_back_its_own_claim(tmp_path: Path) -> None: +def test_strict_soft_file_lock_reads_back_its_own_claim(tmp_path: Path) -> None: # pragma: needs hard-link with StrictSoftFileLock(tmp_path / "resource.lock", timeout=2) as lock: assert {claim.hostname for claim in lock.claims} == {host_name()}