Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#34294 closed Bug (fixed)

File locking fails if argtypes redefined on Windows.

Reported by: Simon Sawicki Owned by: Simon Sawicki
Component: File uploads/storage Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Simon Sawicki)

django.core.files.locks uses ctypes.windll.kernel32 which is a global instance. If other code redefines windll.kernel32.LockFileEx.argtypes the function fails with an error.

import ctypes
import ctypes.wintypes

from django.core.files import locks


class OVERLAPPED(ctypes.Structure):
    _fields_ = [
        ("Internal", ctypes.wintypes.LPVOID),
        ("InternalHigh", ctypes.wintypes.LPVOID),
        ("Offset", ctypes.wintypes.DWORD),
        ("OffsetHigh", ctypes.wintypes.DWORD),
        ("hEvent", ctypes.wintypes.HANDLE),
    ]


ctypes.windll.kernel32.LockFileEx.argtypes = [
    ctypes.wintypes.HANDLE,
    ctypes.wintypes.DWORD,
    ctypes.wintypes.DWORD,
    ctypes.wintypes.DWORD,
    ctypes.wintypes.DWORD,
    ctypes.wintypes.DWORD,
]

with open("./file", "w") as f:
    locks.lock(f, locks.LOCK_EX)

Possible patch:

  • django/core/files/locks.py

    diff --git a/django/core/files/locks.py b/django/core/files/locks.py
    index 5752b08a69..c0f471f87d 100644
    a b if os.name == "nt":  
    3232        POINTER,
    3333        Structure,
    3434        Union,
     35        WinDLL,
    3536        byref,
    3637        c_int64,
    3738        c_ulong,
    3839        c_void_p,
    3940        sizeof,
    40         windll,
    4141    )
    4242    from ctypes.wintypes import BOOL, DWORD, HANDLE
    4343
    if os.name == "nt":  
    7373    LPOVERLAPPED = POINTER(OVERLAPPED)
    7474
    7575    # --- Define function prototypes for extra safety ---
    76     LockFileEx = windll.kernel32.LockFileEx
     76    kernel32 = WinDLL("kernel32")
     77    LockFileEx = kernel32.LockFileEx
    7778    LockFileEx.restype = BOOL
    7879    LockFileEx.argtypes = [HANDLE, DWORD, DWORD, DWORD, DWORD, LPOVERLAPPED]
    79     UnlockFileEx = windll.kernel32.UnlockFileEx
     80    UnlockFileEx = kernel32.UnlockFileEx
    8081    UnlockFileEx.restype = BOOL
    8182    UnlockFileEx.argtypes = [HANDLE, DWORD, DWORD, DWORD, LPOVERLAPPED]
    8283
  • tests/files/tests.py

    diff --git a/tests/files/tests.py b/tests/files/tests.py
    index b3478d2732..d0e633cb7a 100644
    a b class FileTests(unittest.TestCase):  
    200200            self.assertIs(locks.unlock(f1), True)
    201201            self.assertIs(locks.unlock(f2), True)
    202202
     203    @unittest.skipIf(os.name != "nt", "File locking usees WinDLL on windows only")     
     204    def test_local_windll(self):
     205        import ctypes
     206        import ctypes.wintypes
     207
     208        old_argtypes = ctypes.windll.kernel32.LockFileEx.argtypes
     209        try:
     210            ctypes.windll.kernel32.LockFileEx.argtypes = [
     211                ctypes.wintypes.HANDLE,
     212                ctypes.wintypes.DWORD,
     213                ctypes.wintypes.DWORD,
     214                ctypes.wintypes.DWORD,
     215                ctypes.wintypes.DWORD,
     216                ctypes.wintypes.DWORD,
     217            ]
     218            file_path = Path(__file__).parent / "test.png"
     219            with open(file_path) as file:
     220                try:
     221                    locks.lock(file, locks.LOCK_EX)
     222                except Exception:
     223                    self.fail("Locking raised exception with changed argtypes")
     224                else:
     225                    locks.unlock(file)
     226        finally:
     227            ctypes.windll.kernel32.LockFileEx.argtypes = old_argtypes
     228
    203229
    204230class NoNameFileTestCase(unittest.TestCase):
    205231    """

Change History (8)

comment:1 by Simon Sawicki, 2 years ago

Description: modified (diff)

comment:2 by Mariusz Felisiak, 2 years ago

Component: Core (Other)File uploads/storage
Has patch: unset
Owner: changed from nobody to Simon Sawicki
Status: newassigned
Summary: File locking fails if argtypes redefinedFile locking fails if argtypes redefined on Windows.
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Thanks for this ticket. locks is an undocumented internal API, but I agree it's worth improving. At the same time, I'm not sure if a pathological and rare case as this one is worth extra tests. I'd accept PR without tests.

comment:3 by Simon Sawicki, 2 years ago

This caused https://github.com/ytdl-org/youtube-dl/issues/21545 for example, since yt-dl (possibly also other projects) also mutates that global instance. I will open a PR asap, thank you for the quick triage!

comment:4 by Simon Sawicki, 2 years ago

Has patch: set

Relevant PR.

comment:5 by Keryn Knight, 2 years ago

For historic cross-referencing purposes, the aforementioned youtube-dl issue ultimately ended up tracked as #30603.

comment:6 by GitHub <noreply@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In 7eb5391b:

Fixed #34294 -- Protected django.core.files.locks against argtypes redefinition on Windows.

comment:7 by Mariusz Felisiak, 2 years ago

Type: Cleanup/optimizationBug

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 9eae817:

[4.2.x] Fixed #34294 -- Protected django.core.files.locks against argtypes redefinition on Windows.

Backport of 7eb5391b71f473dd13abdaaef143a5509160487f from main

Note: See TracTickets for help on using tickets.
Back to Top