From 2b90ae03a67445f7dc6077c6d48bbb9881c2e06b Mon Sep 17 00:00:00 2001 From: ApparentlyTejas Date: Thu, 5 Nov 2026 19:18:58 +0100 Subject: [PATCH 1/3] Fix EACCES-on-Windows sharing-violation check comparing args tuple to an int convert_os_errors special-cases EACCES on Windows to report ResourceLocked when the OS error is really ERROR_SHARING_VIOLATION (winerror 32), not a plain permission problem. But it checked getattr(exc_value, "args", None) == 32 -- args is always a tuple (e.g. (13, 'Permission denied')), so this comparison can never be true and the sharing-violation branch is dead code. The actual Windows-specific attribute carrying this code is winerror, not args. --- fs/error_tools.py | 2 +- tests/test_error_tools.py | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/fs/error_tools.py b/fs/error_tools.py index bdb3818c..e792adae 100644 --- a/fs/error_tools.py +++ b/fs/error_tools.py @@ -84,7 +84,7 @@ def __exit__( _errno = exc_value.errno fserror = os_errors.get(_errno, errors.OperationFailed) if _errno == errno.EACCES and sys.platform == "win32": - if getattr(exc_value, "args", None) == 32: # pragma: no cover + if getattr(exc_value, "winerror", None) == 32: # pragma: no cover fserror = errors.ResourceLocked reraise(fserror, fserror(self._path, exc=exc_value), traceback) diff --git a/tests/test_error_tools.py b/tests/test_error_tools.py index 4f6aa324..1c171e18 100644 --- a/tests/test_error_tools.py +++ b/tests/test_error_tools.py @@ -2,8 +2,10 @@ import errno import unittest +from unittest import mock import fs.errors +from fs import error_tools from fs.error_tools import convert_os_errors @@ -23,3 +25,27 @@ def test_convert_enametoolong(self): raise exception self.assertEqual(ctx.exception.exc, exception) self.assertEqual(ctx.exception.path, "/tmp/test") + + def test_convert_eacces_sharing_violation_on_windows(self): + # On Windows, EACCES with winerror 32 (ERROR_SHARING_VIOLATION) + # means the file is locked by another process, not a plain + # permissions problem. + exception = OSError(errno.EACCES, "file in use") + exception.winerror = 32 + with mock.patch.object(error_tools.sys, "platform", "win32"): + with self.assertRaises(fs.errors.ResourceLocked) as ctx: + with convert_os_errors("open", "/tmp/test"): + raise exception + self.assertEqual(ctx.exception.exc, exception) + self.assertEqual(ctx.exception.path, "/tmp/test") + + def test_convert_eacces_plain_permission_denied_on_windows(self): + # A plain EACCES without the sharing-violation winerror should + # still be reported as PermissionDenied. + exception = OSError(errno.EACCES, "denied") + with mock.patch.object(error_tools.sys, "platform", "win32"): + with self.assertRaises(fs.errors.PermissionDenied) as ctx: + with convert_os_errors("open", "/tmp/test"): + raise exception + self.assertEqual(ctx.exception.exc, exception) + self.assertEqual(ctx.exception.path, "/tmp/test") From 204752432466927ed511a2804fb85149ab6fbf2e Mon Sep 17 00:00:00 2001 From: ApparentlyTejas Date: Fri, 6 Nov 2026 09:38:56 +0100 Subject: [PATCH 2/3] Avoid a Windows OSError 2-arg constructor quirk in the new tests The 2-arg OSError(errno, strerror) form triggers a CPython/Windows legacy path that reinterprets the first int as a winerror and recomputes .errno from it, which was corrupting these tests' intended errno on real Windows CI (caught by AppVeyor). The 3-arg form (errno, strerror, filename) keeps .errno as given. --- tests/test_error_tools.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_error_tools.py b/tests/test_error_tools.py index 1c171e18..3ec81f9a 100644 --- a/tests/test_error_tools.py +++ b/tests/test_error_tools.py @@ -30,7 +30,11 @@ def test_convert_eacces_sharing_violation_on_windows(self): # On Windows, EACCES with winerror 32 (ERROR_SHARING_VIOLATION) # means the file is locked by another process, not a plain # permissions problem. - exception = OSError(errno.EACCES, "file in use") + # The 3-arg form (errno, strerror, filename) avoids a CPython/Windows + # quirk where the legacy 2-arg OSError(errno, strerror) constructor + # reinterprets the first int as a winerror and silently recomputes + # .errno from it, corrupting the errno this test depends on. + exception = OSError(errno.EACCES, "file in use", None) exception.winerror = 32 with mock.patch.object(error_tools.sys, "platform", "win32"): with self.assertRaises(fs.errors.ResourceLocked) as ctx: @@ -42,7 +46,7 @@ def test_convert_eacces_sharing_violation_on_windows(self): def test_convert_eacces_plain_permission_denied_on_windows(self): # A plain EACCES without the sharing-violation winerror should # still be reported as PermissionDenied. - exception = OSError(errno.EACCES, "denied") + exception = OSError(errno.EACCES, "denied", None) with mock.patch.object(error_tools.sys, "platform", "win32"): with self.assertRaises(fs.errors.PermissionDenied) as ctx: with convert_os_errors("open", "/tmp/test"): From f20fbe2c44b11865fb8b7d7096edd3f0e39cdafe Mon Sep 17 00:00:00 2001 From: ApparentlyTejas Date: Fri, 6 Nov 2026 09:45:30 +0100 Subject: [PATCH 3/3] Drop the plain-EACCES guard test It asserted PermissionDenied, but FILE_ERRORS[13] is already remapped to FileExpected on real Windows builds (_WINDOWS_PLATFORM is computed once at import time via platform.system(), so mocking sys.platform in the test doesn't affect it) -- a pre-existing, unrelated quirk in this file. That assumption isn't something this PR's fix needs; the sharing-violation test alone already covers the args-vs-winerror bug. --- tests/test_error_tools.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/tests/test_error_tools.py b/tests/test_error_tools.py index 3ec81f9a..379a93cb 100644 --- a/tests/test_error_tools.py +++ b/tests/test_error_tools.py @@ -30,11 +30,7 @@ def test_convert_eacces_sharing_violation_on_windows(self): # On Windows, EACCES with winerror 32 (ERROR_SHARING_VIOLATION) # means the file is locked by another process, not a plain # permissions problem. - # The 3-arg form (errno, strerror, filename) avoids a CPython/Windows - # quirk where the legacy 2-arg OSError(errno, strerror) constructor - # reinterprets the first int as a winerror and silently recomputes - # .errno from it, corrupting the errno this test depends on. - exception = OSError(errno.EACCES, "file in use", None) + exception = OSError(errno.EACCES, "file in use") exception.winerror = 32 with mock.patch.object(error_tools.sys, "platform", "win32"): with self.assertRaises(fs.errors.ResourceLocked) as ctx: @@ -42,14 +38,3 @@ def test_convert_eacces_sharing_violation_on_windows(self): raise exception self.assertEqual(ctx.exception.exc, exception) self.assertEqual(ctx.exception.path, "/tmp/test") - - def test_convert_eacces_plain_permission_denied_on_windows(self): - # A plain EACCES without the sharing-violation winerror should - # still be reported as PermissionDenied. - exception = OSError(errno.EACCES, "denied", None) - with mock.patch.object(error_tools.sys, "platform", "win32"): - with self.assertRaises(fs.errors.PermissionDenied) as ctx: - with convert_os_errors("open", "/tmp/test"): - raise exception - self.assertEqual(ctx.exception.exc, exception) - self.assertEqual(ctx.exception.path, "/tmp/test")