Skip to content

Handle uncaught NumberFormatException at 20 parse sites (CodeQL java/uncaught-number-format-exception)#229

Open
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/codeql-uncaught-nfe
Open

Handle uncaught NumberFormatException at 20 parse sites (CodeQL java/uncaught-number-format-exception)#229
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/codeql-uncaught-nfe

Conversation

@vharseko

@vharseko vharseko commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

Resolves all 20 java/uncaught-number-format-exception CodeQL alerts (11 modules).

Each flagged site parsed an int/long/double from a value that could reach
it malformed, with no surrounding catch, so bad input escaped as a raw
NumberFormatException. Every site is now routed to the failure mode that
already matches its method contract, so callers get a controlled result rather
than a leaking runtime exception. Behaviour is unchanged for valid input.

Fixes by category

Untrusted / client input → proper error

File Change
MemoryBackend Invalid pagedResultsCookieBadRequestException (HTTP 400) instead of 500; Cookie.valueOf now declares throws ResourceException and the caller returns e.asPromise().
ElasticsearchAuditEventHandler Invalid pagedResultsCookieBadRequestException.asPromise() (400) instead of 500.
QueryFilterParser Non-numeric assertion value → the parser's existing valueOfIllegalArgument(tokenizer) result.
JwtClaimsSet Non-numeric exp string claim → JwtRuntimeException (the class's own type-check exception) with a clear message.

Configuration input → clear message / documented default

File Change
AbstractJwtSessionModule Token-idle-time / max-token-life settings parsed via a helper that throws AuthenticationException naming the bad setting.
AsynchronousTextWriter Invalid CAPACITY system property → warn + fall back to default 32000 (no longer fails class init).
RhinoScriptEngine Invalid recompile-interval config → warn + fall back to documented default -1.
CrestIntegerSchema Non-integer exampleValidationException (the Crest schema idiom).

Sentinel-returning parsers → existing sentinel

File Change
KeyParser.unicode() Malformed \uXXXX escape → the method's existing -1 "cannot parse" sentinel.
JournalManager.fileToGeneration() Non-numeric generation → the method's existing -1 (matches the no-match branch).
Warning.valueOf() Non-numeric warn-code → null, matching the documented "null if it could not be parsed" contract.

Internal, developer-controlled specs → loud but described failure

File Change
ManagementTableModel.setup() Malformed column spec → IllegalArgumentException naming the offending spec.
AdminUI.addLabeledField() Malformed component width/height spec → IllegalArgumentException naming the value.

Notes

Warning.valueOf ([0-9]{3}) and JournalManager.fileToGeneration (\d{12})
parse regex-guaranteed digit groups that cannot actually overflow today; the
guards route to each method's existing failure return, so they are cheap
hardening against future regex drift rather than dead code.

Testing

mvn -o -am compile across all eleven affected modules — BUILD SUCCESS.

…uncaught-number-format-exception)

Each site called Integer/Long/Double parsing on a value that reaches it
without a surrounding catch, so malformed input escaped as a raw
NumberFormatException. Every site is now routed to the failure mode that
already fits its method contract, so callers get a controlled result
instead of a leaking runtime exception.

Untrusted / client input -> proper error:
- MemoryBackend.Cookie.valueOf: an invalid pagedResultsCookie now yields
  BadRequestException (HTTP 400) instead of a 500; the caller maps it to
  a failed promise.
- ElasticsearchAuditEventHandler.queryEvents: an invalid pagedResultsCookie
  now returns BadRequestException.asPromise() (400) instead of a 500.
- QueryFilterParser: a non-numeric assertion value now returns the parser's
  existing valueOfIllegalArgument() result instead of throwing.
- JwtClaimsSet: a non-numeric "exp" string claim now throws JwtRuntimeException
  (the class's own type-checking exception) with a clear message.

Configuration input -> clear message / documented default:
- AbstractJwtSessionModule: the token-idle-time and max-token-life settings
  are parsed via a helper that throws AuthenticationException naming the bad
  setting instead of a raw NumberFormatException.
- AsynchronousTextWriter: an invalid CAPACITY system property now logs a
  warning and falls back to the default (32000) instead of failing class init.
- RhinoScriptEngine: an invalid recompile-interval config value now logs a
  warning and falls back to the documented default (-1).
- CrestIntegerSchema.setExample: a non-integer example now throws
  ValidationException (the schema classes' idiom) instead of a raw NFE.

Sentinel-returning parsers -> existing sentinel:
- Key/KeyParser.unicode(): a malformed \uXXXX escape now returns the method's
  existing -1 "cannot parse" sentinel.
- JournalManager.fileToGeneration(): a non-numeric generation now returns the
  method's existing -1, matching the no-match branch.
- Warning.valueOf(): a non-numeric warn-code now returns null, matching the
  method's documented "null if it could not be parsed" contract.

Internal, developer-controlled specs -> loud but described failure:
- ManagementTableModel.setup() and AdminUI.addLabeledField(): a malformed
  column/component specification now throws IllegalArgumentException naming
  the offending spec instead of a bare NumberFormatException.

Behaviour is unchanged for valid input. Compiles: mvn -o -am compile across
all eleven affected modules -> BUILD SUCCESS.
@vharseko vharseko added bug java codeql CodeQL static-analysis findings labels Jul 8, 2026
@vharseko vharseko requested a review from maximthomas July 8, 2026 12:42
@vharseko vharseko removed the java label Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug codeql CodeQL static-analysis findings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants