Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ jobs:
fail-fast: false
matrix:
os: ["ubuntu-latest"]
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "3.14"]
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "3.13t", "3.14", "3.14t"]
include:
- os: macos-latest
python-version: "3.x"
Expand Down
64 changes: 35 additions & 29 deletions magic/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ def __init__(
self.flags |= MAGIC_NO_CHECK_SIMH

self.cookie = magic_open(self.flags)
self.lock = threading.Lock()

magic_load(self.cookie, magic_file)

Expand Down Expand Up @@ -138,34 +137,31 @@ def from_buffer(self, buf):
"""
Identify the contents of `buf`
"""
with self.lock:
try:
# if we're on python3, convert buf to bytes
# otherwise this string is passed as wchar*
# which is not what libmagic expects
# NEXTBREAK: only take bytes
if type(buf) == str and str != bytes:
buf = buf.encode("utf-8", errors="replace")
return maybe_decode(magic_buffer(self.cookie, buf))
except MagicException as e:
return self._handle509Bug(e)
try:
# if we're on python3, convert buf to bytes
# otherwise this string is passed as wchar*
# which is not what libmagic expects
# NEXTBREAK: only take bytes
if type(buf) == str and str != bytes:
buf = buf.encode("utf-8", errors="replace")
return maybe_decode(magic_buffer(self.cookie, buf))
except MagicException as e:
return self._handle509Bug(e)

def from_file(self, filename):
# raise FileNotFoundException or IOError if the file does not exist
os.stat(filename, follow_symlinks=self.flags & MAGIC_SYMLINK)

with self.lock:
try:
return maybe_decode(magic_file(self.cookie, filename))
except MagicException as e:
return self._handle509Bug(e)
try:
return maybe_decode(magic_file(self.cookie, filename))
except MagicException as e:
return self._handle509Bug(e)

def from_descriptor(self, fd):
with self.lock:
try:
return maybe_decode(magic_descriptor(self.cookie, fd))
except MagicException as e:
return self._handle509Bug(e)
try:
return maybe_decode(magic_descriptor(self.cookie, fd))
except MagicException as e:
return self._handle509Bug(e)

def _handle509Bug(self, e):
# libmagic 5.09 has a bug where it might fail to identify the
Expand Down Expand Up @@ -317,6 +313,9 @@ def coerce_filename(filename):
return filename


# libmagic is not thread-safe: guard for concurrent calls on a global scope
LOCK = threading.Lock()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cclauss I realized there is no reason to allow passing a custom lock


magic_open = libmagic.magic_open
magic_open.restype = magic_t
magic_open.argtypes = [c_int]
Expand All @@ -340,7 +339,8 @@ def coerce_filename(filename):


def magic_file(cookie, filename):
return _magic_file(cookie, coerce_filename(filename))
with LOCK:
return _magic_file(cookie, coerce_filename(filename))


_magic_buffer = libmagic.magic_buffer
Expand All @@ -350,7 +350,8 @@ def magic_file(cookie, filename):


def magic_buffer(cookie, buf):
return _magic_buffer(cookie, buf, len(buf))
with LOCK:
return _magic_buffer(cookie, buf, len(buf))


magic_descriptor = libmagic.magic_descriptor
Expand All @@ -365,7 +366,8 @@ def magic_buffer(cookie, buf):


def magic_descriptor(cookie, fd):
return _magic_descriptor(cookie, fd)
with LOCK:
return _magic_descriptor(cookie, fd)


_magic_load = libmagic.magic_load
Expand All @@ -375,7 +377,8 @@ def magic_descriptor(cookie, fd):


def magic_load(cookie, filename):
return _magic_load(cookie, coerce_filename(filename))
with LOCK:
return _magic_load(cookie, coerce_filename(filename))


magic_setflags = libmagic.magic_setflags
Expand Down Expand Up @@ -408,14 +411,16 @@ def magic_setparam(cookie, param, val):
if not _has_param:
raise NotImplementedError("magic_setparam not implemented")
v = c_size_t(val)
return _magic_setparam(cookie, param, byref(v))
with LOCK:
return _magic_setparam(cookie, param, byref(v))


def magic_getparam(cookie, param):
if not _has_param:
raise NotImplementedError("magic_getparam not implemented")
val = c_size_t()
_magic_getparam(cookie, param, byref(val))
with LOCK:
_magic_getparam(cookie, param, byref(val))
return val.value


Expand All @@ -430,7 +435,8 @@ def magic_getparam(cookie, param):
def version():
if not _has_version:
raise NotImplementedError("magic_version not implemented")
return magic_version()
with LOCK:
return magic_version()


MAGIC_NONE = 0x000000 # No flags
Expand Down
25 changes: 25 additions & 0 deletions test/python_magic_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@

import pytest

try:
from concurrent.futures import ThreadPoolExecutor
HAS_CONCURRENT_FUTURES = True
except ImportError: # python 2.7
HAS_CONCURRENT_FUTURES = False

# for output which reports a local time
os.environ["TZ"] = "GMT"

Expand Down Expand Up @@ -321,6 +327,25 @@ def test_symlink(self):

self.assertRaises(IOError, m_follow.from_file, tmp_broken)

@unittest.skipIf(not HAS_CONCURRENT_FUTURES, "concurrent.futures not available in Python 2.7")
def test_thread_safety(self):
"""Test that concurrent from_file calls don't crash (would SEGV without global lock)"""
filename = os.path.join(self.TESTDATA_DIR, "test.pdf")

m = magic.Magic(mime=True)

def check_file(_):
result = m.from_file(filename)
self.assertEqual(result, "application/pdf")
return result

with ThreadPoolExecutor(100) as executor:
results = list(executor.map(check_file, range(100)))

# All calls should complete successfully
self.assertEqual(len(results), 100)
self.assertTrue(all(r == "application/pdf" for r in results))


if __name__ == "__main__":
unittest.main()
3 changes: 3 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ envlist =
py311,
py312,
py313,
py313t,
py314,
py314t,
mypy

[testenv]
Expand Down