Repository navigation
fix: Setup NPE when optional parameters are missing (#956) - #982
Open
ismisepaul wants to merge 2 commits into
Open
ismisepaul wants to merge 2 commits into
ismisepaul wants to merge 2 commits into
Conversation
Setup.doPost called equalsIgnoreCase on the raw dboverride, enableMongoChallenge and unsafeLevels parameters. A client that left one out (as the curl setup in the docs does) got HTTP 500 after the database settings had already been written. Compare with the constant first, so a missing parameter means "not enabled" / "neither override nor upgrade". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new test helper must create its configuration directory so regression tests can run independently on a clean build.
1 open issue
What changed in this PR
Fixes setup failures when optional parameters are omitted, addressing #956 without changing behavior for supplied values.
Changes:
- Uses null-safe comparisons for
dboverride,enableMongoChallenge, andunsafeLevels. - Adds integration tests for setup requests that omit these parameters.
| File | Description |
|---|---|
src/main/java/servlets/Setup.java |
Treats missing optional parameters as disabled or no override. |
src/it/java/servlets/SetupIT.java |
Adds a shared request helper and two regression tests. |
Review effort: Balanced
Review follow-up. Pass the optional setup parameters as a map instead of key/value varargs, and create the directory the setup auth file goes in, so the new tests also pass when SetupIT runs alone on a clean build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #956.
What
Setup.doPostcalledequalsIgnoreCaseon the rawenableMongoChallengeandunsafeLevelsparameters. A client that left either one out got HTTP 500 after setup had already written the database settings. The curl setup indocs/database-configuration.mdandAGENTS.mdleaves them out.dboverridehad the same problem. Leaving it out is valid: it means "neither override nor upgrade". So it's fixed here too, with its own test.All three now compare with the constant first (
"enable".equalsIgnoreCase(...)), so a missing parameter means "not enabled".Tests
Two new
SetupITtests post setup on an instance that isn't installed yet, using the DB settings from.env:enableMongoChallenge/unsafeLevelsdboverrideBoth fail on
devwith the NPE and pass with the fix. I also merged this branch with #959 (the setup advisory fix, which changes the same method) and ranSetupITandSetupAuthTeston the result. It merges cleanly and both pass.Every step of
test.ymlpassed locally (lint, build, PGP check, compose build, Tomcat config check, unit tests, integration tests).🤖 Generated with Claude Code