CAMEL-24407: simple predicate fails for long digital strings - #25945
CAMEL-24407: simple predicate fails for long digital strings#25945k-krawczyk wants to merge 1 commit into
Conversation
Numbers with more digits than a long can hold, such as bank account numbers, made comparisons throw NumberFormatException. isNumber only checks that the text is all digits, and the callers then parsed it with Long.parseLong or Integer.valueOf. Compare those as BigInteger instead. A number that does not fit in a long can never equal an int or long, so equality with one is false. A numeric literal in a simple predicate that does not fit in a long is now kept as literal text, so it takes the same comparison path. Co-authored-by: Claude <noreply@anthropic.com>
|
As an outside contributor I cannot set reviewers on this PR. @ammachado @davsclaus you have worked on the touched files most recently - would one of you take a look when you have a moment? Reported by Claude Code on behalf of Karol Krawczyk |
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
davsclaus
left a comment
There was a problem hiding this comment.
Review
This one touches core comparison logic in ObjectHelper, so I went a bit deeper than usual:
- Built
camel-supportandcamel-core-languageson the PR branch and ran the full test suites for both modules, plusTypeCoerceCompareTest,SimpleOperatorTest, and bothObjectHelperTestclasses (camel-core and camel-support) individually — all pass, zero failures across either module. - Ran
mvn install -DskipTests -Psourcecheckon both touched modules — clean. - Manually traced the rewritten
typeCoerceCompareStringStringmixed-type branch (int-format vs float-format strings, e.g."7.0"vs"7"), suspecting a regression to string-lexicographic fallback comparison. VerifiedisFloatingNumber()actually matches plain integer strings too (not just dotted ones), so the mixed-numeric path is still handled correctly — confirmed by the existingassertEquals(0, ObjectHelper.typeCoerceCompare(tc, "7.0", "7"))test still passing. - Searched the rest of the codebase (
core/,components/,dsl/,tooling/) for other call sites with the same "digit-check then unguardedLong.parseLong/Integer.valueOf" pattern this PR fixes. Found none — the only otherObjectHelper.isNumbercaller (camel-attachments) uses a safetryConvertTothat returnsnullinstead of throwing, so it isn't the same bug shape. - Checked git blame: the mixed int/float comparison logic being modified traces back to CAMEL-21109, a deliberate prior feature — this PR builds on it rather than reverting it.
- Confirmed JIRA CAMEL-24407 is
Bug, correctly assigned to the PR author.
Assessment
Solid, well-scoped fix. It correctly identifies that isNumber()'s "digits only" check doesn't imply "fits in a long," fixes all the affected comparison paths (==, </>, a numeric literal in the predicate itself, mixed String/Integer/Long), falls back to BigInteger only on the rare overflow path, and deliberately leaves isNumber() itself untouched since SimplePredicateParser and camel-attachments rely on its existing "digits only" meaning. Good test coverage across all the affected layers.
No blocking concerns.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
gnodet
left a comment
There was a problem hiding this comment.
Well-crafted bug fix for NumberFormatException when the Simple language compares numeric strings exceeding Long.MAX_VALUE (e.g. bank account numbers like 12345678901234567890).
The approach is correct, conservative, and backward-compatible:
- String-to-string comparisons fall back to
BigIntegerwhen values don't fit in a long - Numbers outside the long range correctly return
falsefor equality againstint/long, and useBigIntegerfor ordering - Numeric literals in predicates that don't fit in a long are kept as literal text, avoiding the
NumberFormatExceptionduring route building - The existing
isNumber()semantics are preserved (correctly —camel-attachmentsrelies on its "digits only" meaning), with the newisLongNumber()added alongside
The fix also addresses an additional bug beyond what the JIRA describes: typeCoerceIntLong previously called Integer.valueOf(rightValue) when leftValue was an Integer, which would throw for strings like "99999999999" that fit in a long but not an int. The new code promotes both sides to long, which is correct.
The restructuring of typeCoerceCompareStringString removes the explicit mixed integer + float code paths, but correctness is preserved because isFloatingNumber accepts integer strings (no dot required).
Test coverage is thorough across 4 test files covering equals, compare, and Simple predicate scenarios.
📋 PR Metadata
| Aspect | Current | Suggested |
|---|---|---|
| Labels | core |
+ bug |
| Milestone | (none) | 4.23.0 |
🔀 Backport Status
ObjectHelper and SimplePredicateParser) with no dependency changes, so backport risk is low. Consider backporting to camel-4.18.x (LTS).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
The
simplelanguage throwsNumberFormatExceptionwhen it compares numbers with more digitsthan a long can hold, such as bank account numbers.
ObjectHelper.isNumberonly checks that thetext is all digits, and the callers then parse it with
Long.parseLongorInteger.valueOf.The reproducer from the issue:
Scope
The failure is wider than the reported
==case:<,>,<=,>=between two such strings${header.Account1} == 12345678901234567890),which already fails while the route is being built
Integerwhen the string does not fit in an int(for example
"99999999999")Affected versions
Reproduced on
ObjectHelper.typeCoerceEquals/typeCoerceComparein 3.20.2, 4.10.7, 4.18.2,4.22.0 and current main. On 2.25.4 the same comparison returns
true, so this is a regressionintroduced in the Camel 3 line rather than in 4.
Fix
String to String comparisons fall back to
BigIntegerwhen the value does not fit in a long.A number outside the long range can never equal an int or a long, so equality against one is
false, and ordering against one is decided asBigInteger. A numeric literal in a predicatethat does not fit in a long is now kept as literal text, so it takes the same comparison path
as a header would.
isNumber()is left alone on purpose:SimplePredicateParserandcamel-attachmentsrely onits "digits only" meaning.
Tests
isLongNumbercases incamel-supportObjectHelperTestcamel-coreObjectHelperTestTypeCoerceCompareTestSimpleOperatorTestReported by Claude Code on behalf of Karol Krawczyk