[vharseko]

Closes code scanning alerts #29, #30, #31 (java/unused-format-argument, warning). All three are real message bugs: the argument was supplied but never rendered.

Alert	Location	Before	After
#29	Heaplets.java	logger.warn("… instantiate %s as a Heaplet", c.getName(), e) — SLF4J logs a literal %s, the class name is lost	{}
#30	ResolveLocationJsonValueFunction.java	String.format("$location value ({}) cannot be null …", …) — exception message shows a literal {}	%s, filled with the raw (unevaluated) $location value, so "${missing}" is reported as $location value (${missing}) instead of (null)
#31	PolicyEnforcementFilter.java	"Returned resource ('{}' does not match current request URI (''))" — second placeholder missing, original never printed, unbalanced parentheses	('{}') … ('{}')

Tests added:

• HeapletsTest.shouldLogClassNameWhenHeapletCannotBeInstantiated — captures the WARN with a logback ListAppender via a heaplet whose constructor throws; failed before the fix with …instantiate [%s] as a Heaplet.
• ResolveLocationJsonValueFunctionTest.shouldReportTheValueWhenLocationIsNull — failed before the fix with $location value ({}) cannot be null ….
• ResolveLocationJsonValueFunctionTest.shouldReportTheExpressionWhenLocationEvaluatesToNull — "$location": "${missing}"; failed with $location value (null) … while the value was evaluated before fetch().

No test for #31: openig-openam runs its tests on slf4j-nop, so the log line cannot be captured without adding a logging backend to the module's test dependencies; the change is confined to the format string of a // Should never happen branch.

[maximthomas]

praise: Each fix puts the placeholder where the argument already was, and the two testable fixes get tests that fail at the base.

• Heaplets.java:86 now logs c.getName() through {} instead of a literal %s.
• PolicyEnforcementFilter.java:325 has two {} for its two arguments and balanced parentheses.
• HeapletsTest.shouldLogClassNameWhenHeapletCannotBeInstantiated captures the WARN with a logback ListAppender and asserts the exact text.

---

suggestion (non-blocking): The %s argument in ResolveLocationJsonValueFunction.fetch is always null, so the message never shows a $location value.

openig-core/src/main/java/org/forgerock/openig/util/ResolveLocationJsonValueFunction.java:104-108, :72

fetch() receives the node after as(evaluated(bindings)) (:72). as(url()) returns null only when the value isNull() (commons util JsonValueFunctions.TO_URL); a malformed or non-string value throws before this point. So {"$location": null} and {"$location": "${missing}"} both produce $location value (null) cannot be null (or evaluated to null). The expression that evaluated to null is lost, although the commit body says the exception now "reports the $location value". Passing the raw value through keeps the alert fixed and makes the message useful:

if (node.isMap() && node.size() == 1 && node.isDefined("$location")) {
    JsonValue location = node.get("$location");
    return fetch(location.as(evaluated(bindings)), location.getObject());
}

private JsonValue fetch(JsonValue location, Object expression) {
    URL url = location.as(url());
    if (url == null) {
        throw new JsonValueException(location,
                                     format("$location value (%s) cannot be null (or evaluated to null)",
                                            expression));
    }

Pin: a case with "$location": "${missing}" that asserts $location value (${missing}). It fails against the current code.

---

note (non-blocking): The commit text does not match the behaviour.

• openig-core/src/main/java/org/forgerock/openig/util/ResolveLocationJsonValueFunction.java: the commit says the exception now reports the $location value, but on this branch the value is always null (see the suggestion above).

[vharseko]

Both points are addressed in f2ea5a1; the branch is also rebased onto current master (no conflicts).

• suggestion: fetch() now takes the raw $location node and evaluates it itself (location.as(evaluated(bindings)).as(url())), so the message prints the unevaluated value. I kept the single-parameter signature instead of fetch(evaluated, raw): evaluated() keeps the node's pointer, so the JsonValueExceptions on the malformed-URL and IOException paths still point at the same node.
  Pin added: shouldReportTheExpressionWhenLocationEvaluatesToNull ("$location": "${missing}" → $location value (${missing}) …). On the previous head it failed with /$location: $location value (null) cannot be null (or evaluated to null).
• note: the new commit describes the actual behaviour, and the PR description (row fix ibm mq jms_text message processing #30 and the test list) is updated to match.

[maximthomas]

praise: Moving the $location evaluation into fetch() makes the #30 message report what the user configured, without changing anything else the exception shows.

• fetch() evaluates the value itself (openig-core/src/main/java/org/forgerock/openig/util/ResolveLocationJsonValueFunction.java:104) and formats the raw location.getObject() (:106-108), so "${missing}" is reported as $location value (${missing}); shouldReportTheExpressionWhenLocationEvaluatesToNull pins it.
• The exception's pointer is unchanged: ExpressionJsonTransformFunction.apply returns the evaluated value with the input's pointer (openig-core/src/main/java/org/forgerock/openig/util/ExpressionJsonTransformFunction.java:73), and JsonValueException.getMessage() renders only that pointer, so the IOException message at ResolveLocationJsonValueFunction.java:115 reads as before.