From d5e33ab1f22502d9aefd3b616a93139be835ce17 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Tue, 8 Nov 2022 15:25:02 +0100 Subject: [PATCH] Fix(export API): accept `true` and `false` as param values (#2349) ## What This PR updates the `paramToBool` function to first check whether the incoming argument is a boolean. This fixes the bug where using `true` or `false` (e.g. `Strategies=false`) in the query would cause the server to crash. I've also added a test case to check for these values. ## Why While working on the import/export API docs, I noticed that using `false` in an export request caused the server to crash. As we want to allow `true` and `false` (and use these values in the documentation), we should ensure that they work as expected. ## Background It's likely that this bug was introduced when we added the new OpenAPI query parameters to the export endpoint. Because of the way that the OpenAPI service we use does conversion, we now get `true` and `false` converted to actual boolean values instead of strings. The `paramToBool` function didn't account for that previously, which is why it caused the server to crash. --- src/lib/routes/admin-api/state.ts | 4 ++++ src/test/e2e/api/admin/state.e2e.test.ts | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/src/lib/routes/admin-api/state.ts b/src/lib/routes/admin-api/state.ts index a195bd6636..45499c02da 100644 --- a/src/lib/routes/admin-api/state.ts +++ b/src/lib/routes/admin-api/state.ts @@ -23,6 +23,10 @@ import { OpenAPIV3 } from 'openapi-types'; const upload = multer({ limits: { fileSize: 5242880 } }); const paramToBool = (param, def) => { + if (typeof param === 'boolean') { + return param; + } + if (param === null || param === undefined) { return def; } diff --git a/src/test/e2e/api/admin/state.e2e.test.ts b/src/test/e2e/api/admin/state.e2e.test.ts index 9cd69cab13..e743d47dfe 100644 --- a/src/test/e2e/api/admin/state.e2e.test.ts +++ b/src/test/e2e/api/admin/state.e2e.test.ts @@ -55,6 +55,12 @@ test('exports strategies and features as attachment', async () => { .expect(200); }); +test('accepts "true" and "false" as parameter values', () => { + return app.request + .get('/api/admin/state/export?strategies=true&tags=false') + .expect(200); +}); + test('imports strategies and features', async () => { return app.request .post('/api/admin/state/import')