diff --git a/docs/changelog/709.bugfix.rst b/docs/changelog/709.bugfix.rst new file mode 100644 index 00000000..4af82458 --- /dev/null +++ b/docs/changelog/709.bugfix.rst @@ -0,0 +1,3 @@ +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 136bd614..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 @@ -905,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{socket.gethostname()}\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 b5080679..6cbfea5c 100644 --- a/tests/soft_rw/test_soft_rw_sync.py +++ b/tests/soft_rw/test_soft_rw_sync.py @@ -770,6 +770,35 @@ 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", + [pytest.param("host with space", id="space"), pytest.param("wörks", id="non-ascii")], +) +@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: + 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: + holder.release() + holder.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 diff --git a/tests/test_host_name_grammar.py b/tests/test_host_name_grammar.py new file mode 100644 index 00000000..ab91b0b9 --- /dev/null +++ b/tests/test_host_name_grammar.py @@ -0,0 +1,49 @@ +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.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: # pragma: needs hard-link + 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: