fix: Feature flag boolean parsing error (#35551)

[gortiz-dotcms]

Summary

Fixes #35551 — Feature flag boolean parsing is case-sensitive.

• Backend (ConfigurationResource): recoveryFromConfig previously fell through to Config.getStringProperty for all unprefixed FEATURE_FLAG_* keys, returning raw strings ("True", "FALSE", "1") that the frontend could not reliably coerce. A new BOOLEAN_FEATURE_FLAGS set identifies every boolean flag in the whitelist. For those keys, a dedicated parseBooleanFlag helper now resolves the value to a native JSON Boolean, handling all industry-standard truthy/falsy variants (case-insensitive, whitespace-trimmed), and preserves the "NOT_FOUND" sentinel for keys that are not defined anywhere on the server. Unrecognised values log a WARN and resolve to false.
• Frontend (DotCustomEventHandlerService): The CONTENT_EDITOR2_ENABLED flag check was a strict string comparison (=== 'true') that silently breaks once the backend returns a native boolean. Updated to === true.
• Testability (ConfigurationResource): Added constructor injection of WebResource (package-private, following the same pattern as PageScannerResource and ReportIssueResource) so the endpoint can be tested without a running container.

Accepted truthy/falsy values

Input	Resolves to
true, True, TRUE, true	true
1	true
false, False, FALSE, false	false
0, `` (empty)	false
key not defined anywhere	"NOT_FOUND" (sentinel preserved for frontend opt-out logic)
any other string	false + WARN log

Test plan

• ConfigurationResourceTest (new unit test in dotCMS/src/test/java) — covers all truthy/falsy variants, NOT_FOUND sentinel, whitelist exclusion, and raw-string passthrough for non-flag keys. Runs without a container via mockStatic(Config.class) and injected WebResource.
• Existing dotcms-integration/.../ConfigurationResourceTest continues to pass unchanged.
• Manual reproduction: set FEATURE_FLAG_UVE_TOGGLE_LOCK=True (capitalised), restart, call GET /api/v1/configuration/config?keys=FEATURE_FLAG_UVE_TOGGLE_LOCK — response now contains native boolean true and the UVE lock/unlock button renders.

🤖 Generated with Claude Code

[claude]

Claude finished @gortiz-dotcms's task in 3m 48s —— View job

---

🔍 dotCMS Backend Review — Done

• Loaded dotCMS conventions (CLAUDE.md, JAVA_STANDARDS.md, DATABASE_PATTERNS.md, REST_API_PATTERNS.md, SECURITY_PRINCIPLES.md, rest/CLAUDE.md)
• Pulled the Java diff (433ea3e1…a2db5489) and read ConfigurationResource.java + the new unit test in full
• Ran four parallel sub-agent reviews: Security, Database, Java Standards, REST API
• Consolidated findings and posted the consolidated review comment (``)
• [⚠️] Formal gh pr review --request-changes was blocked by sandbox approval — I'm unable to submit formal PR reviews, see the FAQ. The review feedback is fully captured in the updated review comment.

Summary of findings

• 🟠 High — Unused import java.io.IOException; at ConfigurationResource.java:15
• 🟠 High — Stale @throws IOException Javadoc at ConfigurationResource.java:132
• 🟠 High — Missing Swagger @Operation / @ApiResponses / @Schema on /config endpoint (lines 134–141), now that the response shape includes a string "NOT_FOUND" sentinel + normalised "true"/"false" strings
• 🟡 Medium — keysQuery not validated with UtilMethods.isSet(...) (lines 149–151)
• ✅ Security, database/cache, and the rest of the REST surface verified clean. Rollback-safety improved by commit a2db5489 (wire format reverted to normalised strings + tolerant Angular compare).
  · Branch issue-35551-feature-flag-boolean-parsing · Review comment

[claude]

Pull Request Unsafe to Rollback!!!

• Category: M-3 — REST / GraphQL / Headless API Contract Change
• Risk Level: 🟡 MEDIUM
• Why it's unsafe: This PR changes the JSON response contract of the /api/v1/configuration?keys=... endpoint. For a fixed set of whitelisted feature-flag keys (the new BOOLEAN_FEATURE_FLAGS set — FEATURE_FLAG_EXPERIMENTS, DOTFAVORITEPAGE_FEATURE_ENABLE, FEATURE_FLAG_TEMPLATE_BUILDER_2, FEATURE_FLAG_SEO_IMPROVEMENTS, FEATURE_FLAG_SEO_PAGE_TOOLS, FEATURE_FLAG_EDIT_URL_CONTENT_MAP, FEATURE_FLAG_NEW_BINARY_FIELD, FEATURE_FLAG_ANNOUNCEMENTS, FEATURE_FLAG_NEW_EDIT_PAGE, FEATURE_FLAG_UVE_PREVIEW_MODE, FEATURE_FLAG_UVE_TOGGLE_LOCK, FEATURE_FLAG_UVE_STYLE_EDITOR, FEATURE_FLAG_PAGE_SCANNER, FEATURE_FLAG_UVE_LEGACY_SCRIPT_INJECTION, FEATURE_FLAG_NEW_BLOCK_EDITOR, CONTENT_EDITOR2_ENABLED) the value type in the JSON response changes from string ("true" / "false") to native JSON boolean (true / false). The Angular admin UI is updated in lock-step to compare with === true instead of === 'true'. After a rollback to N-1: (a) any browser- or CDN-cached N frontend bundle that performs === true will silently evaluate to false when the N-1 backend returns the string "true", causing those feature-flagged admin UI features to appear disabled until the bundle is refreshed; (b) any headless/external client that has built against the new native-boolean shape will break on the string response. There is no version negotiation on this endpoint and the change is bidirectional (N→N-1 and N-1→N consumers see incompatible shapes for the same keys).
• Code that makes it unsafe:
  • dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java — new BOOLEAN_FEATURE_FLAGS set (lines ~62-80) plus the new branch if (BOOLEAN_FEATURE_FLAGS.contains(key)) { return parseBooleanFlag(key); } inside getValueByKey and the new parseBooleanFlag(...) method that returns Boolean.TRUE / Boolean.FALSE instead of the original Config.getStringProperty(...) string.
  • core-web/apps/dotcms-ui/src/app/api/services/dot-custom-event-handler/dot-custom-event-handler.service.ts — the comparison response[FeaturedFlags.FEATURE_FLAG_CONTENT_EDITOR2_ENABLED] === true (replacing === 'true') demonstrates that the consuming code now expects a native boolean and will silently mis-evaluate the string returned by N-1.
• Alternative (if possible): Keep the wire format string-typed and fix the underlying capitalization/whitespace bug in the consumer instead: have the Angular service do a tolerant compare (String(value).trim().toLowerCase() === 'true') so the response shape stays backward-compatible. If the server-side normalization is preferred long-term, ship it in two phases: (N) normalize the string value (return "true" / "false" only) so cached N-1 clients keep working; (N+1, after N-1 is outside the rollback window) flip the return type to native boolean and update the consumer's comparison. This follows the same backward-compatible API-contract pattern called out in M-3.

[claude]

🔍 dotCMS Backend Review

[🟠 High] dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java:15

Unused import. The diff removed throws IOException from getConfigVariables (line 141), but the import was left behind. No other code in the file references IOException — this is dead code that will produce a Javadoc / static-analysis warning.

import java.io.IOException;

💡 Delete the import line.

---

[🟠 High] dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java:132

Stale @throws IOException Javadoc. The method signature on line 141 no longer declares throws IOException, but the Javadoc still advertises it. Javadoc generation will warn (@throws tag references checked exception not declared in the method signature) and the comment misleads readers.

 * @return
 * @throws IOException
 */
@Path("/config")

💡 Drop the @throws IOException line. The empty @param / @return placeholders above are also worth tightening while you're here.

---

[🟠 High] dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java:134-141

Missing Swagger @Operation / @ApiResponses / @Schema annotations on the /config endpoint. This PR materially changes the response shape: each value in the returned Map<String,Object> is now polymorphic — normalised lowercase string "true"/"false" for the 16 keys in BOOLEAN_FEATURE_FLAGS, the sentinel string "NOT_FOUND" for undefined flags, Integer for number:-prefixed keys, List<String> for list:-prefixed keys, native Boolean for boolean:-prefixed keys, raw String otherwise. Per dotCMS/src/main/java/com/dotcms/rest/CLAUDE.md, a Map<String,Object> payload requires @Schema(type = "object", description = "...") with a concrete description and example. Without it, the auto-generated openapi.yaml (verified by CI) cannot describe the contract, and SDK consumers will not know that flag values are stringly-typed booleans rather than native JSON booleans.

@Path("/config")
@GET
@JSONP
@NoCache
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
public final Response getConfigVariables(@Context final HttpServletRequest request,

💡 Add @Operation(summary = "...", description = "...") plus @ApiResponses(@ApiResponse(responseCode = "200", content = @Content(schema = @Schema(type = "object", description = "Map of configuration key to value. Values for keys in BOOLEAN_FEATURE_FLAGS are normalised lowercase strings 'true'/'false'; undefined flags return the sentinel 'NOT_FOUND'. 'number:' prefix returns Integer, 'list:' returns array of strings, 'boolean:' returns native boolean.")))). Then regenerate the yaml with ./mvnw compile -pl :dotcms-core -DskipTests and commit dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml alongside the Java change — CI verifies the committed file matches the build.

---

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java:149-151

keysQuery is not validated with UtilMethods.isSet(...) before use. The StringUtils.splitByCommas(keysQuery) call + if (null != keys) guard only catches null; an empty / whitespace-only ?keys= request silently returns {} with no signal that the input was malformed. This is the dotCMS REST convention documented in docs/backend/REST_API_PATTERNS.md. Low-impact today but a deviation from the documented standard.

@QueryParam("keys") final String keysQuery) {
    new WebResource.InitBuilder(webResource)...init();
    final String[] keys = StringUtils.splitByCommas(keysQuery);
    final Map<String,Object> resultMap = new LinkedHashMap<>();
    if (null != keys) {

💡 Replace the null check with UtilMethods.isSet(keysQuery) before splitting, e.g. if (!UtilMethods.isSet(keysQuery)) { return Response.ok(new ResponseEntityView(resultMap)).build(); } after the init() call.

---

Items verified clean

• Security: no findings. Auth posture (requiredBackendUser(true) + rejectWhenNoUser(true)) is unchanged. WHITE_LIST gate still runs before recoveryFromConfig — the new BOOLEAN_FEATURE_FLAGS branch cannot read arbitrary Config keys, and every key in BOOLEAN_FEATURE_FLAGS is also in WHITE_LIST. The Logger.warn lambda only logs the (constant) flag key and its property value — no PII, no secrets. No System.getProperty/System.getenv, no SQL, no hardcoded secrets.
• Database / cache: no findings. The changes are purely in-memory Config.getStringProperty reads — no SQL, no DotConnect, no Hibernate, no CacheLocator, no transactions.
• Java standards (other than items above): classic switch statement (Java 11 compatible — not a Java 14+ switch expression). Locale.ROOT used for case-folding. Logger.warn(Class, Supplier<String>) overload exists and the lambda only captures effectively-final locals (key, rawValue). No raw types. JUnit 5 + Mockito mockStatic is an established convention. The package-private test constructor + new WebResource() pattern matches the documented dotCMS REST test pattern (PageScannerResource, ReportIssueResource).
• REST surface (other than items above): @NoCache present on the read endpoint. The new WebResource.InitBuilder(WebResource) invocation still terminates in .init(). No empty/silent catches. No stack traces leaked.

Rollback-safety note: the latest commit a2db5489 reverted the wire format from native JSON booleans to normalised lowercase strings ("true"/"false"), and the Angular consumer was updated to a tolerant compare (val === true || val === 'true'). This addresses the M-3 rollback risk that earlier reviews flagged — the response contract for boolean flag keys is now string-shaped both before and after the PR, just normalised case-insensitively after.

---

Next steps

• 🟠 High findings should be fixed before merging. The IOException import + Javadoc are mechanical; the missing Swagger annotations require regenerating openapi.yaml.
• 🟡 You can ask me to handle mechanical fixes inline: @claude drop the unused IOException import + stale @throws IOException Javadoc, add @Operation/@ApiResponses/@Schema(type="object") to ConfigurationResource.getConfigVariables, add a UtilMethods.isSet(keysQuery) guard, and regenerate openapi.yaml
• Every new push triggers a fresh review automatically

[claude]

Pull Request Unsafe to Rollback!!!

• Category: M-3 — REST / GraphQL / Headless API Contract Change

• Risk Level: 🟡 MEDIUM

• Why it's unsafe: This PR changes the JSON response value type of GET /api/v1/configuration/config?keys=... for a fixed set of whitelisted feature-flag keys (BOOLEAN_FEATURE_FLAGS: FEATURE_FLAG_EXPERIMENTS, DOTFAVORITEPAGE_FEATURE_ENABLE, FEATURE_FLAG_TEMPLATE_BUILDER_2, FEATURE_FLAG_SEO_IMPROVEMENTS, FEATURE_FLAG_SEO_PAGE_TOOLS, FEATURE_FLAG_EDIT_URL_CONTENT_MAP, FEATURE_FLAG_NEW_BINARY_FIELD, FEATURE_FLAG_ANNOUNCEMENTS, FEATURE_FLAG_NEW_EDIT_PAGE, FEATURE_FLAG_UVE_PREVIEW_MODE, FEATURE_FLAG_UVE_TOGGLE_LOCK, FEATURE_FLAG_UVE_STYLE_EDITOR, FEATURE_FLAG_PAGE_SCANNER, FEATURE_FLAG_UVE_LEGACY_SCRIPT_INJECTION, FEATURE_FLAG_NEW_BLOCK_EDITOR, CONTENT_EDITOR2_ENABLED). These keys now return native JSON booleans (true/false) instead of strings (\"true\"/\"false\"). The endpoint has no version negotiation, and the change is bidirectional — any external/headless consumer (mobile app, SDK, integration partner, custom Angular fork) that has built against the new native-boolean shape will silently mis-evaluate the string returned by N-1 after rollback (and vice versa for N-1 bundles cached on browsers/CDNs after upgrade to N).

  Mitigation already in place (reduces but does not eliminate risk): The admin Angular consumer at core-web/apps/dotcms-ui/src/app/api/services/dot-custom-event-handler/dot-custom-event-handler.service.ts was updated in this PR to a tolerant compare val === true || val === 'true', so the admin UI itself survives rollback. The residual risk is for any non-admin consumer of this endpoint.

• Code that makes it unsafe:

  • dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java — the new BOOLEAN_FEATURE_FLAGS set (lines ~70-87), the new branch in getValueByKey (if (BOOLEAN_FEATURE_FLAGS.contains(key)) { return parseBooleanFlag(key); }, lines ~187-189), and the new parseBooleanFlag method (lines ~199-220) that returns Boolean.TRUE/Boolean.FALSE instead of the original Config.getStringProperty(...) string.
  • core-web/apps/dotcms-ui/src/app/api/services/dot-custom-event-handler/dot-custom-event-handler.service.ts — val === true || val === 'true' confirms the consumer now expects a native boolean as the new wire format.

• Alternative (if possible): Two safer paths, in order of preference:

  1. Keep the wire format string-typed and harden the consumer — have the server keep returning \"true\"/\"false\" strings (after normalizing capitalization/whitespace server-side) and rely on the now-tolerant Angular compare (val === true || val === 'true'). The contract stays backward-compatible while the original capitalization bug is still fixed.
  2. Two-phase contract change — Release N: normalize the string value server-side (return \"true\"/\"false\" only) so cached N-1 clients keep working. Release N+1 (after N-1 is outside the rollback window): flip the return type to native boolean and update all consumers. This matches the standard M-3 backward-compatible contract pattern.