Skip to content

Validate configuration map entries before saving - #552

Open
tonygermano wants to merge 2 commits into
OpenIntegrationEngine:mainfrom
tonygermano:fix/520-config-map-validation
Open

tonygermano wants to merge 2 commits into
OpenIntegrationEngine:mainfrom
tonygermano:fix/520-config-map-validation

Conversation

@tonygermano

@tonygermano tonygermano commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

Summary

Fixes #520.

Neither configuration map PUT endpoint checks the body before saving it. The XML and JSON readers ignore the map's generic types, so a request can deserialize to a null map, or a map with a null or non-string key, or a null or non-ConfigurationProperty entry. DefaultConfigurationController.setConfigurationProperties saved the map before building the in-memory copy, so in database mode the bad map was written to the database and then the in-memory update threw. On the next startup the map failed to load and then read as empty. A null key didn't throw at all. It was persisted and broke the Administrator's Configuration Map tab.

PUT /configurationMap/{key} from #515 goes through the same method, so a <null/> body there does the same damage. @NicoPiel and @gibson9583 both reported that in their reviews of #515.

setConfigurationProperties now rejects a null map, and checks every raw entry's key and value type. If any check fails, it throws a ControllerException before saving anything. Both endpoints go through that method, so one check covers both.

Notes for reviewers

  • A rejected request still comes back as a 500, because the servlet wraps the ControllerException. What [BUG] Invalid configuration map PUT body is saved to the database, and the map is empty after a restart #520 asked for is that nothing gets saved and the map is unaffected. Returning 400 instead would mean checking the body in the servlets, or adding a new exception type mapped to 400. I've left that for a separate change. Let me know if you'd rather have it here.
  • The tests are in their own commit, before the fix, and they fail against the old code.

Testing

DefaultConfigurationControllerTest has new database-mode cases for a null map, a null entry, a null key, a non-string key, a non-ConfigurationProperty entry, and a null property sent to the single-entry endpoint. Each one checks that the stored database row and the in-memory map are both unchanged after the rejection.

Claude checked:

  • The new tests fail on the test commit and pass with the fix.
  • ./gradlew clean build -PdisableSigning=true passes.

Release Note

Invalid configuration map updates through the REST API no longer empty the map after a restart

With configurationmap.location = database, an invalid body sent to PUT /api/server/configurationMap was saved to the database before the request failed. Examples are a <null/> body, an entry with no value, or a key that isn't a string. The server kept using the old map until the next restart, so nothing looked wrong. After the restart the map couldn't be loaded and read as empty, so $cfg lookups returned nothing. If someone saved the Configuration Map panel in that state, the original entries were lost for good. A null key was also accepted and kept, and it stopped the Administrator's Configuration Map panel from loading. The server now checks every key and entry and rejects an invalid update before anything is saved. The same check applies to the new single-entry PUT /api/server/configurationMap/{key} endpoint. (#520, #552)

Rejected updates still return a 500, as they did before, but the stored map is no longer changed. File mode wasn't affected, because those bodies already failed before the file was written.

If your configuration map came up empty after a restart, and the first request for it after the restart failed with a NullPointerException or ClassCastException, the configuration.properties row for category core in the CONFIGURATION table probably holds a bad map. Fix or delete that row by hand and restart.


🤖 Drafted with Claude Code

Cover rejecting invalid input in database mode on both the single-entry
and whole-map paths: a null map, and a null or wrongly typed key or
entry.

These fail against the current code. Request bodies are deserialized
without their generic types, so a map can hold a null or a value of
any type. A null or wrongly typed entry is written to the database
before building the in-memory map throws, and on the next startup the
configuration map fails to load and then reads as empty. A null or
wrongly typed key is persisted the same way; a null key is accepted
outright, and the Administrator's configuration map tab sorts keys
with a case-insensitive comparator that cannot handle it.

Issue: OpenIntegrationEngine#520
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Tony Germano <tony@germano.name>
Request bodies for either configuration map PUT endpoint are
deserialized without their generic types, so setConfigurationProperties
can receive a null map, or a map whose keys or entries are null or of
the wrong type, for example from an XML body of <null/> or an entry
with an <int> key. These were persisted in database mode before the
in-memory update failed, leaving a database row that breaks loading
the map on the next startup. Check the raw entries first and throw a
ControllerException instead.

Fixes OpenIntegrationEngine#520

Issue: OpenIntegrationEngine#520
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Tony Germano <tony@germano.name>
@tonygermano
tonygermano force-pushed the fix/520-config-map-validation branch from 57485c4 to cbe54b6 Compare November 10, 2026 12:56
@github-actions

Copy link
Copy Markdown

Test Results

  142 files  ±0    142 suites  ±0   3m 32s ⏱️ +45s
  943 tests +6    943 ✅ +6  0 💤 ±0  0 ❌ ±0 
1 105 runs  +6  1 099 ✅ +6  6 💤 ±0  0 ❌ ±0 

Results for commit cbe54b6. ± Comparison against base commit 594c6fb.

@tonygermano
tonygermano marked this pull request as ready for review November 10, 2026 13:13
@tonygermano

tonygermano commented Nov 10, 2026 •

Copy link
Copy Markdown
Member Author

Rebased and marked ready for review. Updated the PR description to remove the message about this one begin stacked on #515 now that is has merged, and added the Release Note section.

This must merge before the next release to prevent #515 from being introduced with a transitive bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Invalid configuration map PUT body is saved to the database, and the map is empty after a restart

3 participants