Use skipBytes(long) in places where integer overflow is possible #3529
Conversation
| @@ -231,7 +231,7 @@ else if (TypeUINT12) { | |||
| int thisSeries = getSeries(); | |||
| for (int i=0; i<thisSeries; i++) { | |||
| setSeries(i); | |||
| in.skipBytes(getImageCount() * FormatTools.getPlaneSize(this)); | |||
| in.skipBytes((long) getImageCount() * FormatTools.getPlaneSize(this)); | |||
There was a problem hiding this comment.
This seems to have impacted the non-regression tests for one li-flim sample file - see https://merge-ci.openmicroscopy.org/jenkins/job/BIOFORMATS-test-folder/52532/console. We might need to look at the data with or without this PR to establish whether the configuration needs to be regenerated or there is an issue with this change.
There was a problem hiding this comment.
Reasonably certain that the configuration needs to be updated, as proposed here: https://github.com/openmicroscopy/data_repo_config/pull/445
Note that without that change, the MD5s for series 0 and series 1 were the same, which points to this PR fixing a problem.
There was a problem hiding this comment.
Went over all the modified readers and all proposed changes look sensible. Overall, I agree with this defensive approach both to prevent integer overflow as well as to fix real bugs as in the case of the li-flim sample. Happy for this PR to be merged for inclusion in the next patch release unless other @ome/formats members want to review it as well.
Inspired by #3519, #3303, and #3157. This uses
skipBytes(long)in more places where integer overflow seems plausible.I would expect this to be safe for a patch release and have no impact on tests. I don't anticipate this will fix any outstanding issues, it's just an attempt at being more defensive.