Skip to content

[IMAGING-386] TiffField numeric getters throw ImagingException for non-number values - #560

Open
sharanggupta wants to merge 1 commit into
apache:masterfrom
sharanggupta:IMAGING-386-tiff-field-numeric-value-type
Open

sharanggupta wants to merge 1 commit into
apache:masterfrom
sharanggupta:IMAGING-386-tiff-field-numeric-value-type

Conversation

@sharanggupta

Copy link
Copy Markdown

Fixes IMAGING-386.

What

TiffField.getIntValue() returned ((Number) o).intValue(). For a field whose type is BYTE and whose count is not 1, FieldTypeByte.getValue returns a byte[], so the cast threw a ClassCastException, although the method documents @throws ImagingException if the value is missing or cannot be converted. The ticket's TIFF declares its FillOrder tag (a single SHORT in the specification) as BYTE with count 0, and AbstractImageDataReader.decompress reads that tag before any codec runs, so Imaging.getBufferedImage escaped with a ClassCastException instead of the documented ImagingException. The sibling methods getIntArrayValue, getIntValueOrArraySum and getLongArrayValue already turn an unsupported value into an ImagingException.

How

A private getNumberValue() does the existing null check and now also rejects a value that is not a Number with an ImagingException that names the tag and the actual value type. getIntValue() uses it. getLongValue() and getDoubleValue() had the identical unchecked cast and the same Javadoc, so they use it too; I kept them in this change because it is the same defect and the same three lines, but I can split them out if you prefer. A normal numeric value returns exactly what it did before.

Tests

  • TiffFieldTest: getIntValue, getLongValue and getDoubleValue throw ImagingException for a BYTE field with count 0; a numeric field still returns its value (control).
  • TiffFieldTest.testGetBufferedImageWithNonNumericFillOrder: the 438-byte file attached to the ticket (added as images/tiff/IMAGING-386/crash-46f4...tif) now gives an ImagingException from Imaging.getBufferedImage.

Without the change 4 of the 5 tests fail with ClassCastException; with it all 1264 tests pass. Verified locally with the default mvn goal on Java 17 (RAT, Checkstyle, SpotBugs, PMD, Javadoc) and the test suite on Java 8. This PR is independent of #559 (IMAGING-385); the two touch different classes.

Before you push a pull request, review this list:

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute? Claude (Anthropic) helped draft the fix and the tests; I reviewed all of it, ran the builds and checked that the new tests fail without the change.
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

…n-number values

TiffField.getIntValue() cast the field value to Number without checking it.
A BYTE field whose count is not 1 holds a byte[], so a malformed TIFF that
declares FillOrder that way made Imaging.getBufferedImage() fail with a
ClassCastException instead of the documented ImagingException.

Check the type once in a private getNumberValue() and use it from
getIntValue(), getLongValue() and getDoubleValue(), which had the same
unchecked cast. The null check and the returned values are unchanged.
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.

1 participant