LUCENE-9396: Improve truncation detection for points.

[jpountz]

TestAllFilesDetectTruncation still passes with this change.

[rmuir]

Should we keep the existing check also? It does more than just check length, it validates this "footer" too (e.g. detecting invalid bits).

[jpountz]

@rmuir I combined them in an overloaded retrieveChecksum(IndexInput, long) variant, what do you think?

[rmuir]

I like the new organization by having an optional parameter to retrieve. But I think the exception handling can be simpler here? I think it would be better to check the file's length and if its wrong throw exception, then call retrieveChecksum? Since we know the length beforehand, there isn't sense IMO in doing suppressed dances here.

[rmuir]

i'm proposing something like this for the new method:

if (expectedLength < footerLength()) {
  throw new IllegalArgumentException("expectedLength cannot be less than the footer length");
}
if (in.length() < expectedLength) {
  throw new CorruptIndexException("truncated file: length=" + in.length() + " but expectedLength==" + expectedLength, in);
} else if (in.length() > expectedLength) {
  throw new CorruptIndexException("file too long: length=" + in.length() + " but expectedLength==" + expectedLength, in);
}
return retrieveChecksum(in);

[jpountz]

I liked doing the suppressed dance to still get information about the shape of the footer, e.g. for the case when the footer would be correct but the length retrieved from the meta file would be incorrect, similarly to how checkFooter tells us that the checksum passed when reading a file failed even though the checksum is correct. No strong feelings, I don't mind removing it if you don't like it.

[rmuir]

How would the length in the meta file be incorrect? It comes from a checksum-verified file, right?

I think with the current PR, you'll get a somewhat confusing exception for a truncated file, probably something like this:

CorruptIndexException("misplaced codec footer (file truncated?): remaining=" + remaining + ", expected=" + expected + ", fp=" + in.getFilePointer(), in)
   |--- suppressed: new CorruptIndexException("truncated file: length=" + in.length() + " but expectedLength==" + expectedLength, in)

Since we actually know the length up-front, we can take advantage of that to just deliver the better exception: truncated file directly to the user?

[jpountz]

I think it's doing the opposite actually:

CorruptIndexException("truncated file: length=" + in.length() + " but expectedLength==" + expectedLength, in)
   |--- suppressed: CorruptIndexException("misplaced codec footer (file truncated?): remaining=" + remaining + ", expected=" + expected + ", fp=" + in.getFilePointer(), in)

[rmuir]

Yeah, you are right, its not that bad. But still i would prefer to just deliver the clear exception without adding a less-clear suppressed one.

[jpountz]

I implemented your suggestion.