Skip to content

Commit 99522c4

Browse files
committed
refactor how Content-Type headers are checked
1 parent a06037d commit 99522c4

10 files changed

Lines changed: 167 additions & 99 deletions

File tree

rest.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ const eitherContent = function (req, res, next) {
109109
if (mimeType === "text/plain" || mimeType === "application/json" || mimeType === "application/ld+json") return next()
110110
return next(createExpressError({
111111
statusCode: 415,
112-
statusMessage: `Unsupported Content-Type: ${contentType}. This endpoint requires text/plain.`
112+
statusMessage: `Unsupported Content-Type: ${contentType}. This endpoint requires application/json, application/ld+json, or text/plain.`
113113
}))
114114
}
115115

@@ -205,4 +205,4 @@ It may not have completed at all, and most likely did not complete successfully.
205205
res.status(error.status).send(error.message)
206206
}
207207

208-
export default { checkPatchOverrideSupport, jsonContent, textContent, messenger }
208+
export default { checkPatchOverrideSupport, jsonContent, textContent, eitherContent, messenger }

routes/__tests__/contentType.test.js

Lines changed: 149 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -2,193 +2,262 @@ import express from "express"
22
import request from "supertest"
33
import rest from '../../rest.js'
44

5-
// Set up a minimal Express app with the Content-Type validation middleware
5+
/**
6+
* Tests for the Content-Type validation middlewares: jsonContent, textContent, and eitherContent.
7+
* Each middleware is applied per-route rather than as a blanket middleware.
8+
*/
9+
10+
// Set up a minimal Express app mirroring the real app's body parsers
611
const routeTester = express()
712
routeTester.use(express.json({ type: ["application/json", "application/ld+json"] }))
813
routeTester.use(express.text())
914

10-
11-
// Mount the validateContentType middleware on /api just like api-routes.js
12-
routeTester.use("/api", rest.validateContentType)
13-
14-
// Simple JSON-only endpoint (like /api/create, /api/query, etc.)
15-
routeTester.post("/api/create", (req, res) => {
15+
// JSON-only endpoints (like /api/create, /api/query, /api/update, etc.)
16+
routeTester.post("/json-endpoint", rest.jsonContent, (req, res) => {
1617
res.status(200).json({ received: req.body })
1718
})
18-
19-
// Search endpoint that accepts text/plain
20-
routeTester.post("/api/search", (req, res) => {
19+
routeTester.put("/json-endpoint", rest.jsonContent, (req, res) => {
2120
res.status(200).json({ received: req.body })
2221
})
23-
24-
// GET endpoint should pass through without Content-Type validation
25-
routeTester.get("/api/info", (req, res) => {
26-
res.status(200).json({ info: true })
27-
})
28-
29-
// DELETE endpoint should pass through without Content-Type validation
30-
routeTester.delete("/api/delete/:_id", (req, res) => {
31-
res.status(200).json({ deleted: req.params._id })
22+
routeTester.patch("/json-endpoint", rest.jsonContent, (req, res) => {
23+
res.status(200).json({ received: req.body })
3224
})
3325

34-
// PUT endpoint (like /api/update, /api/bulkUpdate)
35-
routeTester.put("/api/update", (req, res) => {
26+
// Text-only endpoint
27+
routeTester.post("/text-endpoint", rest.textContent, (req, res) => {
3628
res.status(200).json({ received: req.body })
3729
})
3830

39-
// Release endpoint uses PATCH without a JSON body (uses Slug header)
40-
routeTester.patch("/api/release/:_id", (req, res) => {
41-
res.status(200).json({ released: req.params._id })
31+
// Either JSON or text endpoint (like /api/search)
32+
routeTester.post("/either-endpoint", rest.eitherContent, (req, res) => {
33+
res.status(200).json({ received: req.body })
4234
})
4335

4436
// Error handler matching the app's pattern
4537
routeTester.use(rest.messenger)
4638

47-
describe("Content-Type validation middleware", () => {
39+
describe("jsonContent middleware", () => {
4840

49-
it("accepts application/json with valid JSON body", async () => {
41+
it("accepts application/json", async () => {
5042
const response = await request(routeTester)
51-
.post("/api/create")
43+
.post("/json-endpoint")
5244
.set("Content-Type", "application/json")
5345
.send({ test: "data" })
5446
expect(response.statusCode).toBe(200)
5547
expect(response.body.received.test).toBe("data")
5648
})
5749

58-
it("accepts application/ld+json with valid JSON body", async () => {
50+
it("accepts application/ld+json", async () => {
5951
const response = await request(routeTester)
60-
.post("/api/create")
52+
.post("/json-endpoint")
6153
.set("Content-Type", "application/ld+json")
54+
// Must stringify manually; supertest's .send(object) would override Content-Type to application/json
6255
.send(JSON.stringify({ "@context": "http://example.org", test: "ld" }))
6356
expect(response.statusCode).toBe(200)
6457
expect(response.body.received["@context"]).toBe("http://example.org")
6558
})
6659

6760
it("accepts application/json with charset parameter", async () => {
6861
const response = await request(routeTester)
69-
.post("/api/create")
62+
.post("/json-endpoint")
7063
.set("Content-Type", "application/json; charset=utf-8")
7164
.send({ test: "charset" })
7265
expect(response.statusCode).toBe(200)
7366
})
7467

7568
it("accepts application/ld+json with charset parameter", async () => {
7669
const response = await request(routeTester)
77-
.post("/api/create")
70+
.post("/json-endpoint")
7871
.set("Content-Type", "application/ld+json; charset=utf-8")
72+
// Must stringify manually; supertest's .send(object) would override Content-Type to application/json
7973
.send(JSON.stringify({ "@context": "http://example.org", test: "ld-charset" }))
8074
expect(response.statusCode).toBe(200)
8175
})
8276

83-
it("returns 415 for missing Content-Type header", async () => {
77+
it("accepts Content-Type with unusual casing", async () => {
78+
const response = await request(routeTester)
79+
.post("/json-endpoint")
80+
.set("Content-Type", "Application/JSON")
81+
.send({ test: "casing" })
82+
expect(response.statusCode).toBe(200)
83+
expect(response.body.received.test).toBe("casing")
84+
})
85+
86+
it("accepts application/json on PUT", async () => {
87+
const response = await request(routeTester)
88+
.put("/json-endpoint")
89+
.set("Content-Type", "application/json")
90+
.send({ test: "put-data" })
91+
expect(response.statusCode).toBe(200)
92+
expect(response.body.received.test).toBe("put-data")
93+
})
94+
95+
it("accepts application/json on PATCH", async () => {
8496
const response = await request(routeTester)
85-
.post("/api/create")
97+
.patch("/json-endpoint")
98+
.set("Content-Type", "application/json")
99+
.send({ test: "patch-data" })
100+
expect(response.statusCode).toBe(200)
101+
expect(response.body.received.test).toBe("patch-data")
102+
})
103+
104+
it("returns 415 for missing Content-Type", async () => {
105+
const response = await request(routeTester)
106+
.post("/json-endpoint")
86107
.unset("Content-Type")
87108
.send(Buffer.from('{"test":"data"}'))
88109
expect(response.statusCode).toBe(415)
89110
expect(response.text).toContain("Missing or empty Content-Type header")
90111
})
91112

92-
it("returns 415 for text/plain on JSON-only endpoint", async () => {
113+
it("returns 415 for text/plain", async () => {
93114
const response = await request(routeTester)
94-
.post("/api/create")
115+
.post("/json-endpoint")
95116
.set("Content-Type", "text/plain")
96117
.send("some plain text")
97118
expect(response.statusCode).toBe(415)
98119
expect(response.text).toContain("Unsupported Content-Type")
99120
})
100121

122+
it("returns 415 for text/plain on PUT", async () => {
123+
const response = await request(routeTester)
124+
.put("/json-endpoint")
125+
.set("Content-Type", "text/plain")
126+
.send("some text")
127+
expect(response.statusCode).toBe(415)
128+
expect(response.text).toContain("Unsupported Content-Type")
129+
})
130+
101131
it("returns 415 for application/xml", async () => {
102132
const response = await request(routeTester)
103-
.post("/api/create")
133+
.post("/json-endpoint")
104134
.set("Content-Type", "application/xml")
105135
.send("<root/>")
106136
expect(response.statusCode).toBe(415)
107137
expect(response.text).toContain("Unsupported Content-Type")
108138
})
109139

110-
it("allows text/plain on search endpoint", async () => {
140+
it("returns 415 for comma-separated multiple Content-Type values", async () => {
111141
const response = await request(routeTester)
112-
.post("/api/search")
142+
.post("/json-endpoint")
143+
.set("Content-Type", "application/json, text/plain")
144+
.send('{"test":"data"}')
145+
expect(response.statusCode).toBe(415)
146+
expect(response.text).toContain("Multiple Content-Type values are not allowed")
147+
})
148+
149+
it("returns 415 for valid type smuggled via comma after charset", async () => {
150+
const response = await request(routeTester)
151+
.post("/json-endpoint")
152+
.set("Content-Type", "application/json; charset=utf-8, text/plain")
153+
.send('{"test":"data"}')
154+
expect(response.statusCode).toBe(415)
155+
expect(response.text).toContain("Multiple Content-Type values are not allowed")
156+
})
157+
})
158+
159+
describe("textContent middleware", () => {
160+
161+
it("accepts text/plain", async () => {
162+
const response = await request(routeTester)
163+
.post("/text-endpoint")
113164
.set("Content-Type", "text/plain")
114-
.send("search terms")
165+
.send("hello world")
115166
expect(response.statusCode).toBe(200)
116-
expect(response.body.received).toBe("search terms")
167+
expect(response.body.received).toBe("hello world")
117168
})
118169

119-
it("allows application/json on search endpoint", async () => {
170+
it("accepts text/plain with charset parameter", async () => {
120171
const response = await request(routeTester)
121-
.post("/api/search")
122-
.set("Content-Type", "application/json")
123-
.send({ searchText: "hello" })
172+
.post("/text-endpoint")
173+
.set("Content-Type", "text/plain; charset=utf-8")
174+
.send("hello charset")
124175
expect(response.statusCode).toBe(200)
125-
expect(response.body.received.searchText).toBe("hello")
126176
})
127177

128-
it("accepts Content-Type with unusual casing", async () => {
178+
it("returns 415 for missing Content-Type", async () => {
129179
const response = await request(routeTester)
130-
.post("/api/create")
131-
.set("Content-Type", "Application/JSON")
132-
.send({ test: "casing" })
133-
expect(response.statusCode).toBe(200)
134-
expect(response.body.received.test).toBe("casing")
180+
.post("/text-endpoint")
181+
.unset("Content-Type")
182+
.send(Buffer.from("hello"))
183+
expect(response.statusCode).toBe(415)
184+
expect(response.text).toContain("Missing or empty Content-Type header")
135185
})
136186

137-
it("skips validation for GET requests", async () => {
187+
it("returns 415 for application/json", async () => {
138188
const response = await request(routeTester)
139-
.get("/api/info")
140-
expect(response.statusCode).toBe(200)
141-
expect(response.body.info).toBe(true)
189+
.post("/text-endpoint")
190+
.set("Content-Type", "application/json")
191+
.send({ test: "data" })
192+
expect(response.statusCode).toBe(415)
193+
expect(response.text).toContain("Unsupported Content-Type")
194+
expect(response.text).toContain("text/plain")
142195
})
143196

144-
it("skips validation for DELETE requests", async () => {
197+
it("returns 415 for comma-separated multiple Content-Type values", async () => {
145198
const response = await request(routeTester)
146-
.delete("/api/delete/abc123")
147-
expect(response.statusCode).toBe(200)
148-
expect(response.body.deleted).toBe("abc123")
199+
.post("/text-endpoint")
200+
.set("Content-Type", "text/plain, application/json")
201+
.send("hello")
202+
expect(response.statusCode).toBe(415)
203+
expect(response.text).toContain("Multiple Content-Type values are not allowed")
149204
})
205+
})
206+
207+
describe("eitherContent middleware", () => {
150208

151-
it("skips validation for PATCH on release endpoint", async () => {
209+
it("accepts application/json", async () => {
152210
const response = await request(routeTester)
153-
.patch("/api/release/abc123")
211+
.post("/either-endpoint")
212+
.set("Content-Type", "application/json")
213+
.send({ searchText: "hello" })
154214
expect(response.statusCode).toBe(200)
155-
expect(response.body.released).toBe("abc123")
215+
expect(response.body.received.searchText).toBe("hello")
156216
})
157217

158-
it("accepts application/json on PUT endpoint", async () => {
218+
it("accepts application/ld+json", async () => {
159219
const response = await request(routeTester)
160-
.put("/api/update")
161-
.set("Content-Type", "application/json")
162-
.send({ test: "put-data" })
220+
.post("/either-endpoint")
221+
.set("Content-Type", "application/ld+json")
222+
// Must stringify manually; supertest's .send(object) would override Content-Type to application/json
223+
.send(JSON.stringify({ "@context": "http://example.org" }))
163224
expect(response.statusCode).toBe(200)
164-
expect(response.body.received.test).toBe("put-data")
225+
expect(response.body.received["@context"]).toBe("http://example.org")
165226
})
166227

167-
it("returns 415 for text/plain on PUT endpoint", async () => {
228+
it("accepts text/plain", async () => {
168229
const response = await request(routeTester)
169-
.put("/api/update")
230+
.post("/either-endpoint")
170231
.set("Content-Type", "text/plain")
171-
.send("some text")
232+
.send("search terms")
233+
expect(response.statusCode).toBe(200)
234+
expect(response.body.received).toBe("search terms")
235+
})
236+
237+
it("returns 415 for missing Content-Type", async () => {
238+
const response = await request(routeTester)
239+
.post("/either-endpoint")
240+
.unset("Content-Type")
241+
.send(Buffer.from("hello"))
172242
expect(response.statusCode).toBe(415)
173-
expect(response.text).toContain("Unsupported Content-Type")
243+
expect(response.text).toContain("Missing or empty Content-Type header")
174244
})
175245

176-
it("returns 415 for comma-separated multiple Content-Type values", async () => {
246+
it("returns 415 for application/xml", async () => {
177247
const response = await request(routeTester)
178-
.post("/api/create")
179-
.set("Content-Type", "application/json, text/plain")
180-
.send('{"test":"data"}')
248+
.post("/either-endpoint")
249+
.set("Content-Type", "application/xml")
250+
.send("<root/>")
181251
expect(response.statusCode).toBe(415)
182-
expect(response.text).toContain("Multiple Content-Type values are not allowed")
252+
expect(response.text).toContain("Unsupported Content-Type")
183253
})
184254

185-
it("returns 415 for valid type smuggled via comma after charset", async () => {
255+
it("returns 415 for comma-separated multiple Content-Type values", async () => {
186256
const response = await request(routeTester)
187-
.post("/api/create")
188-
.set("Content-Type", "application/json; charset=utf-8, text/plain")
257+
.post("/either-endpoint")
258+
.set("Content-Type", "application/json, text/plain")
189259
.send('{"test":"data"}')
190260
expect(response.statusCode).toBe(415)
191261
expect(response.text).toContain("Multiple Content-Type values are not allowed")
192262
})
193-
194263
})

routes/bulkCreate.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ const router = express.Router()
55
//This controller will handle all MongoDB interactions.
66
import controller from '../db-controller.js'
77
import auth from '../auth/index.js'
8-
import { jsonContent } from '../rest.js'
8+
import rest from '../rest.js'
99

1010
router.route('/')
11-
.post(auth.checkJwt, jsonContent, controller.bulkCreate)
11+
.post(auth.checkJwt, rest.jsonContent, controller.bulkCreate)
1212
.all((req, res, next) => {
1313
res.statusMessage = 'Improper request method for creating, please use POST.'
1414
res.status(405)

routes/bulkUpdate.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ const router = express.Router()
55
//This controller will handle all MongoDB interactions.
66
import controller from '../db-controller.js'
77
import auth from '../auth/index.js'
8-
import { jsonContent } from '../rest.js'
8+
import rest from '../rest.js'
99

1010
router.route('/')
11-
.put(auth.checkJwt, jsonContent, controller.bulkUpdate)
11+
.put(auth.checkJwt, rest.jsonContent, controller.bulkUpdate)
1212
.all((req, res, next) => {
1313
res.statusMessage = 'Improper request method for creating, please use PUT.'
1414
res.status(405)

routes/create.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ const router = express.Router()
44
//This controller will handle all MongoDB interactions.
55
import controller from '../db-controller.js'
66
import auth from '../auth/index.js'
7-
import { jsonContent } from '../rest.js'
7+
import rest from '../rest.js'
88

99
router.route('/')
10-
.post(auth.checkJwt, jsonContent, controller.create)
10+
.post(auth.checkJwt, rest.jsonContent, controller.create)
1111
.all((req, res, next) => {
1212
res.statusMessage = 'Improper request method for creating, please use POST.'
1313
res.status(405)

0 commit comments

Comments
 (0)