New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed issue with BigTiff vs non-BigTiff offset value packing #2247

Merged
merged 1 commit into from Jun 22, 2017

Conversation

Projects
None yet
3 participants
@lossyrob
Member

lossyrob commented Jun 22, 2017

Fixes #2241

The TIFF Spec has a condition that is an optimization, but has caused a lot of issues.

A TIFF-tag is structured like [code, type, length, offset]. The code determines what tag it is, the type is the datatype such as Byte or Double, the length is the number of elements in the array of values, and the offset is the offset in the file where the values live.

If the value of the data is a small size, the value can be packed into the space that stores the offset. In this way, the tiff tag reads as [code, type, length, value] instead. This only happens if the byte size of type, multiplied by the length of the data, is less than the byte size of the offset value. In the regular TIFF case, the offset is a 4-byte wide Int, so any value that can be stored in 4 or less bytes is stored in the offset field, instead of some other location in the file which is pointed to by the value in the offset field.

For BigTiff, the offset field is an 8-byte wide Long value. We accounted for this when we first implemented bigtiffs. However, we didn't account for the changes in the packing mechanism: now if the value of the tag is less than or equal to 8 bytes, that value can be packed into the offset field of the tiff tag. The problem with not catching this packing is that, if you don't know the offset actually refers to a packed value, then you read that value as an offset into the file. For many cases this will be a very large number that will point to some huge offset in the file that doesn't actually exist; this is where the strange errors around BigTiff tag reading were coming from.

This PR fixes this. The architecture of how we were utilizing implicit classes to add method extensions to ByteReader for reading arrays is clever but a bit unmalleable (I might have designed it, or at least didn't change it in the past, so that's on me). The type class I introduced was the least invasive way to implement this that I could think of; perhaps not perfect but it gets the job done. It changes technically public API, but in reality the API is internal enough that I can't imagine the change effecting any users (though I will note the API change in the CHANGELOG)

@lossyrob lossyrob added this to the 1.1 milestone Jun 22, 2017

@jbouffard

This comment has been minimized.

Contributor

jbouffard commented Jun 22, 2017

👍

@lossyrob lossyrob merged commit e86a5c5 into locationtech:master Jun 22, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
ip-validation
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment