Fix CodeQL note-severity alerts: array logging and uncaught NumberFormatException

[vharseko]

Second batch of note-severity code scanning fixes (after #814), covering 24 alerts of three rules.

Arrays logged as [Ljava.net.InetAddress;@1a2b3c (java/print-array, 5 alerts)

HostPort.isEquivalentTo() concatenated the InetAddress[] arrays into its trace message, and BrowserController concatenated the referral String[] into the displayed node description, so both printed the array identity instead of its content. Both now use Arrays.toString().

Missing space between concatenated literals (java/missing-space-in-concatenation, 2 alerts)

Two trace messages read "…has been definedin the associated password policy" and "…because readingits attributes failed".

Uncaught NumberFormatException on external input (java/uncaught-number-format-exception, 17 alerts)

Numbers coming from clients, replication peers, on-disk state and system properties are now parsed with the error channel the surrounding code already uses:

• MemoryBackend — the simple paged results cookie comes straight from the client, so a forged cookie failed the search with an unchecked exception. It is now rejected with a protocol error; the diagnostic message reports the cookie in hexadecimal form, following the EntryContainer precedent.
• GSERParser (both the SDK and the server copy) — a value which matches the GSER integer pattern but does not fit in an int is now reported as a decoding error rather than escaping the parser.
• CSN — the string constructor now validates the length and the hexadecimal fields; a short value previously raised StringIndexOutOfBoundsException. ByteArrayScanner.nextCSNUTF8() converts the new LocalizedIllegalArgumentException into the DataFormatException its callers expect, so a malformed CSN in a legacy replication message is handled like every other parsing error of that scanner (previously a 28-character non-hexadecimal CSN escaped as NumberFormatException). ChangelogBackend turns an invalid (replicationCSN=…) assertion value into invalidAttributeSyntax, mirroring the existing handling of changeNumber. The changelog also validates CSNs it reads back from disk: ReplicationEnvironment.readOfflineStateFile and FileReplicaDB.decodeKeyFromString report a corrupted value as a ChangelogException, the error both already declare. The ECL cookie path in MultiDomainServerState was already protected.
• SubtreeSpecification, SchemaUtils.parseRuleID — an out of range number in a client provided value is reported as a syntax error instead of an unchecked exception.
• ReplicationEnvironment reports a corrupted domains state file as a ChangelogException; BackupManager reports a base backup descriptor whose last log file size is missing or malformed (the previous code also raised NPE on a missing property), and traces and ignores a file suffix which is too large to have been generated by it (the suffix scan also quotes the base path before building its regex, so metacharacters in a backup path no longer misbehave); the JDBC backend falls back to the default connection TTL with a warning instead of failing class initialization when org.openidentityplatform.opendj.jdbc.ttl does not hold a non-negative number (a negative value used to reach Caffeine and raise ExceptionInInitializerError).
• backendstat (ID2Entry, ID2ChildrenCount) — the tool still prints the exception summary either way, but it now names the invalid entry ID (Invalid entry ID: "abc") instead of the raw For input string: "abc".
• GeneralizedTimeSyntax — false positive: the fraction buffer is seeded with "0." and the parse loop appends only ASCII digits, so the flagged Double.parseDouble cannot throw. The two statements are moved inside the existing try block only to close the alert; no behavior changes.

Alerts deliberately left open

• FixedTimeRotationPolicy (2) — false positive: the time-of-day property is constrained by the configuration definition to ^(([0-1][0-9])|([2][0-3]))([0-5][0-9])$, always four digits.
• CtsAccessTokenResolver (1) — false positive: the promise chain already has thenCatchRuntimeException which converts it into an access token exception.
• ProductInformation (4) — the values are read from a properties file generated by the build.
• The remaining alerts of this rule live in the command line tools, the control panel, the SNMP connection handler and the examples; they are left for a separate change.

Testing

New tests cover the new error branches: CSNTest (invalid string representations), ByteArrayTest (truncated and non-hexadecimal CSN in a replication message), GSERParserTestCase (integer overflow values), MemoryBackendTestCase (forged paged results cookie → protocolError).

• opendj-core: full test suite — 8176 tests, all passing.
• opendj-server-legacy (-Pprecommit): CSNTest (1253), SchemaBackendTestCase (165), GeneralizedTimeSyntaxTest (72), HostPortTest (37), TestSubtreeSpecification (35), ChangelogBackendTestCase (30), MultiDomainServerStateTest (20), ByteArrayTest (16), TestBackupAndRestore (12), BackupManagerTestCase (11), CSNGeneratorTest (4), ServerStateTest (4) — 1659 tests, all passing.

[maximthomas]

Solid batch — the "route the parse failure into the error channel the surrounding code already uses" pattern is applied faithfully, and the triage is accurate. Verified: both modules compile clean; every new checked exception reaches a caller that already declares it; no test depends on the old exception types; the alert inventory matches exactly (5 print-array + 2 missing-space + 17 uncaught-number-format = 24), and all three "left open" false-positive justifications hold (time-of-day really is regex-constrained to 4 digits, CtsAccessTokenResolver really does end in thenCatchRuntimeException, ProductInformation really does read build-generated properties).

Requesting changes on two one-liners.

ByteArrayScanner.nextCSNUTF8() no longer converts truncated CSNs (blocker)

opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/ByteArrayScanner.java:264-272 converts only IndexOutOfBoundsException. Before this PR, a truncated CSN hit str.substring(0, 16) → StringIndexOutOfBoundsException, which that catch converted. Now CSN throws LocalizedIllegalArgumentException (extends IllegalArgumentException), so it escapes unchecked:

                     pre-PR CSN                                         this PR
truncated "00000" →  DataFormatException: begin 0, end 16, length 5  →  LocalizedIllegalArgumentException: Invalid CSN: "00000"
28-char non-hex   →  NumberFormatException (already escaped)         →  LocalizedIllegalArgumentException (still escapes)

Reachable from AckMsg:164, UpdateMsg.decodeHeader:245, LDAPUpdateMsg:353/361, ByteArrayScanner:362 — peer-supplied bytes on legacy protocol versions. (ChangeTimeHeartbeatMsg is fine, it wraps in catch (RuntimeException).)

Runtime impact is low — Session.receive() catches RuntimeException and ServerReader/ReplicationBroker catch Exception, so the session is torn down either way — but it inverts this PR's own goal in the file it's otherwise hardening. The next method down (nextDN(), line 290) already has the right pattern:

    catch (LocalizedIllegalArgumentException | IndexOutOfBoundsException e)
    {
      throw new DataFormatException(e.getMessage());
    }

CachedConnection still dies on a negative TTL (blocker)

opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java guards non-numeric values but not negatives. Against the bundled Caffeine:

ttl="-1" -> IllegalArgumentException: duration cannot be negative: -1000000 NANOSECONDS

So -Dorg.openidentityplatform.opendj.jdbc.ttl=-1 still produces ExceptionInInitializerError — the exact failure mode the change removes, via a different bad value:

final long millis = Long.parseLong(ttl.trim());
if (millis >= 0) {
    return millis;
}
// fall through to the existing warning + DEFAULT_TTL_MS

Two claims in the description are inaccurate (docs)

• GeneralizedTimeSyntax is listed under real fixes, but the flagged Double.parseDouble cannot throw: finishDecodingFraction seeds the buffer with "0." and the switch appends only ASCII digits. Double.parseDouble("0." + 10000 nines) returns 1.0. Alert 1203 is a false positive; moving the two statements into the existing catch (Exception e) silences it without fixing anything. It belongs next to FixedTimeRotationPolicy, not next to SubtreeSpecification / SchemaUtils.parseRuleID — those two are genuinely reachable (INT_TOKEN is \d+; parseOID returns an unvalidated token that SchemaBackend:1340 feeds from a client modification value).
• backendstat does not "report an invalid entry ID instead of a stack trace" — dumpBackendTree/dumpStorageTree catch Exception and print ERR_BACKEND_TOOL_ERROR_READING_TREE.get(stackTraceToSingleLineString(e)). Only the message inside improves (Invalid entry ID: "abc" vs For input string: "abc").

No tests (minor)

None of the new error branches are covered. The test classes already have the hooks:

• CSNTest — the new length/hex contract. Highest value: this is exactly the class of breakage the nextCSNUTF8 issue above is.
• opendj-core/src/test/java/org/forgerock/opendj/ldap/GSERParserTestCase.java:109 — already has an invalid-integer block, add "99999999999".
• opendj-core/src/test/java/org/forgerock/opendj/ldap/MemoryBackendTestCase.java:506 — already builds a paged-results search, add a forged-cookie PROTOCOL_ERROR assertion.

Nits

• Cookie echoed raw: MemoryBackend.decodePagedResultsCookie puts the opaque client cookie into the diagnostic message. The server precedent handles this defensively — EntryContainer.java:1135 uses pageRequest.getCookie().toHexString() with UNWILLING_TO_PERFORM. Suggest pagedResults.getCookie().toHexString(). The catch also drops the cause e.
• Half-hardened ReplicationEnvironment: now that CSN has a documented contract, two on-disk-data sites are one line each from complete — ReplicationEnvironment.readOfflineStateFile:699 (catches only IOException though it declares throws ChangelogException) and FileReplicaDB.decodeKeyFromString:418.
• Silent fallback: BackupManager.parseSuffixNumber maps an unparseable suffix to 0. The reasoning is sound (regex is \d*, so a failure means overflow), but a silent 0 in a method that picks the next file number deserves a logger.trace.
• SubtreeSpecification.nextInt: new InputMismatchException(e.getMessage()) drops the cause, and the message is discarded by the isValid = false path anyway. Every other throw site in that class uses the no-arg form.
• ChangelogBackend.decodeCSN: returns a CSN both callers discard, and drops the cause. Mirrors decodeChangeNumber so it's consistent — but void validateCSN would read better.
• Dead API surface: GSERParser.nextInteger() has zero callers in either module (CertificateExactMatchingRuleImpl uses nextBigInteger), and the whole server-side org.opends.server.protocols.asn1.GSERParser has no consumers in main or test. Correct fixes, but API hardening rather than reachable defects — worth not reading the "external input" framing too broadly.
• CSN.valueOf(String) javadoc wasn't updated with the new @throws, though the constructor it delegates to was.
• Lazy validation: ReplicationEnvironment validates domain IDs in findNextDomainId rather than readDomainsStateFile, so a corrupt state file only surfaces when a new domain is created. The catch-all in readDomainsStateFile already uses the same message. Judgment call, not a defect.
• Pre-existing, adjacent: getHighestSuffixNumberForPath builds Pattern.compile(baseFile + "\\d*") with an unescaped path — regex metacharacters in a backup path would misbehave. Not introduced here; flagging only because the PR touches the method.

[vharseko]

Thanks for the thorough review — every point checked out. Addressed in d05cb26:

Blockers

• ByteArrayScanner.nextCSNUTF8() now converts LocalizedIllegalArgumentException | IndexOutOfBoundsException into DataFormatException, per the nextDN() pattern. This is also exactly what CI caught: ByteArrayTest.testByteArrayScanner_nextCSNUTF8_throwsExceptionWhenInvalidCSN was the single failure in all five ubuntu jobs.
• CachedConnection accepts only a non-negative TTL; a negative value now takes the same warning-and-default path as a non-numeric one.

Description — reworded as suggested: GeneralizedTimeSyntax is now listed as a false positive (the fraction buffer really is "0." plus ASCII digits only; the move into the try block only closes the alert), and the backendstat entry claims only the improved message inside the reported summary.

Nits applied

• ReplicationEnvironment.readOfflineStateFile and FileReplicaDB.decodeKeyFromString report a corrupted CSN as ChangelogException — the latter mirroring the existing FileChangeNumberIndexDB.decodeKeyFromString.
• MemoryBackend reports the cookie via toHexString() and keeps the cause.
• ChangelogBackend: void validateCSN, cause kept. (One correction: it has a single call site — decodeChangeNumber is the one with several.)
• BackupManager: logger.trace on the ignored suffix, and Pattern.quote(...) for the base path since the method was already being touched.
• CSN.valueOf(String) javadoc @throws added; SubtreeSpecification uses the no-arg InputMismatchException like the rest of the class.
• Left as-is per your notes: the lazy domain-id validation and the GSER API-surface framing.

Tests — all three suggested hooks plus one more: six invalid-string cases in CSNTest, "2147483648"/"99999999999" in GSERParserTestCase, a forged-cookie protocolError assertion in MemoryBackendTestCase, and a 28-character non-hexadecimal CSN case in ByteArrayTest.

Full runs locally: opendj-core 8176/0; the twelve opendj-server-legacy classes from the Testing section 1659/0.