Skip to content

Commit f4fcfdf

Browse files
committed
fix: add missing type checks to parameter validators
Add instanceof type checks to 3 validators that only checked parameter existence but not type (filter in Update/Delete, keys/indexName in DropIndex). Also add options type validation to the 5 validators for operations that support options (Insert, Update, CreateIndex, CreateView, RenameCollection). This closes the remaining ClassCastException gaps in MongoOperation getters — since validate() runs at load time via the TemplatePayload contract, the @SuppressWarnings("unchecked") casts are now effectively safe. Resolves ANALYSIS.md issue #4.
1 parent 6538255 commit f4fcfdf

9 files changed

Lines changed: 225 additions & 13 deletions

ANALYSIS.md

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,10 @@ YAML -> MongoOperation (deserialized) -> MongoOperationValidator -> MongoOperati
4343
**Original issue:** `modifyCollection` used `OperationValidator.NO_OP` in `MongoOperationType`, so none of its three parameters (`validator`, `validationLevel`, `validationAction`) were validated. A user could pass `validationLevel: "banana"` and it would pass validation, only to fail at MongoDB runtime.
4444
**Resolution:** `ModifyCollectionParametersValidator` now validates at load time: requires at least one recognized parameter, checks `validator` is a document (Map), checks `validationLevel` is one of `off`/`strict`/`moderate`, and checks `validationAction` is one of `error`/`warn`. Multiple errors accumulate. Wired into `MongoOperationType.MODIFY_COLLECTION` replacing the `NO_OP` validator.
4545

46-
### #4 - HIGH: Type-unsafe parameter extraction throughout MongoOperation
47-
**File:** `MongoOperation.java:46-115`
48-
**Impact:** Every parameter getter uses `@SuppressWarnings("unchecked")` with raw casts from `Map<String, Object>`. Examples:
49-
- `getDocuments()` (line 47): `(List<Map<String, Object>>) parameters.get("documents")` - throws `ClassCastException` if YAML has `documents: "not a list"`
50-
- `getKeys()` (line 54): `(Map<String, Object>) parameters.get("keys")` - same issue
51-
- `getFilter()` (line 68): same pattern
52-
- `getUpdate()` (line 108): same pattern
53-
54-
While the validator catches some of these cases, the validator runs only on the apply path (see #1). Additionally, `getOptions()` (line 62) is called by operators but NEVER validated by `MongoOperationValidator` - an unknown key in `options` produces no error.
55-
**Risk:** `ClassCastException` at runtime with unhelpful stack trace instead of a clear validation error.
56-
**Fix:** Either add type checking in the getters (return Optional/throw descriptive error), or ensure validation is exhaustive for all parameter access paths.
46+
### #4 - ~~HIGH: Type-unsafe parameter extraction throughout MongoOperation~~ RESOLVED
47+
**Status:** Fixed by adding type checks to all parameter validators.
48+
**Original issue:** Every parameter getter in `MongoOperation` uses `@SuppressWarnings("unchecked")` raw casts from `Map<String, Object>`, risking `ClassCastException` at runtime if YAML contained wrong-typed values. Additionally, `getOptions()` was called by operators but never validated.
49+
**Resolution:** All parameter validators now include `instanceof` type checks that run at load time via `MongoOperation.validate()` (the `TemplatePayload` contract). Specifically: `UpdateParametersValidator` and `DeleteParametersValidator` check that `filter` is a `Map`; `DropIndexParametersValidator` checks that `keys` is a `Map` and `indexName` is a `String`; and the 5 validators for operations that support options (`InsertParametersValidator`, `UpdateParametersValidator`, `CreateIndexParametersValidator`, `CreateViewParametersValidator`, `RenameCollectionParametersValidator`) check that `options` is a `Map` when present. Since the framework guarantees `validate()` runs before any operator executes, the `@SuppressWarnings("unchecked")` casts in getters are now effectively safe — they can never be reached with wrong-typed data.
5750

5851
### #5 - HIGH: CreateIndexOperator claims `transactional=true` but ignores the session
5952
**File:** `CreateIndexOperator.java:28,32-35`

src/main/java/io/flamingock/template/mongodb/validation/CreateIndexParametersValidator.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
5353
"'keys' cannot be empty"));
5454
}
5555

56+
Object options = params.get("options");
57+
if (options != null && !(options instanceof Map)) {
58+
errors.add(new TemplatePayloadValidationError("parameters.options",
59+
"'options' must be a document"));
60+
}
61+
5662
return errors;
5763
}
5864
}

src/main/java/io/flamingock/template/mongodb/validation/CreateViewParametersValidator.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
5050
"'pipeline' must be a list"));
5151
}
5252

53+
Object options = params.get("options");
54+
if (options != null && !(options instanceof Map)) {
55+
errors.add(new TemplatePayloadValidationError("parameters.options",
56+
"'options' must be a document"));
57+
}
58+
5359
return errors;
5460
}
5561
}

src/main/java/io/flamingock/template/mongodb/validation/DeleteParametersValidator.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,13 @@ public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
2929
List<TemplatePayloadValidationError> errors = new ArrayList<>();
3030
Map<String, Object> params = operation.getParameters();
3131

32-
if (params == null || !params.containsKey("filter")) {
32+
Object filter = params == null ? null : params.get("filter");
33+
if (filter == null) {
3334
errors.add(new TemplatePayloadValidationError("parameters.filter",
3435
"Delete operation requires 'filter' parameter"));
36+
} else if (!(filter instanceof Map)) {
37+
errors.add(new TemplatePayloadValidationError("parameters.filter",
38+
"'filter' must be a document"));
3539
}
3640

3741
return errors;

src/main/java/io/flamingock/template/mongodb/validation/DropIndexParametersValidator.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,16 @@ public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
4343
"DropIndex operation requires either 'indexName' or 'keys' parameter"));
4444
}
4545

46+
if (indexName != null && !(indexName instanceof String)) {
47+
errors.add(new TemplatePayloadValidationError("parameters.indexName",
48+
"'indexName' must be a string"));
49+
}
50+
51+
if (keys != null && !(keys instanceof Map)) {
52+
errors.add(new TemplatePayloadValidationError("parameters.keys",
53+
"'keys' must be a map"));
54+
}
55+
4656
return errors;
4757
}
4858
}

src/main/java/io/flamingock/template/mongodb/validation/InsertParametersValidator.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,12 @@ public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
6161
}
6262
}
6363

64+
Object options = params.get("options");
65+
if (options != null && !(options instanceof Map)) {
66+
errors.add(new TemplatePayloadValidationError("parameters.options",
67+
"'options' must be a document"));
68+
}
69+
6470
return errors;
6571
}
6672
}

src/main/java/io/flamingock/template/mongodb/validation/RenameCollectionParametersValidator.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
4141
"'target' cannot be null or empty"));
4242
}
4343

44+
Object options = params.get("options");
45+
if (options != null && !(options instanceof Map)) {
46+
errors.add(new TemplatePayloadValidationError("parameters.options",
47+
"'options' must be a document"));
48+
}
49+
4450
return errors;
4551
}
4652
}

src/main/java/io/flamingock/template/mongodb/validation/UpdateParametersValidator.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,13 @@ public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
3535
return errors;
3636
}
3737

38-
if (!params.containsKey("filter")) {
38+
Object filter = params.get("filter");
39+
if (filter == null) {
3940
errors.add(new TemplatePayloadValidationError("parameters.filter",
4041
"Update operation requires 'filter' parameter"));
42+
} else if (!(filter instanceof Map)) {
43+
errors.add(new TemplatePayloadValidationError("parameters.filter",
44+
"'filter' must be a document"));
4145
}
4246

4347
Object update = params.get("update");
@@ -49,6 +53,12 @@ public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
4953
"'update' must be a document"));
5054
}
5155

56+
Object options = params.get("options");
57+
if (options != null && !(options instanceof Map)) {
58+
errors.add(new TemplatePayloadValidationError("parameters.options",
59+
"'options' must be a document"));
60+
}
61+
5262
return errors;
5363
}
5464
}

src/test/java/io/flamingock/template/mongodb/model/MongoOperationValidateTest.java

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,28 @@ void insertDocumentsWrongTypeTest() {
217217
assertTrue(errors.get(0).getMessage().contains("must be a list"));
218218
}
219219

220+
@Test
221+
@DisplayName("WHEN insert options is wrong type THEN validation fails")
222+
void insertOptionsWrongTypeTest() {
223+
MongoOperation op = new MongoOperation();
224+
op.setType("insert");
225+
op.setCollection("test");
226+
Map<String, Object> params = new HashMap<>();
227+
List<Map<String, Object>> docs = new ArrayList<>();
228+
Map<String, Object> doc = new HashMap<>();
229+
doc.put("name", "Test");
230+
docs.add(doc);
231+
params.put("documents", docs);
232+
params.put("options", "not a document");
233+
op.setParameters(params);
234+
235+
List<TemplatePayloadValidationError> errors = op.validate();
236+
237+
assertEquals(1, errors.size());
238+
assertEquals("parameters.options", errors.get(0).getField());
239+
assertTrue(errors.get(0).getMessage().contains("must be a document"));
240+
}
241+
220242
@Test
221243
@DisplayName("WHEN insert is valid THEN validation passes")
222244
void insertValidTest() {
@@ -310,6 +332,47 @@ void updateParamWrongTypeTest() {
310332
assertTrue(errors.get(0).getMessage().contains("must be a document"));
311333
}
312334

335+
@Test
336+
@DisplayName("WHEN update filter is wrong type THEN validation fails")
337+
void updateFilterWrongTypeTest() {
338+
MongoOperation op = new MongoOperation();
339+
op.setType("update");
340+
op.setCollection("test");
341+
Map<String, Object> params = new HashMap<>();
342+
params.put("filter", "not a document");
343+
Map<String, Object> update = new HashMap<>();
344+
update.put("$set", new HashMap<>());
345+
params.put("update", update);
346+
op.setParameters(params);
347+
348+
List<TemplatePayloadValidationError> errors = op.validate();
349+
350+
assertEquals(1, errors.size());
351+
assertEquals("parameters.filter", errors.get(0).getField());
352+
assertTrue(errors.get(0).getMessage().contains("must be a document"));
353+
}
354+
355+
@Test
356+
@DisplayName("WHEN update options is wrong type THEN validation fails")
357+
void updateOptionsWrongTypeTest() {
358+
MongoOperation op = new MongoOperation();
359+
op.setType("update");
360+
op.setCollection("test");
361+
Map<String, Object> params = new HashMap<>();
362+
params.put("filter", new HashMap<>());
363+
Map<String, Object> update = new HashMap<>();
364+
update.put("$set", new HashMap<>());
365+
params.put("update", update);
366+
params.put("options", "not a document");
367+
op.setParameters(params);
368+
369+
List<TemplatePayloadValidationError> errors = op.validate();
370+
371+
assertEquals(1, errors.size());
372+
assertEquals("parameters.options", errors.get(0).getField());
373+
assertTrue(errors.get(0).getMessage().contains("must be a document"));
374+
}
375+
313376
@Test
314377
@DisplayName("WHEN update missing both filter and update THEN both errors reported")
315378
void updateMissingBothTest() {
@@ -363,6 +426,23 @@ void deleteMissingFilterTest() {
363426
assertTrue(errors.get(0).getMessage().contains("requires 'filter'"));
364427
}
365428

429+
@Test
430+
@DisplayName("WHEN delete filter is wrong type THEN validation fails")
431+
void deleteFilterWrongTypeTest() {
432+
MongoOperation op = new MongoOperation();
433+
op.setType("delete");
434+
op.setCollection("test");
435+
Map<String, Object> params = new HashMap<>();
436+
params.put("filter", "not a document");
437+
op.setParameters(params);
438+
439+
List<TemplatePayloadValidationError> errors = op.validate();
440+
441+
assertEquals(1, errors.size());
442+
assertEquals("parameters.filter", errors.get(0).getField());
443+
assertTrue(errors.get(0).getMessage().contains("must be a document"));
444+
}
445+
366446
@Test
367447
@DisplayName("WHEN delete has empty filter THEN validation passes")
368448
void deleteEmptyFilterTest() {
@@ -464,6 +544,26 @@ void createIndexKeysWrongTypeTest() {
464544
assertTrue(errors.get(0).getMessage().contains("must be a map"));
465545
}
466546

547+
@Test
548+
@DisplayName("WHEN createIndex options is wrong type THEN validation fails")
549+
void createIndexOptionsWrongTypeTest() {
550+
MongoOperation op = new MongoOperation();
551+
op.setType("createIndex");
552+
op.setCollection("test");
553+
Map<String, Object> params = new HashMap<>();
554+
Map<String, Object> keys = new HashMap<>();
555+
keys.put("email", 1);
556+
params.put("keys", keys);
557+
params.put("options", "not a document");
558+
op.setParameters(params);
559+
560+
List<TemplatePayloadValidationError> errors = op.validate();
561+
562+
assertEquals(1, errors.size());
563+
assertEquals("parameters.options", errors.get(0).getField());
564+
assertTrue(errors.get(0).getMessage().contains("must be a document"));
565+
}
566+
467567
@Test
468568
@DisplayName("WHEN createIndex is valid THEN validation passes")
469569
void createIndexValidTest() {
@@ -500,6 +600,40 @@ void dropIndexMissingBothTest() {
500600
assertTrue(errors.get(0).getMessage().contains("'indexName' or 'keys'"));
501601
}
502602

603+
@Test
604+
@DisplayName("WHEN dropIndex keys is wrong type THEN validation fails")
605+
void dropIndexKeysWrongTypeTest() {
606+
MongoOperation op = new MongoOperation();
607+
op.setType("dropIndex");
608+
op.setCollection("test");
609+
Map<String, Object> params = new HashMap<>();
610+
params.put("keys", "not a map");
611+
op.setParameters(params);
612+
613+
List<TemplatePayloadValidationError> errors = op.validate();
614+
615+
assertEquals(1, errors.size());
616+
assertEquals("parameters.keys", errors.get(0).getField());
617+
assertTrue(errors.get(0).getMessage().contains("must be a map"));
618+
}
619+
620+
@Test
621+
@DisplayName("WHEN dropIndex indexName is wrong type THEN validation fails")
622+
void dropIndexIndexNameWrongTypeTest() {
623+
MongoOperation op = new MongoOperation();
624+
op.setType("dropIndex");
625+
op.setCollection("test");
626+
Map<String, Object> params = new HashMap<>();
627+
params.put("indexName", 123);
628+
op.setParameters(params);
629+
630+
List<TemplatePayloadValidationError> errors = op.validate();
631+
632+
assertEquals(1, errors.size());
633+
assertEquals("parameters.indexName", errors.get(0).getField());
634+
assertTrue(errors.get(0).getMessage().contains("must be a string"));
635+
}
636+
503637
@Test
504638
@DisplayName("WHEN dropIndex has indexName THEN validation passes")
505639
void dropIndexWithIndexNameTest() {
@@ -569,6 +703,24 @@ void renameCollectionEmptyTargetTest() {
569703
assertTrue(errors.get(0).getMessage().contains("cannot be null or empty"));
570704
}
571705

706+
@Test
707+
@DisplayName("WHEN renameCollection options is wrong type THEN validation fails")
708+
void renameCollectionOptionsWrongTypeTest() {
709+
MongoOperation op = new MongoOperation();
710+
op.setType("renameCollection");
711+
op.setCollection("oldName");
712+
Map<String, Object> params = new HashMap<>();
713+
params.put("target", "newName");
714+
params.put("options", "not a document");
715+
op.setParameters(params);
716+
717+
List<TemplatePayloadValidationError> errors = op.validate();
718+
719+
assertEquals(1, errors.size());
720+
assertEquals("parameters.options", errors.get(0).getField());
721+
assertTrue(errors.get(0).getMessage().contains("must be a document"));
722+
}
723+
572724
@Test
573725
@DisplayName("WHEN renameCollection is valid THEN validation passes")
574726
void renameCollectionValidTest() {
@@ -656,6 +808,25 @@ void createViewPipelineWrongTypeTest() {
656808
assertTrue(errors.get(0).getMessage().contains("must be a list"));
657809
}
658810

811+
@Test
812+
@DisplayName("WHEN createView options is wrong type THEN validation fails")
813+
void createViewOptionsWrongTypeTest() {
814+
MongoOperation op = new MongoOperation();
815+
op.setType("createView");
816+
op.setCollection("testView");
817+
Map<String, Object> params = new HashMap<>();
818+
params.put("viewOn", "sourceCollection");
819+
params.put("pipeline", Collections.singletonList(new HashMap<>()));
820+
params.put("options", "not a document");
821+
op.setParameters(params);
822+
823+
List<TemplatePayloadValidationError> errors = op.validate();
824+
825+
assertEquals(1, errors.size());
826+
assertEquals("parameters.options", errors.get(0).getField());
827+
assertTrue(errors.get(0).getMessage().contains("must be a document"));
828+
}
829+
659830
@Test
660831
@DisplayName("WHEN createView is valid THEN validation passes")
661832
void createViewValidTest() {

0 commit comments

Comments
 (0)