Skip to content

fix(tiff): Improve TIFF robustness for non-matching tag/metadata types - #5036

Merged
lgritz merged 2 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-tiffmetaxlate
Feb 12, 2026
Merged

fix(tiff): Improve TIFF robustness for non-matching tag/metadata types#5036
lgritz merged 2 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-tiffmetaxlate

Conversation

@lgritz

@lgritz lgritz commented Feb 9, 2026

Copy link
Copy Markdown
Collaborator

This is a more comprehensive fix for issues discovered in PR #5035.

The original problem reported in Issue #5023 was a crash when writing TIFF information that was supposed to be arrays of more than one rational, it was reading past the end of a memory array.

#5035 is a minimal, immediate fix to address the crashes. But in the process, I saw a number of ways in which we were dropping metadata on the floor when the types didn't exactly match, but that we could handle with automatic conversion.

The new cases that we handle with this PR are:

  • Exif RESOLUTIONUNIT tag is a short, but by convention we store it by the name as a string in OIIO metadata, so we need to convert back to a code (we did so for the main TIFF metadata, but not for Exif in TIFF).
  • Handle Exif "version" and "flashpixversion" metadata which have unusual encoding in TIFF files (they are 4-character strings, but must be stored in a TIFF tag of type BYTES, not as the usual type ASCII that most strings use.
  • Handle things that TIFF insists are ASCII but that come to us as metadata that's strings. Easy -- our ParamValue.get_string() automatically converts ther things like ints or floats into string representation.
  • Much more flexibility in automatically converting among the signed and unsigned, 16 and 32 bit, integer types when the metadata in our ImageSpec is integer but not the specific type of integer that TIFF/Exif thinks it should be.

This doesn't appear to change the results of anything in our testsuite, but it's possible that some non-TIFF-to-TIFF image conversions that contain Exif data may now do certain type conversions properly instead of just silently dropping the metadata that had non-matching (but reasonably valid) types.

Additionally, to do this nicely, I ended up adding a new TypeURational alias in typedesc.h (similar to TypeRational, but the case where both numerator and denominator are unsigned ints).

And also fixed a random comment typo I noticed in tiffinput.cpp.

@jessey-git

Copy link
Copy Markdown
Contributor

The original bug report now produces the following log output. The first line is expected but I'm unsure why Exif:ExifVersion is being reported?

Unhandled EXIF Exif:LensSpecification (float) / tag 42034 tifftype 5 count 4
Unhandled EXIF exif:ExifVersion (int) / tag 36864 tifftype 7 count 1

The original attribute looks like: exif:ExifVersion: 221

@lgritz

lgritz commented Feb 11, 2026

Copy link
Copy Markdown
Collaborator Author

DEBUG builds (but not Release) print that message when a known Exif attribute is encountered but it can't figure out what to do. This is supposed to help us catch the set of unhandled cases. Currently, nothing in the testsuite triggers this.

The file in the bug report is deeply flawed. The Exif "LensSpecification" is supposed to be composed of 4 rational (or float, on our side) numbers. But the exr file in question just gives a single float.

But I do think we can probably handle the ExifVersion, let me fix that.

Fixes AcademySoftwareFoundation#5023

This was crashing when writing TIFF information that was supposed to
be arrays of more than one rational, it was reading past the end of
a memory array.

But upon looking at it, I realized that for regular ints and floats as
well, we were only handling scalars properly. And furthermore, there
were some places where we were dropping metadata on the floor when the
types didn't exactly match, but that we *could* handle with a little
conversion.

More specifically, the new cases that we handle are:

* Exif RESOLUTIONUNIT tag is a short, but by convention we store it by
  the name as a string in OIIO metadata, so we need to convert back to
  a code (we did so for the main TIFF metadata, but not for Exif in
  TIFF).
* Handle Exif "version" and "flashpixversion" metadata which have
  unusual encoding in TIFF files (they are 4-character strings, but
  must be stored in a TIFF tag of type BYTES, not as the usual type
  ASCII that most strings use.
* Handle things that TIFF insists are ASCII but that come to us as
  metadata that's strings. Easy -- our `ParamValue.get_string()`
  automatically converts ther things like ints or floats into string
  representation.
* Much more flexibility in automatically converting among the signed
  and unsigned, 16 and 32 bit, integer types when the metadata in our
  ImageSpec is integer but not the specific type of integer that
  TIFF/Exif thinks it should be.

This doesn't appear to change the results of anything in our
testsuite, but it's possible that some non-TIFF-to-TIFF image
conversions that contain Exif data may now correctly do certain type
conversins properly instead of just silently dropping the metadata
that needed certain type conversions.

Additionally, to do this nicely, I ended up adding a new TypeURational
alias in typedesc.h (similar to TypeRational, but the case where both
numerator and denominator are unsigned ints).

And also fixed a random comment typo I noticed in tiffinput.cpp.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
@jessey-git

Copy link
Copy Markdown
Contributor

Code looks good now and exiftool shows the (corrected) version on the converted .tif:
Exif Version : 0221

But somehow oiiotool --info -v does not list it. What might be causing that?

@lgritz

lgritz commented Feb 11, 2026

Copy link
Copy Markdown
Collaborator Author

Code looks good now and exiftool shows the (corrected) version on the converted .tif: Exif Version : 0221

But somehow oiiotool --info -v does not list it. What might be causing that?

I'm investigating. I think there are some deficiencies on the TIFF reading side as well. Since that's a separate code path and set of issues than the present PR that addresses the writing side of things, I'd like to merge this PR if the write looks correct, and then I will make a separate PR for fixes to come on the reading side. Sound ok?

@jessey-git jessey-git left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, sounds reasonable.

@lgritz
lgritz merged commit 8a39d36 into AcademySoftwareFoundation:main Feb 12, 2026
56 of 57 checks passed
@lgritz
lgritz deleted the lg-tiffmetaxlate branch February 12, 2026 00:56
lgritz added a commit to lgritz/OpenImageIO that referenced this pull request Feb 15, 2026
AcademySoftwareFoundation#5036)

This is a more comprehensive fix for issues discovered in PR AcademySoftwareFoundation#5035.

The original problem reported in Issue AcademySoftwareFoundation#5023 was a crash when writing
TIFF information that was supposed to be arrays of more than one
rational, it was reading past the end of a memory array.

process, I saw a number of ways in which we were dropping metadata on
the floor when the types didn't exactly match, but that we *could*
handle with automatic conversion.

The new cases that we handle with this PR are:

* Exif RESOLUTIONUNIT tag is a short, but by convention we store it by
the name as a string in OIIO metadata, so we need to convert back to a
code (we did so for the main TIFF metadata, but not for Exif in TIFF).
* Handle Exif "version" and "flashpixversion" metadata which have
unusual encoding in TIFF files (they are 4-character strings, but must
be stored in a TIFF tag of type BYTES, not as the usual type ASCII that
most strings use.
* Handle things that TIFF insists are ASCII but that come to us as
metadata that's strings. Easy -- our `ParamValue.get_string()`
automatically converts ther things like ints or floats into string
representation.
* Much more flexibility in automatically converting among the signed and
unsigned, 16 and 32 bit, integer types when the metadata in our
ImageSpec is integer but not the specific type of integer that TIFF/Exif
thinks it should be.

This doesn't appear to change the results of anything in our testsuite,
but it's possible that some non-TIFF-to-TIFF image conversions that
contain Exif data may now do certain type conversions properly instead
of just silently dropping the metadata that had non-matching (but
reasonably valid) types.

Additionally, to do this nicely, I ended up adding a new TypeURational
alias in typedesc.h (similar to TypeRational, but the case where both
numerator and denominator are unsigned ints).

And also fixed a random comment typo I noticed in tiffinput.cpp.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Feb 24, 2026
AcademySoftwareFoundation#5036)

This is a more comprehensive fix for issues discovered in PR AcademySoftwareFoundation#5035.

The original problem reported in Issue AcademySoftwareFoundation#5023 was a crash when writing
TIFF information that was supposed to be arrays of more than one
rational, it was reading past the end of a memory array.

AcademySoftwareFoundation#5035 is a minimal, immediate fix to address the crashes. But in the
process, I saw a number of ways in which we were dropping metadata on
the floor when the types didn't exactly match, but that we *could*
handle with automatic conversion.

The new cases that we handle with this PR are:

* Exif RESOLUTIONUNIT tag is a short, but by convention we store it by
the name as a string in OIIO metadata, so we need to convert back to a
code (we did so for the main TIFF metadata, but not for Exif in TIFF).
* Handle Exif "version" and "flashpixversion" metadata which have
unusual encoding in TIFF files (they are 4-character strings, but must
be stored in a TIFF tag of type BYTES, not as the usual type ASCII that
most strings use.
* Handle things that TIFF insists are ASCII but that come to us as
metadata that's strings. Easy -- our `ParamValue.get_string()`
automatically converts ther things like ints or floats into string
representation.
* Much more flexibility in automatically converting among the signed and
unsigned, 16 and 32 bit, integer types when the metadata in our
ImageSpec is integer but not the specific type of integer that TIFF/Exif
thinks it should be.

This doesn't appear to change the results of anything in our testsuite,
but it's possible that some non-TIFF-to-TIFF image conversions that
contain Exif data may now do certain type conversions properly instead
of just silently dropping the metadata that had non-matching (but
reasonably valid) types.

Additionally, to do this nicely, I ended up adding a new TypeURational
alias in typedesc.h (similar to TypeRational, but the case where both
numerator and denominator are unsigned ints).

And also fixed a random comment typo I noticed in tiffinput.cpp.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
lgritz added a commit that referenced this pull request Feb 26, 2026
PR #5036 incidentally added a new TypeURational alias, but I neglected
to make the corresponding change on the Python bindings, to update some
of the docs, or to fully test it. This PR completes those items.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
lgritz added a commit to lgritz/OpenImageIO that referenced this pull request Feb 27, 2026
…5036 (AcademySoftwareFoundation#5057)

PR AcademySoftwareFoundation#5036 incidentally added a new TypeURational alias, but I neglected
to make the corresponding change on the Python bindings, to update some
of the docs, or to fully test it. This PR completes those items.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 1, 2026
AcademySoftwareFoundation#5036)

This is a more comprehensive fix for issues discovered in PR AcademySoftwareFoundation#5035.

The original problem reported in Issue AcademySoftwareFoundation#5023 was a crash when writing
TIFF information that was supposed to be arrays of more than one
rational, it was reading past the end of a memory array.

AcademySoftwareFoundation#5035 is a minimal, immediate fix to address the crashes. But in the
process, I saw a number of ways in which we were dropping metadata on
the floor when the types didn't exactly match, but that we *could*
handle with automatic conversion.

The new cases that we handle with this PR are:

* Exif RESOLUTIONUNIT tag is a short, but by convention we store it by
the name as a string in OIIO metadata, so we need to convert back to a
code (we did so for the main TIFF metadata, but not for Exif in TIFF).
* Handle Exif "version" and "flashpixversion" metadata which have
unusual encoding in TIFF files (they are 4-character strings, but must
be stored in a TIFF tag of type BYTES, not as the usual type ASCII that
most strings use.
* Handle things that TIFF insists are ASCII but that come to us as
metadata that's strings. Easy -- our `ParamValue.get_string()`
automatically converts ther things like ints or floats into string
representation.
* Much more flexibility in automatically converting among the signed and
unsigned, 16 and 32 bit, integer types when the metadata in our
ImageSpec is integer but not the specific type of integer that TIFF/Exif
thinks it should be.

This doesn't appear to change the results of anything in our testsuite,
but it's possible that some non-TIFF-to-TIFF image conversions that
contain Exif data may now do certain type conversions properly instead
of just silently dropping the metadata that had non-matching (but
reasonably valid) types.

Additionally, to do this nicely, I ended up adding a new TypeURational
alias in typedesc.h (similar to TypeRational, but the case where both
numerator and denominator are unsigned ints).

And also fixed a random comment typo I noticed in tiffinput.cpp.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad <shaamaan@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 1, 2026
…5036 (AcademySoftwareFoundation#5057)

PR AcademySoftwareFoundation#5036 incidentally added a new TypeURational alias, but I neglected
to make the corresponding change on the Python bindings, to update some
of the docs, or to fully test it. This PR completes those items.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad <shaamaan@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
AcademySoftwareFoundation#5036)

This is a more comprehensive fix for issues discovered in PR AcademySoftwareFoundation#5035.

The original problem reported in Issue AcademySoftwareFoundation#5023 was a crash when writing
TIFF information that was supposed to be arrays of more than one
rational, it was reading past the end of a memory array.

AcademySoftwareFoundation#5035 is a minimal, immediate fix to address the crashes. But in the
process, I saw a number of ways in which we were dropping metadata on
the floor when the types didn't exactly match, but that we *could*
handle with automatic conversion.

The new cases that we handle with this PR are:

* Exif RESOLUTIONUNIT tag is a short, but by convention we store it by
the name as a string in OIIO metadata, so we need to convert back to a
code (we did so for the main TIFF metadata, but not for Exif in TIFF).
* Handle Exif "version" and "flashpixversion" metadata which have
unusual encoding in TIFF files (they are 4-character strings, but must
be stored in a TIFF tag of type BYTES, not as the usual type ASCII that
most strings use.
* Handle things that TIFF insists are ASCII but that come to us as
metadata that's strings. Easy -- our `ParamValue.get_string()`
automatically converts ther things like ints or floats into string
representation.
* Much more flexibility in automatically converting among the signed and
unsigned, 16 and 32 bit, integer types when the metadata in our
ImageSpec is integer but not the specific type of integer that TIFF/Exif
thinks it should be.

This doesn't appear to change the results of anything in our testsuite,
but it's possible that some non-TIFF-to-TIFF image conversions that
contain Exif data may now do certain type conversions properly instead
of just silently dropping the metadata that had non-matching (but
reasonably valid) types.

Additionally, to do this nicely, I ended up adding a new TypeURational
alias in typedesc.h (similar to TypeRational, but the case where both
numerator and denominator are unsigned ints).

And also fixed a random comment typo I noticed in tiffinput.cpp.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
…5036 (AcademySoftwareFoundation#5057)

PR AcademySoftwareFoundation#5036 incidentally added a new TypeURational alias, but I neglected
to make the corresponding change on the Python bindings, to update some
of the docs, or to fully test it. This PR completes those items.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
AcademySoftwareFoundation#5036)

This is a more comprehensive fix for issues discovered in PR AcademySoftwareFoundation#5035.

The original problem reported in Issue AcademySoftwareFoundation#5023 was a crash when writing
TIFF information that was supposed to be arrays of more than one
rational, it was reading past the end of a memory array.

AcademySoftwareFoundation#5035 is a minimal, immediate fix to address the crashes. But in the
process, I saw a number of ways in which we were dropping metadata on
the floor when the types didn't exactly match, but that we *could*
handle with automatic conversion.

The new cases that we handle with this PR are:

* Exif RESOLUTIONUNIT tag is a short, but by convention we store it by
the name as a string in OIIO metadata, so we need to convert back to a
code (we did so for the main TIFF metadata, but not for Exif in TIFF).
* Handle Exif "version" and "flashpixversion" metadata which have
unusual encoding in TIFF files (they are 4-character strings, but must
be stored in a TIFF tag of type BYTES, not as the usual type ASCII that
most strings use.
* Handle things that TIFF insists are ASCII but that come to us as
metadata that's strings. Easy -- our `ParamValue.get_string()`
automatically converts ther things like ints or floats into string
representation.
* Much more flexibility in automatically converting among the signed and
unsigned, 16 and 32 bit, integer types when the metadata in our
ImageSpec is integer but not the specific type of integer that TIFF/Exif
thinks it should be.

This doesn't appear to change the results of anything in our testsuite,
but it's possible that some non-TIFF-to-TIFF image conversions that
contain Exif data may now do certain type conversions properly instead
of just silently dropping the metadata that had non-matching (but
reasonably valid) types.

Additionally, to do this nicely, I ended up adding a new TypeURational
alias in typedesc.h (similar to TypeRational, but the case where both
numerator and denominator are unsigned ints).

And also fixed a random comment typo I noticed in tiffinput.cpp.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
…5036 (AcademySoftwareFoundation#5057)

PR AcademySoftwareFoundation#5036 incidentally added a new TypeURational alias, but I neglected
to make the corresponding change on the Python bindings, to update some
of the docs, or to fully test it. This PR completes those items.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
AcademySoftwareFoundation#5036)

This is a more comprehensive fix for issues discovered in PR AcademySoftwareFoundation#5035.

The original problem reported in Issue AcademySoftwareFoundation#5023 was a crash when writing
TIFF information that was supposed to be arrays of more than one
rational, it was reading past the end of a memory array.

AcademySoftwareFoundation#5035 is a minimal, immediate fix to address the crashes. But in the
process, I saw a number of ways in which we were dropping metadata on
the floor when the types didn't exactly match, but that we *could*
handle with automatic conversion.

The new cases that we handle with this PR are:

* Exif RESOLUTIONUNIT tag is a short, but by convention we store it by
the name as a string in OIIO metadata, so we need to convert back to a
code (we did so for the main TIFF metadata, but not for Exif in TIFF).
* Handle Exif "version" and "flashpixversion" metadata which have
unusual encoding in TIFF files (they are 4-character strings, but must
be stored in a TIFF tag of type BYTES, not as the usual type ASCII that
most strings use.
* Handle things that TIFF insists are ASCII but that come to us as
metadata that's strings. Easy -- our `ParamValue.get_string()`
automatically converts ther things like ints or floats into string
representation.
* Much more flexibility in automatically converting among the signed and
unsigned, 16 and 32 bit, integer types when the metadata in our
ImageSpec is integer but not the specific type of integer that TIFF/Exif
thinks it should be.

This doesn't appear to change the results of anything in our testsuite,
but it's possible that some non-TIFF-to-TIFF image conversions that
contain Exif data may now do certain type conversions properly instead
of just silently dropping the metadata that had non-matching (but
reasonably valid) types.

Additionally, to do this nicely, I ended up adding a new TypeURational
alias in typedesc.h (similar to TypeRational, but the case where both
numerator and denominator are unsigned ints).

And also fixed a random comment typo I noticed in tiffinput.cpp.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
Signed-off-by: Vlad <shaamaan@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
…5036 (AcademySoftwareFoundation#5057)

PR AcademySoftwareFoundation#5036 incidentally added a new TypeURational alias, but I neglected
to make the corresponding change on the Python bindings, to update some
of the docs, or to fully test it. This PR completes those items.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
Signed-off-by: Vlad <shaamaan@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
AcademySoftwareFoundation#5036)

This is a more comprehensive fix for issues discovered in PR AcademySoftwareFoundation#5035.

The original problem reported in Issue AcademySoftwareFoundation#5023 was a crash when writing
TIFF information that was supposed to be arrays of more than one
rational, it was reading past the end of a memory array.

AcademySoftwareFoundation#5035 is a minimal, immediate fix to address the crashes. But in the
process, I saw a number of ways in which we were dropping metadata on
the floor when the types didn't exactly match, but that we *could*
handle with automatic conversion.

The new cases that we handle with this PR are:

* Exif RESOLUTIONUNIT tag is a short, but by convention we store it by
the name as a string in OIIO metadata, so we need to convert back to a
code (we did so for the main TIFF metadata, but not for Exif in TIFF).
* Handle Exif "version" and "flashpixversion" metadata which have
unusual encoding in TIFF files (they are 4-character strings, but must
be stored in a TIFF tag of type BYTES, not as the usual type ASCII that
most strings use.
* Handle things that TIFF insists are ASCII but that come to us as
metadata that's strings. Easy -- our `ParamValue.get_string()`
automatically converts ther things like ints or floats into string
representation.
* Much more flexibility in automatically converting among the signed and
unsigned, 16 and 32 bit, integer types when the metadata in our
ImageSpec is integer but not the specific type of integer that TIFF/Exif
thinks it should be.

This doesn't appear to change the results of anything in our testsuite,
but it's possible that some non-TIFF-to-TIFF image conversions that
contain Exif data may now do certain type conversions properly instead
of just silently dropping the metadata that had non-matching (but
reasonably valid) types.

Additionally, to do this nicely, I ended up adding a new TypeURational
alias in typedesc.h (similar to TypeRational, but the case where both
numerator and denominator are unsigned ints).

And also fixed a random comment typo I noticed in tiffinput.cpp.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
Signed-off-by: Vlad <shaamaan@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
…5036 (AcademySoftwareFoundation#5057)

PR AcademySoftwareFoundation#5036 incidentally added a new TypeURational alias, but I neglected
to make the corresponding change on the Python bindings, to update some
of the docs, or to fully test it. This PR completes those items.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
Signed-off-by: Vlad <shaamaan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants