Skip to content

Commit 806f7fc

Browse files
committed
Refactor express.json to only be used on necessary routes
The express-unless library ended up being a cognitive risk to easily understanding when a json body needs to be parsed and under which routes and conditions. By modifying our router logic to only execute express.json() when the endpoint really needs it, we can avoid this problem and improve code intent and clarity. Signed-off-by: Jeremy Ho <jujaga@gmail.com>
1 parent 52e737b commit 806f7fc

File tree

6 files changed

+10
-29
lines changed

6 files changed

+10
-29
lines changed

app/app.js

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ const compression = require('compression');
33
const config = require('config');
44
const cors = require('cors');
55
const express = require('express');
6-
const { unless } = require('express-unless');
76
const { ValidationError } = require('express-validation');
87

98
const { AuthMode, DEFAULTCORS } = require('./src/components/constants');
@@ -31,17 +30,8 @@ let probeId;
3130
let queueId;
3231

3332
const app = express();
34-
const jsonParser = express.json();
35-
jsonParser.unless = unless;
3633
app.use(compression());
3734
app.use(cors(DEFAULTCORS));
38-
app.use(jsonParser.unless({
39-
path: [{
40-
// Matches on only the createObject and updateObject endpoints
41-
url: /.*(?<!permission)\/object(\/[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12})?(\/)?(\?.*)?$/i,
42-
methods: ['PUT']
43-
}]
44-
}));
4535
app.use(express.urlencoded({ extended: true }));
4636

4737
// Skip if running tests

app/package-lock.json

Lines changed: 0 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
"date-fns": "^2.30.0",
4242
"express": "^4.18.2",
4343
"express-basic-auth": "^1.2.1",
44-
"express-unless": "^2.1.3",
4544
"express-validation": "^4.1.0",
4645
"express-winston": "^4.2.0",
4746
"js-yaml": "^4.1.0",

app/src/routes/v1/bucket.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
const router = require('express').Router();
1+
const express = require('express');
2+
const router = express.Router();
23

34
const { Permissions } = require('../../components/constants');
45
const { bucketController, syncController } = require('../../controllers');
@@ -10,7 +11,7 @@ router.use(checkAppMode);
1011
router.use(requireSomeAuth);
1112

1213
/** Creates a bucket */
13-
router.put('/', bucketValidator.createBucket, (req, res, next) => {
14+
router.put('/', express.json(), bucketValidator.createBucket, (req, res, next) => {
1415
bucketController.createBucket(req, res, next);
1516
});
1617

@@ -35,7 +36,7 @@ router.get('/', bucketValidator.searchBuckets, (req, res, next) => {
3536
});
3637

3738
/** Updates a bucket */
38-
router.patch('/:bucketId', bucketValidator.updateBucket, hasPermission(Permissions.UPDATE), (req, res, next) => {
39+
router.patch('/:bucketId', express.json(), bucketValidator.updateBucket, hasPermission(Permissions.UPDATE), (req, res, next) => {
3940
bucketController.updateBucket(req, res, next);
4041
});
4142

app/src/routes/v1/permission/bucketPermission.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
const router = require('express').Router();
1+
const express = require('express');
2+
const router = express.Router();
23

34
const { Permissions } = require('../../../components/constants');
45
const { bucketPermissionController } = require('../../../controllers');
@@ -20,7 +21,7 @@ router.get('/:bucketId', bucketPermissionValidator.listPermissions, currentObjec
2021
});
2122

2223
/** Grants bucket permissions to users */
23-
router.put('/:bucketId', bucketPermissionValidator.addPermissions, currentObject, hasPermission(Permissions.MANAGE), (req, res, next) => {
24+
router.put('/:bucketId', express.json(), bucketPermissionValidator.addPermissions, currentObject, hasPermission(Permissions.MANAGE), (req, res, next) => {
2425
bucketPermissionController.addPermissions(req, res, next);
2526
});
2627

app/src/routes/v1/permission/objectPermission.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
const router = require('express').Router();
1+
const express = require('express');
2+
const router = express.Router();
23

34
const { Permissions } = require('../../../components/constants');
45
const { objectPermissionController } = require('../../../controllers');
@@ -20,7 +21,7 @@ router.get('/:objectId', objectPermissionValidator.listPermissions, currentObjec
2021
});
2122

2223
/** Grants object permissions to users */
23-
router.put('/:objectId', objectPermissionValidator.addPermissions, currentObject, hasPermission(Permissions.MANAGE), (req, res, next) => {
24+
router.put('/:objectId', express.json(), objectPermissionValidator.addPermissions, currentObject, hasPermission(Permissions.MANAGE), (req, res, next) => {
2425
objectPermissionController.addPermissions(req, res, next);
2526
});
2627

0 commit comments

Comments
 (0)