Dhairya-007 opened a new pull request, #29676:
URL: https://github.com/apache/camel/pull/29676
## Problem
`NaturalSortComparator.isDigit` excludes the character it is meant to
include:
```java
private boolean isDigit(char c) {
return c >= 48 && c < 57; // 57 is '9'
}
```
`'9'` is therefore never treated as part of a number, so
`getNumSequenceLength` stops before it and the comparison falls back to
comparing character values. Any two strings whose digit runs start with a 9
sort wrongly:
```
before: [8, 10, 11, 100, 9, 90, camel-2.8.0, camel-2.10.0, camel-2.11.0,
camel-2.9.0,
node-0000000009, node-0000000008, node-0000000010]
after: [8, 9, 10, 11, 90, 100, camel-2.8.0, camel-2.9.0, camel-2.10.0,
camel-2.11.0,
node-0000000008, node-0000000009, node-0000000010]
```
(Produced by running the comparator before and after the change over the
same input.)
`node-0000000009` sorting before `node-0000000008` is the case that matters
here: ZooKeeper's sequential znode names are exactly what the children consumer
sorts with this comparator, and the class javadoc advertises version sorting,
where `camel-2.9.0` after `camel-2.11.0` is just as wrong.
The existing `NaturalSortComparatorTest` never caught it because no value in
its fixture contains a 9.
## Fix
- `return c >= '0' && c <= '9';` — same range, written so the boundary is
visible. `Character.isDigit` is deliberately not used: it would also accept
non-ASCII digits, which `getNumSequenceLength` and the digit-by-digit
comparison do not handle.
- `NaturalSortComparator(Order)` only assigned the field when the argument
was non-null, leaving `order` null otherwise, so every `compare()` threw an NPE
on `order.direction`. It now falls back to `Ascending`, matching the
no-argument constructor.
- Dropped the stale `// TODO first can contain null and the next line causes
NPE then`: the three null checks at the top of `compare()` already return
before that line.
## Tests
`NaturalSortComparatorTest` gets `9` and `90` added to the existing fixture,
plus three cases: version strings spanning 8/9/10/11, ZooKeeper-style
sequential node names, and `new NaturalSortComparator(null)` sorting ascending.
All four fail on `main`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
