Project

Profile

Help

Bug #5447

closed

Failure in bin:pack-integer with little-endian negative integer

Added by Michael Kay about 2 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
EXSLT extensions
Sprint/Milestone:
-
Start date:
2022-04-04
Due date:
% Done:

100%

Estimated time:
Legacy ID:
Applies to branch:
10, 11, trunk
Fix Committed on Branch:
10, 11, trunk
Fixed in Maintenance Release:
Platforms:

Description

At line 1100, in method packBytes (called by pack-integer), is the loop

               while (length > 0)
                    result[i++] = (byte) (255);

which is clearly wrong because the condition doesn't change within the loop.

It's possible that this code is unreachable; more likely, it's not reached by any of our test cases. We have relatively few tests for bin:pack-integer on negative numbers, but it's a complicated path.

Actions #1

Updated by Michael Kay about 2 years ago

The bug can be very easily demonstrated with the test

Q{http://expath.org/ns/binary}pack-integer(-23, 4, 'LE')

(Essentially, any attempt to call pack-integer with a negative value in littleEndian format.

It doesn't cause an infinite loop, it loops until it hits an array-bounds failure:

java.lang.ArrayIndexOutOfBoundsException: Index 4 out of bounds for length 4
	at com.saxonica.functions.extfn.EXPathBinaryFunctionSet.packBytes(EXPathBinaryFunctionSet.java:1096)
	at com.saxonica.functions.extfn.EXPathBinaryFunctionSet.access$800(EXPathBinaryFunctionSet.java:24)
	at com.saxonica.functions.extfn.EXPathBinaryFunctionSet$BinaryPackInteger.call(EXPathBinaryFunctionSet.java:589)

It's rather surprising that there are no test cases that catch this. The "binary" test set from John Lumley appears to have no tests that exercise negative numbers in pack-integer. The "binary2" test set from Christian GrĂ¼n does have tests for negative numbers, but none of them are little-endian.

Actions #2

Updated by Michael Kay about 2 years ago

  • Subject changed from Code smell in EXPathBinaryFunctionSet to Failure in bin:pack-integer with little-endian negative integer
  • Status changed from New to In Progress

Fix tested on main development branch.

Leaving open until tests are updated and the fix is ported to other branches.

Actions #3

Updated by Michael Kay about 2 years ago

  • Category set to EXSLT extensions
  • Status changed from In Progress to Resolved
  • Priority changed from Low to Normal
  • Applies to branch 10, 11, trunk added
  • Fix Committed on Branch 10, 11, trunk added

New test cases committed to expath test suite (which has now been separated into its own repository).

Patch copied to the 10.x and 11.x branches.

Actions #4

Updated by Debbie Lockett over 1 year ago

  • Fixed in Maintenance Release 11.4 added

Bug fix applied in the Saxon 11.4 maintenance release. (Issue remains open awaiting Saxon 10 maintenance release.)

Actions #5

Updated by Michael Kay over 1 year ago

See also bug #5690, which has led to revisiting this bug and retrofitting the 11.x code for pack-integer to 10.x.

Actions #6

Updated by O'Neil Delpratt over 1 year ago

  • Fixed in Maintenance Release 12.0 added

Bug issue fix applied in the Saxon 12.0 Major Release. Leaving this bug marked as Resolved until fix applied to Saxon 10 branch.

Actions #7

Updated by O'Neil Delpratt about 1 year ago

  • Status changed from Resolved to Closed
  • % Done changed from 0 to 100
  • Fixed in Maintenance Release 10.9 added

Bug fix applied in the Saxon 10.9 maintenance release.

Please register to edit this issue

Also available in: Atom PDF