Big signatures broken since 1.2.18

[lapo-luchini]

This apparently harmless change in eb875bc24b688ced9b7ea57e021b1c48cec32ea9 broke (some) digital signatures due to rounding:

      bf.append('[');
-     for (int i : range) bf.append(i).append(' ');
+     for (long i : range) bf.append(i).append(' ');
      bf.append(']');

…because ByteBuffer.append(int) exists, while ByteBuffer.append(long) does not and thus implicitly uses ByteBuffer.append(float), where the value doesn't fit exactly.

Final effect is a signature where this happens:

-/Filter/Adobe.PPKLite/Type/Sig/ByteRange [0 178 3704 225636843 ]
+/Filter/Adobe.PPKLite/Type/Sig/ByteRange [0 178 3704 225636848 ]

Notice that the fourth number gets rounded up to the next multiple of 16, this breaks the range calculation by actually going after the end of file, but in other instances it is rounded down and we get a signature "not covering the full document" and also broken in any case, as during the signature the range used was right, so the hash is different.

Smallest diff solution is using an explicit cast to double to avoid implicit cast to float:

      bf.append('[');
-     for (long i : range) bf.append(i).append(' ');
+     for (long i : range) bf.append((double) i).append(' ');
      bf.append(']');

…but I think that using doubles to print ints and longs is really ugly, and probably re-writing both those methods to print an integral number directly is cleaner.

[lapo-luchini]

According to StackOverflow the first integer that can't fit in a float is 2^24+1 = 16_777_217, which is the max file size that can be safely signed by OpenPDF 1.2.18-1.3.19.

[lapo-luchini]

BTW I'd never have understood this if I didn't notice that the IntelliJ debugger was translating code thus:

[andreasrosdal]

Thanks for reporting. Pull requests welcome.

[andreasrosdal]

Could you please create an unit test for this? That way, we are sure that signatures are correct and that it will not happen again.

[lapo-luchini]

@andreasrosdal I did in #386, but I fear that test is fairly specific to this bug, doesn't ensure signatures will always be good for any other possible breakage. To do that, I'd say we should use a real signature scheme which actually checks the hash (and that'd be more complex to test)… but I'd say still better to have this test that don't have it.

[andreasrosdal]

should use a real signature scheme which actually checks the hash (and that'd be more complex to test)

Yes, this would be great to have!