Skip to content

CAMEL-24407: simple predicate fails for long digital strings - #25945

Open
k-krawczyk wants to merge 1 commit into
apache:mainfrom
k-krawczyk:CAMEL-24407-simple-long-digital-strings
Open

CAMEL-24407: simple predicate fails for long digital strings#25945
k-krawczyk wants to merge 1 commit into
apache:mainfrom
k-krawczyk:CAMEL-24407-simple-long-digital-strings

Conversation

@k-krawczyk

Copy link
Copy Markdown
Contributor

The simple language throws NumberFormatException when it compares numbers with more digits
than a long can hold, such as bank account numbers. ObjectHelper.isNumber only checks that the
text is all digits, and the callers then parse it with Long.parseLong or Integer.valueOf.

The reproducer from the issue:

from("timer:test?repeatCount=1&delay=1000")
    .setHeader("Account1").constant("12345678901234567890")
    .setHeader("Account2").constant("12345678901234567890")
    .filter().simple("${header.Account1} == ${header.Account2}")
        .log(LoggingLevel.INFO, "Accounts equals")
    .end();

Scope

The failure is wider than the reported == case:

  • <, >, <=, >= between two such strings
  • a numeric literal in the expression itself (${header.Account1} == 12345678901234567890),
    which already fails while the route is being built
  • the same literal in quotes, so quoting is not a workaround
  • a String compared against an Integer when the string does not fit in an int
    (for example "99999999999")

Affected versions

Reproduced on ObjectHelper.typeCoerceEquals / typeCoerceCompare in 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 regression
introduced in the Camel 3 line rather than in 4.

Fix

String to String comparisons fall back to BigInteger when 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 as BigInteger. A numeric literal in a predicate
that 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: SimplePredicateParser and camel-attachments rely on
its "digits only" meaning.

Tests

  • isLongNumber cases in camel-support ObjectHelperTest
  • big number equality in camel-core ObjectHelperTest
  • big number ordering in TypeCoerceCompareTest
  • the reported scenario in SimpleOperatorTest

Reported by Claude Code on behalf of Karol Krawczyk

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>
@k-krawczyk

Copy link
Copy Markdown
Contributor Author

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

@github-actions

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@github-actions github-actions Bot added the core label Aug 31, 2026

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

This one touches core comparison logic in ObjectHelper, so I went a bit deeper than usual:

  • Built camel-support and camel-core-languages on the PR branch and ran the full test suites for both modules, plus TypeCoerceCompareTest, SimpleOperatorTest, and both ObjectHelperTest classes (camel-core and camel-support) individually — all pass, zero failures across either module.
  • Ran mvn install -DskipTests -Psourcecheck on both touched modules — clean.
  • Manually traced the rewritten typeCoerceCompareStringString mixed-type branch (int-format vs float-format strings, e.g. "7.0" vs "7"), suspecting a regression to string-lexicographic fallback comparison. Verified isFloatingNumber() actually matches plain integer strings too (not just dotted ones), so the mixed-numeric path is still handled correctly — confirmed by the existing assertEquals(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 unguarded Long.parseLong/Integer.valueOf" pattern this PR fixes. Found none — the only other ObjectHelper.isNumber caller (camel-attachments) uses a safe tryConvertTo that returns null instead 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 gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 BigInteger when values don't fit in a long
  • Numbers outside the long range correctly return false for equality against int/long, and use BigInteger for ordering
  • Numeric literals in predicates that don't fit in a long are kept as literal text, avoiding the NumberFormatException during route building
  • The existing isNumber() semantics are preserved (correctly — camel-attachments relies on its "digits only" meaning), with the new isLongNumber() 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

⚠️ The PR description confirms this bug was reproduced on 4.18.2 and is a regression from Camel 3. The fix touches only core utility code (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

gnodet added a commit to gnodet/camel that referenced this pull request Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants