Repository: FZJ-INM1-BDA/pytiff (language: python)

Issue:
Non-ascii data in string tags causes a UnicodeDecodeError on Python 3
Although TIFF string tags are only supposed to contain 7-bit ASCII characters, many tools write values in UTF-8 or other encodings that aren't 7-bit clean. OME-TIFF/BioFormats is one such tool, where the XML stored in the ImageDescription tag is explicitly encoded as UTF-8. On Python 3, `pytiff.Tiff._read_ascii` raises a UnicodeDecodeError upon reading such values. On Python 2, where the treatment of string encoding/decoding isn't as rigorous, the problem is effectively ignored.

Would you accept a patch to fix this? I think the simplest approach is to always decode strings as UTF-8, perhaps only under Python 3. This actually mirrors the way `_set_tag` already performs UTF-8 string encoding, only under Python 3. I would also be willing to implement a more flexible approach with a user-controlled encoding if you think that's a better option.


Discussion / hints:
Hi @jmuhlich 

feel free to open a PR to fix this. How would you implement the flexible approach? `_set_tag` should already accept a byte string, so a user could do the encoding and decoding himself.

Fix patch:
diff --git a/pytiff/_pytiff.pyx b/pytiff/_pytiff.pyx
index babf9c5..20d0c85 100644
--- a/pytiff/_pytiff.pyx
+++ b/pytiff/_pytiff.pyx
@@ -1,4 +1,4 @@
-#cython: c_string_type=str, c_string_encoding=ascii
+#cython: c_string_type=str, c_string_encoding=ascii, language_level=2
 """
 pytiff is a python wrapper for the libtiff c api written in cython. It is python 2 and 3 compatible.
 While there are some missing features, it supports reading chunks of tiled greyscale tif images as well as basic reading for color images.
@@ -350,8 +350,8 @@ cdef _get_rgb(np.ndarray[np.uint32_t, ndim=2] inp, short n_samples):
   return rgb
 
 cpdef object rebuild(data):
-    filename, file_mode, bigtiff, current_page = data
-    obj = Tiff(filename, file_mode, bigtiff)
+    filename, file_mode, bigtiff, encoding, current_page = data
+    obj = Tiff(filename, file_mode, bigtiff, encoding)
     obj.set_page(current_page)
     return obj
 
@@ -374,6 +374,7 @@ cdef class Tiff:
     filename (string): The filename of the tiff file.
     file_mode (string): File mode either "w" for writing (old data is deleted), "a" for appending or "r" for reading. Default: "r".
     bigiff (bool): If True the file is assumed to be bigtiff. Default: False.
+    encoding (string): Optional string encoding name to enable Unicode support for "ascii" tags. Default: None (ascii tags are always bytes).
   """
   cdef ctiff.TIFF* tiff_handle
   cdef public short samples_per_pixel
@@ -385,12 +386,13 @@ cdef class Tiff:
   cdef object cache, logger
   cdef public object filename
   cdef object file_mode
+  cdef public object encoding
   cdef public object tags
   cdef _dtype_write
   cdef object _singlepage
   cdef object _pages
 
-  def __cinit__(self, filename, file_mode="r", bigtiff=False):
+  def __cinit__(self, filename, file_mode="r", bigtiff=False, encoding=None):
     if bigtiff:
       file_mode += "8"
     tmp_filename = <string> filename
@@ -398,6 +400,7 @@ cdef class Tiff:
     self.closed = True
     self.filename = tmp_filename
     self.file_mode = tmp_mode
+    self.encoding = encoding
     self._write_mode_n_pages = 0
     self.n_pages = 0
     self._singlepage = False
@@ -469,7 +472,7 @@ cdef class Tiff:
       bigtiff = False
       if "8" in self.file_mode:
           bigtiff = True
-      data = self.filename, self.file_mode, bigtiff, self.current_page
+      data = self.filename, self.file_mode, bigtiff, self.encoding, self.current_page
       return rebuild, (data,)
 
   @property
@@ -1103,7 +1106,7 @@ cdef class Tiff:
     return data, err
 
   def _read_ascii(self, tag):
-    """ reads an ascii string from a Tiff File
+    """ reads a string from a Tiff File
 
         Args:
             tag (integer): the attribute tag
@@ -1113,10 +1116,14 @@ cdef class Tiff:
     """
     cdef char* desc = ''
     err = ctiff.TIFFGetField(self.tiff_handle, tag, &desc)
-    str = <string>desc
-    if str == "":
-      str = None
-    return str, err
+    bytes_desc = <bytes>desc
+    if self.encoding:
+      data = bytes_desc.decode(self.encoding)
+    else:
+      data = bytes_desc
+    if data == "":
+      data = None
+    return data, err
 
   def set_tags(self, tagdict=None, **kwargs):
     """ writes the tag/value pairs in the dict to the Tiff File
@@ -1172,13 +1179,21 @@ cdef class Tiff:
       total_pages = value[1]
       err = ctiff.TIFFSetField(self.tiff_handle, tag, page, total_pages)
       return
-    if isinstance(value, str):
-      value = value + "\0"
-      if PY3:
-         py_byte_string = value.encode()
-         char_ptr = py_byte_string
+    if isinstance(value, bytes):
+      value += b"\0"
+      char_ptr = value
+      err = ctiff.TIFFSetField(self.tiff_handle, tag, char_ptr)
+      return
+    if isinstance(value, unicode):
+      if self.encoding:
+        py_byte_string = value.encode(self.encoding)
+      elif PY3:
+        # Backwards compatibility for old behavior (implicit encoding on PY3).
+        py_byte_string = value.encode()
       else:
-         char_ptr = value
+        py_byte_string = bytes(value)
+      py_byte_string += b"\0"
+      char_ptr = py_byte_string
       err = ctiff.TIFFSetField(self.tiff_handle, tag, char_ptr)
       return
     else:
@@ -1247,7 +1262,7 @@ cdef class Tiff:
       if "8" in self.file_mode:
           mode += "8"
       while current < self.number_of_pages:
-          page = Tiff(self.filename, mode)
+          page = Tiff(self.filename, mode, encoding=self.encoding)
           page.set_page(current)
           current += 1
           page._singlepage = True


Test patch:
diff --git a/pytiff/test/test_tags.py b/pytiff/test/test_tags.py
index cc90312..42cc033 100644
--- a/pytiff/test/test_tags.py
+++ b/pytiff/test/test_tags.py
@@ -1,3 +1,4 @@
+import pytest
 from hypothesis import given, settings
 import hypothesis.strategies as st
 from pytiff import Tiff
@@ -126,6 +127,80 @@ def test_tags_bigtif(tmpdir_factory):
     assert np.all(written_image == image)
     check_written_tags(written_tags, tags)
 
+def test_tags_unicode(tmpdir_factory):
+    testfile = "test_data/unicode_imagedescription.tif"
+    basename = os.path.basename(testfile)
+
+    # Read a UTF-8 tag as bytes.
+    with Tiff(testfile) as handle:
+        tags = handle.read_tags()
+    with tifffile.TiffFile(testfile) as handle:
+        page1 = handle.pages[0]
+        tifffile_tags = {}
+        for tag in page1.tags.values():
+            name, value = tag.name, tag.value
+            tifffile_tags[name] = value
+        image = page1.asarray()
+    compare_tags(tags, tifffile_tags)
+
+    # Decode UTF-8 in a tag.
+    with Tiff(testfile, encoding='utf-8') as handle:
+        tags_unicode = handle.read_tags()
+    compare_tags(tags_unicode, tifffile_tags, encoding='utf-8')
+
+    # Decoding UTF-8 as ascii should error.
+    with pytest.raises(UnicodeDecodeError):
+        _ = Tiff(testfile, encoding='ascii')
+
+    # Write a pre-encoded UTF-8 tag (bytes).
+    filename = str(tmpdir_factory.mktemp("write_tags").join(basename))
+    with Tiff(filename, "w") as handle:
+        handle.set_tags(tags)
+        handle.write(image, method="tile", tile_length=tags[pytiff.tags.tile_length], tile_width=tags[pytiff.tags.tile_width])
+    with Tiff(filename) as handle:
+        written_tags = handle.read_tags()
+        written_image = handle[:]
+    assert np.all(written_image == image)
+    check_written_tags(written_tags, tags)
+
+    # Encode a unicode tag as UTF-8.
+    filename = str(tmpdir_factory.mktemp("write_tags").join(basename))
+    with Tiff(filename, "w", encoding='utf-8') as handle:
+        handle.set_tags(tags_unicode)
+        handle.write(image, method="tile", tile_length=tags_unicode[pytiff.tags.tile_length], tile_width=tags_unicode[pytiff.tags.tile_width])
+    with Tiff(filename, encoding='utf-8') as handle:
+        written_tags = handle.read_tags()
+        written_image = handle[:]
+    assert np.all(written_image == image)
+    check_written_tags(written_tags, tags_unicode)
+
+    # Write a unicode tag containing only ascii, without explicit encoding.
+    filename = str(tmpdir_factory.mktemp("write_tags").join(basename))
+    tags_unicode2 = tags.copy()
+    tags_unicode2[pytiff.tags.image_description] = u'pytiff'
+    with Tiff(filename, "w") as handle:
+        handle.set_tags(tags_unicode2)
+        handle.write(image, method="tile", tile_length=tags_unicode2[pytiff.tags.tile_length], tile_width=tags_unicode2[pytiff.tags.tile_width])
+    with Tiff(filename, encoding='ascii') as handle:
+        written_tags = handle.read_tags()
+        written_image = handle[:]
+        assert np.all(written_image == image)
+    check_written_tags(written_tags, tags_unicode2)
+
+    # Implicitly encode a unicode tag, but only under Python 3 (for backwards
+    # compatibility with old API).
+    if sys.version_info[0] == 3:
+        filename = str(tmpdir_factory.mktemp("write_tags").join(basename))
+        with Tiff(filename, "w") as handle:
+            handle.set_tags(tags_unicode)
+            handle.write(image, method="tile", tile_length=tags_unicode[pytiff.tags.tile_length], tile_width=tags_unicode[pytiff.tags.tile_width])
+        with Tiff(filename, encoding='utf-8') as handle:
+            written_tags = handle.read_tags()
+            written_image = handle[:]
+        assert np.all(written_image == image)
+        check_written_tags(written_tags, tags_unicode)
+
+
 def check_written_tags(written_tags, tags):
     for k in written_tags:
         if "_offsets" in k.name:
@@ -142,12 +217,13 @@ def check_written_tags(written_tags, tags):
         else:
             assert written_tags[k] == tags[k]
 
-def compare_tags(pytiff_tags, tifffile_tags):
+def compare_tags(pytiff_tags, tifffile_tags, encoding=None):
     assert len(pytiff_tags) == len(tifffile_tags), "missing keys: {}".format(set([t.name for t in pytiff_tags]) - set(tifffile_tags.keys()))
     for name in tifffile_tags:
         value = tifffile_tags[name]
         if isinstance(value, bytes):
-            value = value.decode()
+            if encoding:
+                value = value.decode(encoding)
             assert value == pytiff_tags[pytiff.tags[name]], "key {}: {} == {}".format(name, value, pytiff_tags[pytiff.tags[name]])
         elif name == "x_resolution" or name == "y_resolution":
             assert value[0] / value[1] == pytiff_tags[pytiff.tags[name]]
diff --git a/test_data/unicode_imagedescription.tif b/test_data/unicode_imagedescription.tif
new file mode 100644
index 0000000..a1f7ad4
Binary files /dev/null and b/test_data/unicode_imagedescription.tif differ