fix(Authoring): Reject request if project is not valid JSON

[eastagiletracker]

This PR proposes rejecting an authoring save request when the posted project content is not valid JSON, so a malformed body can no longer overwrite a unit's project.json on disk (Fixes #320). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/457. You can sign in with your GitHub ID to claim ownership of the project.

The defect

AuthorAPIController.saveProject writes the request body to disk before anything parses it:

projectService.evictProjectContentCache(project.getId());
projectService.saveProjectContentToDisk(projectJSONString, project);   // raw body -> project.json
projectService.updateMetadataAndLicenseIfNecessary(project, projectJSONString);
projectService.saveProjectToDatabase(project, user, projectJSONString); // first parse, throws here

The first parse of the body happens in saveProjectToDatabase, two calls after project.json has already been replaced. When an author posts content that is not valid JSON, the write succeeds, the JSONException is swallowed by the surrounding catch (Exception e), and the response is errorSavingProject — so the author is told the save failed while the stored unit has in fact been replaced with unparseable content. The content cache was evicted on the line before, so the next read comes off the damaged file rather than from memory.

The change

saveProject now parses the body first and returns before touching the cache or the file if it does not yield a JSON object. The guard sits after the existing canAuthorProject check, so the authorization behavior and its error code are unchanged.

I kept the returned message code as the existing errorSavingProject rather than introducing a new one, deliberately: handleSaveProjectResponse in WISE-Client dispatches on exactly notAllowedToEditThisProject and errorSavingProject, and a code outside that pair falls through both branches and would surface no message to the author at all. Reusing the existing code means the author-facing behavior is identical to today — the same "Error Saving Unit" banner — while the unit on disk survives.

Validation is new JSONObject(...), which is the same parse the downstream code already performs, so the guard accepts exactly the bodies the rest of the save path can handle. A JSON array is rejected along with malformed text, since saveProjectToDatabase and getMetadataFromProjectJSONString both require a top-level object.

Reproducing it at current HEAD

On develop at 1958196, with the four new tests applied but the controller guard reverted:

$ ./mvnw test -Dtest=AuthorAPIControllerTest
[ERROR] Tests run: 19, Failures: 4, Errors: 0, Skipped: 0
[ERROR]   AuthorAPIControllerTest.saveProject_whenProjectJSONIsMalformed_shouldNotSaveAndReturnErrorSavingProject
[ERROR]   AuthorAPIControllerTest.saveProject_whenProjectJSONIsNotAnObject_shouldNotSaveAndReturnErrorSavingProject
[ERROR]   AuthorAPIControllerTest.saveProject_whenProjectJSONIsEmpty_shouldNotSaveAndReturnErrorSavingProject
[ERROR]   AuthorAPIControllerTest.saveProject_whenProjectJSONIsNull_shouldNotSaveAndReturnErrorSavingProject

Each fails because the mocked ProjectService is never asked to write anything, yet the controller calls evictProjectContentCache and saveProjectContentToDisk with the malformed body. With the guard in place all four pass.

Verification

The four tests cover malformed JSON, a top-level array, an empty body, and a null body. The full suite was run on a clean tree before the change and again after it, on JDK 17:

before   Tests run: 410, Failures: 0, Errors: 21, Skipped: 0
after    Tests run: 414, Failures: 0, Errors: 21, Skipped: 0

The identical set of pre-existing failures appears in both runs and is untouched by this change — the Hibernate DAO tests plus WebSecurityConfigAuthorizationTest, which need a database this environment has none of (NoClassDefFoundError: Could not initialize class org.wise.portal.junit.AbstractTransactionalDbTests). No test that passed before this change fails after it, and AuthorAPIControllerTest goes from 15 passing to 19.

One note left deliberately out of scope: createProject writes projectJSONString to a new unit's project.json on the same unvalidated path. Guarding it means either changing that endpoint's String return contract or throwing, so it seemed better left to your judgment than bundled in here.

How this was managed

This work was tracked on a board imported from this repository's own issues and pull requests — 213 stories and 12 labels — with the fix carried by the story for this issue: fix(Authoring): Reject request if project is not valid JSON, on the board at https://eastagiletracker.com/projects/457.

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.

---

Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com