Skip to content

Commit ce0edc3

Browse files
committed
First pass at handling bad Content-Type headers. Focus on a way to do it that doesn't have to touch man files in the api/ directory.
1 parent e2cf86c commit ce0edc3

4 files changed

Lines changed: 167 additions & 2 deletions

File tree

app.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ app.use(
5757
})
5858
)
5959
app.use(logger('dev'))
60-
app.use(express.json())
60+
app.use(express.json({ type: ["application/json", "application/ld+json"] }))
6161
app.use(express.text())
6262
app.use(express.urlencoded({ extended: true }))
6363
app.use(cookieParser())

rest.js

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,52 @@ const checkPatchOverrideSupport = function (req, res) {
2424
return undefined !== override && override === "PATCH"
2525
}
2626

27+
/**
28+
* Middleware to validate Content-Type headers on API write endpoints.
29+
* Ensures that requests carrying bodies have an appropriate Content-Type before
30+
* reaching controllers, preventing unhandled errors from unparsed or mis-parsed bodies.
31+
*
32+
* - Skips validation for methods that don't carry bodies (GET, HEAD, OPTIONS, DELETE)
33+
* - Allows text/plain for /search endpoints (which accept plain text search terms)
34+
* - Requires application/json or application/ld+json for all other write endpoints
35+
* - Returns 400 for missing Content-Type, 415 for unsupported Content-Type
36+
*
37+
* @param {Object} req - Express request object
38+
* @param {Object} res - Express response object
39+
* @param {Function} next - Express next middleware function
40+
*/
41+
const validateContentType = function (req, res, next) {
42+
const skipMethods = ["GET", "HEAD", "OPTIONS", "DELETE"]
43+
if (skipMethods.includes(req.method)) {
44+
return next()
45+
}
46+
47+
const contentType = req.get("Content-Type") ?? ""
48+
49+
if (!contentType) {
50+
res.statusMessage = `Missing Content-Type header. Requests to this endpoint require a Content-Type of "application/json" or "application/ld+json".`
51+
res.status(415)
52+
return next(res)
53+
}
54+
55+
const isJson = contentType.includes("application/json") || contentType.includes("application/ld+json")
56+
const isText = contentType.includes("text/plain")
57+
const isSearchEndpoint = req.path.startsWith("/search")
58+
59+
if (isJson) {
60+
return next()
61+
}
62+
63+
if (isText && isSearchEndpoint) {
64+
return next()
65+
}
66+
67+
const acceptedTypes = `"application/json" or "application/ld+json"${isSearchEndpoint ? ' or "text/plain"' : ''}`
68+
res.statusMessage = `Unsupported Content-Type: "${contentType}". This endpoint requires ${acceptedTypes}.`
69+
res.status(415)
70+
next(res)
71+
}
72+
2773
/**
2874
* Throughout the routes are certain warning, error, and hard fail scenarios.
2975
* REST is all about communication. The response code and the textual body are particular.
@@ -95,6 +141,9 @@ The requested web page or resource could not be found.`
95141
case 409:
96142
// These are all handled in db-controller.js already.
97143
break
144+
case 415:
145+
// Unsupported Media Type. The Content-Type header is not acceptable for this endpoint.
146+
break
98147
case 501:
99148
// Not implemented. Handled upstream.
100149
break
@@ -113,4 +162,4 @@ It may not have completed at all, and most likely did not complete successfully.
113162
res.status(error.status).send(error.message)
114163
}
115164

116-
export default { checkPatchOverrideSupport, messenger }
165+
export default { checkPatchOverrideSupport, validateContentType, messenger }
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import { jest } from "@jest/globals"
2+
import express from "express"
3+
import request from "supertest"
4+
import rest from '../../rest.js'
5+
6+
// Set up a minimal Express app with the Content-Type validation middleware
7+
const routeTester = new express()
8+
routeTester.use(express.json({ type: ["application/json", "application/ld+json"] }))
9+
routeTester.use(express.text())
10+
routeTester.use(express.urlencoded({ extended: false }))
11+
12+
// Mount the validateContentType middleware on /api just like api-routes.js
13+
routeTester.use("/api", rest.validateContentType)
14+
15+
// Simple JSON-only endpoint (like /api/create, /api/query, etc.)
16+
routeTester.post("/api/create", (req, res) => {
17+
res.status(200).json({ received: req.body })
18+
})
19+
20+
// Search endpoint that accepts text/plain
21+
routeTester.post("/api/search", (req, res) => {
22+
res.status(200).json({ received: req.body })
23+
})
24+
25+
// GET endpoint should pass through without Content-Type validation
26+
routeTester.get("/api/info", (req, res) => {
27+
res.status(200).json({ info: true })
28+
})
29+
30+
// Error handler matching the app's pattern
31+
routeTester.use(rest.messenger)
32+
33+
describe("Content-Type validation middleware", () => {
34+
35+
it("accepts application/json with valid JSON body", async () => {
36+
const response = await request(routeTester)
37+
.post("/api/create")
38+
.set("Content-Type", "application/json")
39+
.send({ test: "data" })
40+
expect(response.statusCode).toBe(200)
41+
expect(response.body.received.test).toBe("data")
42+
})
43+
44+
it("accepts application/ld+json with valid JSON body", async () => {
45+
const response = await request(routeTester)
46+
.post("/api/create")
47+
.set("Content-Type", "application/ld+json")
48+
.send(JSON.stringify({ "@context": "http://example.org", test: "ld" }))
49+
expect(response.statusCode).toBe(200)
50+
expect(response.body.received["@context"]).toBe("http://example.org")
51+
})
52+
53+
it("accepts application/json with charset parameter", async () => {
54+
const response = await request(routeTester)
55+
.post("/api/create")
56+
.set("Content-Type", "application/json; charset=utf-8")
57+
.send({ test: "charset" })
58+
expect(response.statusCode).toBe(200)
59+
})
60+
61+
it("returns 415 for missing Content-Type header", async () => {
62+
const response = await request(routeTester)
63+
.post("/api/create")
64+
.unset("Content-Type")
65+
.send(Buffer.from('{"test":"data"}'))
66+
expect(response.statusCode).toBe(415)
67+
expect(response.text).toContain("Missing Content-Type header")
68+
})
69+
70+
it("returns 415 for text/plain on JSON-only endpoint", async () => {
71+
const response = await request(routeTester)
72+
.post("/api/create")
73+
.set("Content-Type", "text/plain")
74+
.send("some plain text")
75+
expect(response.statusCode).toBe(415)
76+
expect(response.text).toContain("Unsupported Content-Type")
77+
})
78+
79+
it("returns 415 for application/xml", async () => {
80+
const response = await request(routeTester)
81+
.post("/api/create")
82+
.set("Content-Type", "application/xml")
83+
.send("<root/>")
84+
expect(response.statusCode).toBe(415)
85+
expect(response.text).toContain("Unsupported Content-Type")
86+
})
87+
88+
it("allows text/plain on search endpoint", async () => {
89+
const response = await request(routeTester)
90+
.post("/api/search")
91+
.set("Content-Type", "text/plain")
92+
.send("search terms")
93+
expect(response.statusCode).toBe(200)
94+
expect(response.body.received).toBe("search terms")
95+
})
96+
97+
it("allows application/json on search endpoint", async () => {
98+
const response = await request(routeTester)
99+
.post("/api/search")
100+
.set("Content-Type", "application/json")
101+
.send({ searchText: "hello" })
102+
expect(response.statusCode).toBe(200)
103+
expect(response.body.received.searchText).toBe("hello")
104+
})
105+
106+
it("skips validation for GET requests", async () => {
107+
const response = await request(routeTester)
108+
.get("/api/info")
109+
expect(response.statusCode).toBe(200)
110+
expect(response.body.info).toBe(true)
111+
})
112+
113+
})

routes/api-routes.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,13 @@ import releaseRouter from './release.js';
4444
import sinceRouter from './since.js';
4545
// Support GET requests like v1/history/{object id} to discover all previous versions tracing back to the prime.
4646
import historyRouter from './history.js';
47+
import rest from '../rest.js'
4748

4849
router.use(staticRouter)
4950
router.use('/id',idRouter)
5051
router.use('/api', compatabilityRouter)
52+
// Validate Content-Type headers on all /api write endpoints (fixes #245, #246, #248)
53+
router.use('/api', rest.validateContentType)
5154
router.use('/api/query', queryRouter)
5255
router.use('/api/search', searchRouter)
5356
router.use('/api/create', createRouter)

0 commit comments

Comments
 (0)