diff --git a/src/common/pipes/filter-validation.pipe.ts b/src/common/pipes/filter-validation.pipe.ts index e2cc07ff0..dbe81b0b5 100644 --- a/src/common/pipes/filter-validation.pipe.ts +++ b/src/common/pipes/filter-validation.pipe.ts @@ -16,12 +16,18 @@ export class FilterValidationPipe implements PipeTransform { ) {} transform(inValue: string): string { const allAllowedKeys: string[] = [...this.allowedObjectKeys]; + let inValueParsed; for (const key in this.filters) { if (this.filters[key]) { allAllowedKeys.push(...this.allowedFilterKeys[key]); } } - const inValueParsed = JSON.parse(inValue ?? "{}"); + try { + inValueParsed = JSON.parse(inValue ?? "{}"); + } catch (err) { + const error = err as Error; + throw new BadRequestException(`Invalid JSON in filter: ${error.message}`); + } const flattenFilterKeys = Object.keys(flattenObject(inValueParsed)); const arbitraryObjectFields = [ "scientificMetadata", diff --git a/src/datasets/datasets.controller.ts b/src/datasets/datasets.controller.ts index 84897e79a..69edf24ed 100644 --- a/src/datasets/datasets.controller.ts +++ b/src/datasets/datasets.controller.ts @@ -155,6 +155,7 @@ export class DatasetsController { headers: Record, queryFilter: { filter?: string }, ) { + let filters: IFilters = {}; // NOTE: If both headers and query filters are present return error because we don't want to support this scenario. if (queryFilter?.filter && (headers?.filter || headers?.where)) { throw new HttpException( @@ -165,24 +166,23 @@ export class DatasetsController { }, HttpStatus.BAD_REQUEST, ); - } else if (queryFilter?.filter) { - const jsonQueryFilters: IFilters = - JSON.parse(queryFilter.filter); - - return jsonQueryFilters; - } else if (headers?.filter) { - const jsonHeadersFilters: IFilters = - JSON.parse(headers.filter); - - return jsonHeadersFilters; - } else if (headers?.where) { - const jsonHeadersWhereFilters: IFilters = - JSON.parse(headers.where); - - return jsonHeadersWhereFilters; + } else { + try { + if (queryFilter?.filter) { + filters = JSON.parse(queryFilter.filter); + } else if (headers?.filter) { + filters = JSON.parse(headers.filter); + } else if (headers?.where) { + filters = JSON.parse(headers.where); + } + } catch (err) { + const error = err as Error; + throw new BadRequestException( + `Invalid JSON in filter: ${error.message}`, + ); + } } - - return {}; + return filters; } updateMergedFiltersForList( diff --git a/test/DatasetFilter.js b/test/DatasetFilter.js index b4a3ccaa5..9c8d93b4b 100644 --- a/test/DatasetFilter.js +++ b/test/DatasetFilter.js @@ -1,4 +1,3 @@ -/* eslint-disable @typescript-eslint/no-var-requires */ "use strict"; const utils = require("./LoginUtils"); @@ -96,7 +95,7 @@ describe("0400: DatasetFilter: Test retrieving datasets using filtering capabili before(() => { db.collection("Dataset").deleteMany({}); }); - beforeEach(async() => { + beforeEach(async () => { accessTokenAdminIngestor = await utils.getToken(appUrl, { username: "adminIngestor", password: TestData.Accounts["adminIngestor"]["password"], @@ -792,7 +791,12 @@ describe("0400: DatasetFilter: Test retrieving datasets using filtering capabili const fields = { mode: {}, scientific: [ - { lhs: "test_field_1", relation: "GREATER_THAN_OR_EQUAL", rhs: 5, unit: "" }, + { + lhs: "test_field_1", + relation: "GREATER_THAN_OR_EQUAL", + rhs: 5, + unit: "", + }, ], }; return request(appUrl) @@ -814,7 +818,12 @@ describe("0400: DatasetFilter: Test retrieving datasets using filtering capabili const fields = { mode: {}, scientific: [ - { lhs: "test_field_1", relation: "LESS_THAN_OR_EQUAL", rhs: 6, unit: "" }, + { + lhs: "test_field_1", + relation: "LESS_THAN_OR_EQUAL", + rhs: 6, + unit: "", + }, ], }; return request(appUrl) @@ -836,7 +845,7 @@ describe("0400: DatasetFilter: Test retrieving datasets using filtering capabili const fields = { mode: {}, scientific: [ - { lhs: "test_field_1", relation: "RANGE", rhs: [5, 7], unit: "" }, + { lhs: "test_field_1", relation: "RANGE", rhs: [5, 7], unit: "" }, ], }; return request(appUrl) @@ -854,6 +863,24 @@ describe("0400: DatasetFilter: Test retrieving datasets using filtering capabili }); }); + it("0425: Should return informative error on malfored json is passed in filter", async () => { + const query = { where: { datasetName: { like: "correct test raw" } } }; + const malformedJson = JSON.stringify(query).replace("}", "},"); + + return request(appUrl) + .get(`/api/v3/datasets`) + .query({ filter: malformedJson }) + .auth(accessTokenAdminIngestor, { type: "bearer" }) + .expect(TestData.BadRequestStatusCode) + .expect("Content-Type", /json/) + .then((res) => { + res.body.should.have + .property("message") + .and.be.equal( + "Invalid JSON in filter: Expected double-quoted property name in JSON at position 52", + ); + }); + }); it("0430: should delete dataset 1", async () => { return request(appUrl) .delete("/api/v3/datasets/" + encodedDatasetPid1) diff --git a/test/DatasetV4.js b/test/DatasetV4.js index 4e7af7ef9..5e1337ec5 100644 --- a/test/DatasetV4.js +++ b/test/DatasetV4.js @@ -619,6 +619,29 @@ describe("2500: Datasets v4 tests", () => { .expect(TestData.BadRequestStatusCode) .expect("Content-Type", /json/); }); + + it("0209: should return informative error on malfored json is passed in filter", async () => { + const filter = { + limits: { + limit: 1, + }, + }; + const malformedJson = JSON.stringify(filter).replace("}", "},"); + + return request(appUrl) + .get(`/api/v4/datasets`) + .query({ filter: malformedJson }) + .auth(accessTokenAdminIngestor, { type: "bearer" }) + .expect(TestData.BadRequestStatusCode) + .expect("Content-Type", /json/) + .then((res) => { + res.body.should.have + .property("message") + .and.be.equal( + "Invalid JSON in filter: Expected double-quoted property name in JSON at position 22", + ); + }); + }); }); describe("Datasets v4 findOne tests", () => {