Repository: zooniverse/panoptes-python-client

Issue:
Improve libmagic MIME-typing solution
Multiple teams have run into issues with the Client's current `libmagic` based MIME-typing solution for non-image media. In most cases, the `libmagic` installation was to blame: namely, the need to install of the underlying C library in addition to the `python-magic` package.

Solution 1: Improve documentation to note the need for C library installation with instructions on how to execute this install. Also consider adding check for presence of C library (e.g., via `ctypes.util.find_library('magic')`) with explanatory error if one is not found.

Solution 2: Replace `libmagic` based MIME-typing with another solution.  As discussed in #210, options include:
- Use existing library for MIME-typing via extension (e.g., `mimetypes`)
- Write manual extension-to-MIME mapping function, mirroring that used by the lab's subject uploader in PFE
- ???

Hints from issue comments:
@lcjohnso: Current plan:
- keep `libmagic` as primary MIME-typing solution (good to reflect on file data rather than trusting extension is accurate), but tone down the feedback info provided to the user.
    - Use `info()` instead of `warning()` -- see https://docs.python.org/3/library/logging.html#logging.Logger.info
    - Use text: "`libmagic` installation not detected. Media type determination will be based on file extensions."
- remove `imghdr`(deprecated from Python 3.13 onward) and adjust implementation to use `mimetypes`
    - Test all Zooniverse-supported mimetypes for compatibility (see list here: https://github.com/zooniverse/Panoptes-Front-End/blob/master/app/pages/lab/subject-set.cjsx#L20)@lcjohnso: Closed by #321 

Solution patch:
diff --git a/panoptes_client/subject.py b/panoptes_client/subject.py
index 092202f..f02659d 100644
--- a/panoptes_client/subject.py
+++ b/panoptes_client/subject.py
@@ -16,6 +16,7 @@
 
 from copy import deepcopy
 from concurrent.futures import ThreadPoolExecutor
+import mimetypes
 
 try:
     import magic
@@ -24,16 +25,13 @@
     import importlib.metadata
     try:
         importlib.metadata.version("python-magic")
-        logging.getLogger('panoptes_client').warn(
-            'Broken libmagic installation detected. The python-magic module is'
-            ' installed but can\'t be imported. Please check that both '
-            'python-magic and the libmagic shared library are installed '
-            'correctly. Uploading media other than images may not work.'
+        logging.getLogger('panoptes_client').info(
+            'libmagic not operational, likely due to lack of shared libraries. '
+            'Media MIME type determination will be based on file extensions.'
         )
     except importlib.metadata.PackageNotFoundError:
         pass
-    import imghdr
-    MEDIA_TYPE_DETECTION = 'imghdr'
+    MEDIA_TYPE_DETECTION = 'mimetypes'
 
 from panoptes_client.panoptes import (
     LinkResolver,
@@ -47,6 +45,19 @@
 RETRY_BACKOFF_INTERVAL = 5
 ASYNC_SAVE_THREADS = 5
 
+ALLOWED_MIME_TYPES = [
+    "image/jpeg",
+    "image/png",
+    "image/gif",
+    "image/svg+xml",
+    "audio/mpeg",
+    "video/mp4",
+    "audio/mp4",
+    "video/mpeg",
+    "text/plain",
+    "application/json",
+]
+
 class Subject(PanoptesObject):
     _api_slug = 'subjects'
     _link_slug = 'subjects'
@@ -226,7 +237,7 @@ def subject_workflow_status(self, workflow_id):
         """
         return next(SubjectWorkflowStatus.where(subject_id=self.id, workflow_id=workflow_id))
 
-    def add_location(self, location):
+    def add_location(self, location, manual_mimetype=None):
         """
         Add a media location to this subject.
 
@@ -234,11 +245,14 @@ def add_location(self, location):
           local file, or a :py:class:`dict` containing MIME types and URLs for
           remote media.
 
+        - **manual_mimetype** optional, passes in a specific MIME type for media item.
+
         Examples::
 
             subject.add_location(my_file)
             subject.add_location('/data/image.jpg')
             subject.add_location({'image/png': 'https://example.com/image.png'})
+            subject.add_location(my_file, manual_mimetype='image/png')
         """
         if type(location) is dict:
             self.locations.append(location)
@@ -252,10 +266,12 @@ def add_location(self, location):
 
         try:
             media_data = f.read()
-            if MEDIA_TYPE_DETECTION == 'magic':
+            if manual_mimetype is not None:
+                media_type = manual_mimetype
+            elif MEDIA_TYPE_DETECTION == 'magic':
                 media_type = magic.from_buffer(media_data, mime=True)
             else:
-                media_type = imghdr.what(None, media_data)
+                media_type = mimetypes.guess_type(location)[0]
                 if not media_type:
                     raise UnknownMediaException(
                         'Could not detect file type. Please try installing '
@@ -263,7 +279,10 @@ def add_location(self, location):
                         'io/en/latest/user_guide.html#uploading-non-image-'
                         'media-types'
                     )
-                media_type = 'image/{}'.format(media_type)
+
+            if media_type not in ALLOWED_MIME_TYPES:
+                raise UnknownMediaException(f"File type {media_type} is not allowed.")
+
             self.locations.append(media_type)
             self._media_files.append(media_data)
             self.modified_attributes.add('locations')


Test patch:
diff --git a/panoptes_client/tests/test_subject.py b/panoptes_client/tests/test_subject.py
new file mode 100644
index 0000000..15b5e83
--- /dev/null
+++ b/panoptes_client/tests/test_subject.py
@@ -0,0 +1,59 @@
+import io
+import unittest
+from unittest.mock import patch, mock_open
+
+from panoptes_client.subject import Subject, UnknownMediaException
+import mimetypes
+
+
+class TestSubject(unittest.TestCase):
+    def setUp(self):
+        self.subject = Subject()
+        self.subject.locations = []
+        self.subject._media_files = []
+        self.subject.modified_attributes = set()
+
+    def test_add_location_with_dict(self):
+        location_dict = {"image/png": "https://example.com/image.png"}
+        self.subject.add_location(location_dict)
+        self.assertIn(location_dict, self.subject.locations)
+        self.assertIn(None, self.subject._media_files)
+        self.assertIn("locations", self.subject.modified_attributes)
+
+    def test_add_location_manual_mimetype_file_like(self):
+        data = b"fake image data"
+        fake_file = io.BytesIO(data)
+        self.subject.add_location(fake_file, manual_mimetype="image/jpeg")
+        self.assertEqual(self.subject.locations[-1], "image/jpeg")
+        self.assertEqual(self.subject._media_files[-1], data)
+        self.assertIn("locations", self.subject.modified_attributes)
+
+    @patch("panoptes_client.subject.magic")
+    def test_add_location_magic_detection(self, mock_magic):
+        mock_magic.from_buffer.return_value = "image/jpeg"
+        data = b"fake image data"
+        fake_file = io.BytesIO(data)
+        self.subject.add_location(fake_file)
+        self.assertEqual(self.subject.locations[-1], "image/jpeg")
+        self.assertEqual(self.subject._media_files[-1], data)
+        self.assertIn("locations", self.subject.modified_attributes)
+        mock_magic.from_buffer.assert_called_with(data, mime=True)
+
+    @patch.object(mimetypes, 'guess_type', return_value=("image/jpeg", None))
+    def test_add_location_mimetypes_detection(self, mock_guess_type):
+        import panoptes_client.subject as subject_module
+        subject_module.MEDIA_TYPE_DETECTION = 'mimetypes'
+
+        m = mock_open(read_data=b"fake image data")
+        with patch("panoptes_client.subject.open", m, create=True):
+            self.subject.add_location("dummy.jpg")
+
+        self.assertEqual(self.subject.locations[-1], "image/jpeg")
+        self.assertEqual(self.subject._media_files[-1], b"fake image data")
+        self.assertIn("locations", self.subject.modified_attributes)
+
+    def test_add_location_invalid_manual_mimetype(self):
+        data = b"fake data"
+        fake_file = io.BytesIO(data)
+        with self.assertRaises(UnknownMediaException):
+            self.subject.add_location(fake_file, manual_mimetype="application/javascript")
\ No newline at end of file