Skip to content

Handle Bad Content-Type Headers RESTfully #255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thehabes merged 43 commits into from
Mar 25, 2026
Merged

Handle Bad Content-Type Headers RESTfully #255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
ce0edc3
First pass at handling bad Content-Type headers. Focus on a way to d…
thehabes Mar 23, 2026
5b91d8d
Changes while reviewing
thehabes Mar 23, 2026
2e014d7
Changes while reviewing
thehabes Mar 23, 2026
1361b2e
Changes while reviewing
thehabes Mar 23, 2026
1190481
Changes while reviewing
thehabes Mar 23, 2026
c02fe7f
Changes while reviewing
thehabes Mar 23, 2026
deeddc3
Changes while reviewing
thehabes Mar 23, 2026
eb08b27
Changes while reviewing
thehabes Mar 23, 2026
fe482d0
Changes while reviewing
thehabes Mar 23, 2026
1a33fe1
Changes while reviewing
thehabes Mar 23, 2026
0f5fa25
Changes while reviewing
thehabes Mar 23, 2026
d88723a
Changes while reviewing
thehabes Mar 23, 2026
bdbfdd5
Changes while reviewing
thehabes Mar 23, 2026
7dcc4ff
Changes while reviewing and testing
thehabes Mar 23, 2026
525b47b
Changes while reviewing and testing
thehabes Mar 23, 2026
1c90464
Fix createExpressError
thehabes Mar 24, 2026
2a7a67d
Fix createExpressError
thehabes Mar 24, 2026
cbb475b
Changes while testing and reviewing
thehabes Mar 24, 2026
ff149f9
Changes while testing and reviewing
thehabes Mar 24, 2026
5337622
Changes while reviewing and testing
thehabes Mar 24, 2026
bfa3a28
Changes while reviewing and testing
thehabes Mar 24, 2026
f077138
Changes while reviewing and testing
thehabes Mar 24, 2026
232307d
Changes while reviewing and testing
thehabes Mar 24, 2026
2b7ea90
reject content-type headers that have multiple or duplicate types
thehabes Mar 24, 2026
6a5dc5c
ah ok get those generic mongo error codes
thehabes Mar 24, 2026
cbef6a6
ah nebermind
thehabes Mar 24, 2026
a06037d
refactor how Content-Type headers are checked
thehabes Mar 24, 2026
99522c4
refactor how Content-Type headers are checked
thehabes Mar 24, 2026
e8dff8d
refactor how Content-Type headers are checked
thehabes Mar 24, 2026
d194bbc
refactor how createExpressError is used
thehabes Mar 24, 2026
f9d4f41
changes while testing and reviewing
thehabes Mar 24, 2026
555de09
It is about time we clean these old logs out. They are not useful be…
thehabes Mar 24, 2026
e1d8b90
Don't respond with the full token
thehabes Mar 24, 2026
1a9c5b2
rename
thehabes Mar 24, 2026
01d3359
Changes while testing and reviewing
thehabes Mar 24, 2026
8d106d0
I like the way it errors this way better
thehabes Mar 24, 2026
0e6101d
Update for tests
thehabes Mar 24, 2026
c283859
yikes bad not === check
thehabes Mar 24, 2026
f3cb69f
catch comma and semicolon smugglers
thehabes Mar 24, 2026
822b602
there is a valid version of the header with a comma. Make sure 415 d…
thehabes Mar 25, 2026
7d7046a
changes while testing
thehabes Mar 25, 2026
e298628
changes while testing
thehabes Mar 25, 2026
69a85f8
changes while testing
thehabes Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,8 @@ app.use(
})
)
app.use(logger('dev'))
app.use(express.json())
app.use(express.json({ type: ["application/json", "application/ld+json"] }))
app.use(express.text())
app.use(express.urlencoded({ extended: true }))
app.use(cookieParser())

//Publicly available scripts, CSS, and HTML pages.
Expand Down
22 changes: 11 additions & 11 deletions controllers/bulk.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { newID, isValidID, db } from '../database/index.js'
import utils from '../utils.js'
Copy link
Copy Markdown
Member

@cubap cubap Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to just call out the functions we need here?

import { _contextid, ObjectID, createExpressError, getAgentClaim, parseDocumentID, idNegotiation } from './utils.js'
import { _contextid, ObjectID, getAgentClaim, parseDocumentID, idNegotiation } from './utils.js'

/**
* Create many objects at once with the power of MongoDB bulkWrite() operations.
Expand All @@ -22,13 +22,13 @@ const bulkCreate = async function (req, res, next) {
if (!Array.isArray(documents)) {
err.message = "The request body must be an array of objects."
err.status = 400
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
if (documents.length === 0) {
err.message = "No action on an empty array."
err.status = 400
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
const gatekeep = documents.filter(d=> {
Expand All @@ -46,7 +46,7 @@ const bulkCreate = async function (req, res, next) {
if (gatekeep.length > 0) {
err.message = "All objects in the body of a `/bulkCreate` must be JSON and must not contain a declared identifier property."
err.status = 400
next(createExpressError(err))
next(utils.createExpressError(err))
return
}

Expand All @@ -55,7 +55,7 @@ const bulkCreate = async function (req, res, next) {
// if(slug){
// const slugError = await exports.generateSlugId(slug)
// if(slugError){
// next(createExpressError(slugError))
// next(utils.createExpressError(slugError))
// return
// }
// else{
Expand Down Expand Up @@ -92,7 +92,7 @@ const bulkCreate = async function (req, res, next) {
}
catch (error) {
//MongoServerError from the client has the following properties: index, code, keyPattern, keyValue
next(createExpressError(error))
next(utils.createExpressError(error))
}
}

Expand All @@ -111,13 +111,13 @@ const bulkUpdate = async function (req, res, next) {
if (!Array.isArray(documents)) {
err.message = "The request body must be an array of objects."
err.status = 400
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
if (documents.length === 0) {
err.message = "No action on an empty array."
err.status = 400
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
const gatekeep = documents.filter(d => {
Expand All @@ -136,7 +136,7 @@ const bulkUpdate = async function (req, res, next) {
if (gatekeep.length > 0) {
err.message = "All objects in the body of a `/bulkUpdate` must be JSON and must contain a declared identifier property."
err.status = 400
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
// unordered bulkWrite() operations have better performance metrics.
Expand All @@ -154,7 +154,7 @@ const bulkUpdate = async function (req, res, next) {
try {
originalObject = await db.findOne({"$or":[{"_id": id}, {"__rerum.slug": id}]})
} catch (error) {
next(createExpressError(error))
next(utils.createExpressError(error))
return
}
if (null === originalObject) continue
Expand Down Expand Up @@ -196,7 +196,7 @@ const bulkUpdate = async function (req, res, next) {
}
catch (error) {
//MongoServerError from the client has the following properties: index, code, keyPattern, keyValue
next(createExpressError(error))
next(utils.createExpressError(error))
}
}

Expand Down
15 changes: 7 additions & 8 deletions controllers/crud.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
import { newID, isValidID, db } from '../database/index.js'
import utils from '../utils.js'
import { _contextid, idNegotiation, generateSlugId, ObjectID, createExpressError, getAgentClaim, parseDocumentID } from './utils.js'
import { _contextid, idNegotiation, generateSlugId, ObjectID, getAgentClaim, parseDocumentID } from './utils.js'

/**
* Create a new Linked Open Data object in RERUM v1.
Expand All @@ -19,7 +19,7 @@ const create = async function (req, res, next) {
if(req.get("Slug")){
let slug_json = await generateSlugId(req.get("Slug"), next)
if(slug_json.code){
next(createExpressError(slug_json))
next(utils.createExpressError(slug_json))
return
}
else{
Expand All @@ -43,7 +43,6 @@ const create = async function (req, res, next) {
delete provided["@context"]

let newObject = Object.assign(context, { "@id": process.env.RERUM_ID_PREFIX + id }, provided, rerumProp, { "_id": id })
console.log("CREATE")
try {
let result = await db.insertOne(newObject)
res.set(utils.configureWebAnnoHeadersFor(newObject))
Expand All @@ -55,7 +54,7 @@ const create = async function (req, res, next) {
}
catch (error) {
//MongoServerError from the client has the following properties: index, code, keyPattern, keyValue
next(createExpressError(error))
next(utils.createExpressError(error))
}
}

Expand All @@ -75,7 +74,7 @@ const query = async function (req, res, next) {
message: "Detected empty JSON object. You must provide at least one property in the /query request body JSON.",
status: 400
}
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
try {
Expand All @@ -84,7 +83,7 @@ const query = async function (req, res, next) {
res.set(utils.configureLDHeadersFor(matches))
res.json(matches)
} catch (error) {
next(createExpressError(error))
next(utils.createExpressError(error))
}
}

Expand Down Expand Up @@ -116,9 +115,9 @@ const id = async function (req, res, next) {
"message": `No RERUM object with id '${id}'`,
"status": 404
}
next(createExpressError(err))
next(utils.createExpressError(err))
} catch (error) {
next(createExpressError(error))
next(utils.createExpressError(error))
}
}

Expand Down
16 changes: 8 additions & 8 deletions controllers/delete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
import { newID, isValidID, db } from '../database/index.js'
import utils from '../utils.js'
import { createExpressError, getAgentClaim, parseDocumentID, getAllVersions, getAllDescendants } from './utils.js'
import { getAgentClaim, parseDocumentID, getAllVersions, getAllDescendants } from './utils.js'

/**
* Mark an object as deleted in the database.
Expand All @@ -26,15 +26,15 @@ const deleteObj = async function(req, res, next) {
try {
id = req.params["_id"] ?? parseDocumentID(JSON.parse(JSON.stringify(req.body))["@id"]) ?? parseDocumentID(JSON.parse(JSON.stringify(req.body))["id"])
} catch(error){
next(createExpressError(error))
next(utils.createExpressError(error))
return
Comment on lines 26 to 30
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In deleteObj, a missing/unparsed body (e.g., no Content-Type) causes JSON.parse(JSON.stringify(req.body)) to throw, which is then wrapped into a generic 500 via createExpressError(error). Since this is a client request-shape problem (no :_id param and no usable JSON body), it should be normalized to a 400/415 with a clear message (e.g., require /delete/:_id or a JSON body with @id and a JSON Content-Type) so bad/missing Content-Type doesn’t surface as a 500 on this endpoint.

Copilot uses AI. Check for mistakes.
}
let agentRequestingDelete = getAgentClaim(req, next)
let originalObject
try {
originalObject = await db.findOne({"$or":[{"_id": id}, {"__rerum.slug": id}]})
} catch (error) {
next(createExpressError(error))
next(utils.createExpressError(error))
return
}
if (null !== originalObject) {
Expand All @@ -58,7 +58,7 @@ const deleteObj = async function(req, res, next) {
})
}
if (err.status) {
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
let preserveID = safe_original["@id"]
Expand All @@ -76,14 +76,14 @@ const deleteObj = async function(req, res, next) {
try {
result = await db.replaceOne({ "_id": originalObject["_id"] }, deletedObject)
} catch (error) {
next(createExpressError(error))
next(utils.createExpressError(error))
return
}
if (result.modifiedCount === 0) {
//result didn't error out, the action was not performed. Sometimes, this is a neutral thing. Sometimes it is indicative of an error.
err.message = "The original object was not replaced with the deleted object in the database."
err.status = 500
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
//204 to say it is deleted and there is nothing in the body
Expand All @@ -94,12 +94,12 @@ const deleteObj = async function(req, res, next) {
//Not sure we can get here, as healHistoryTree might throw and error.
err.message = "The history tree for the object being deleted could not be mended."
err.status = 500
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
err.message = "No object with this id could be found in RERUM. Cannot delete."
err.status = 404
next(createExpressError(err))
next(utils.createExpressError(err))
}

/**
Expand Down
12 changes: 6 additions & 6 deletions controllers/gog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { newID, isValidID, db } from '../database/index.js'
import utils from '../utils.js'
import { _contextid, ObjectID, createExpressError, getAgentClaim, parseDocumentID, idNegotiation } from './utils.js'
import { _contextid, ObjectID, getAgentClaim, parseDocumentID, idNegotiation } from './utils.js'

/**
* THIS IS SPECIFICALLY FOR 'Gallery of Glosses'
Expand Down Expand Up @@ -44,7 +44,7 @@ const _gog_fragments_from_manuscript = async function (req, res, next) {
})
}
if (err.status) {
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
try {
Expand Down Expand Up @@ -138,7 +138,7 @@ const _gog_fragments_from_manuscript = async function (req, res, next) {
}
catch (error) {
console.error(error)
next(createExpressError(error))
next(utils.createExpressError(error))
}
}

Expand All @@ -162,7 +162,7 @@ const _gog_glosses_from_manuscript = async function (req, res, next) {
const skip = parseInt(req.query.skip ?? 0)
let err = { message: `` }
// This request can only be made my Gallery of Glosses production apps.
if (!agentID === "61043ad4ffce846a83e700dd") {
if (agentID !== "61043ad4ffce846a83e700dd") {
err = Object.assign(err, {
message: `Only the Gallery of Glosses can make this request.`,
status: 403
Expand All @@ -176,7 +176,7 @@ const _gog_glosses_from_manuscript = async function (req, res, next) {
})
}
if (err.status) {
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
try {
Expand Down Expand Up @@ -300,7 +300,7 @@ const _gog_glosses_from_manuscript = async function (req, res, next) {
}
catch (error) {
console.error(error)
next(createExpressError(error))
next(utils.createExpressError(error))
}
}

Expand Down
26 changes: 13 additions & 13 deletions controllers/history.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { newID, isValidID, db } from '../database/index.js'
import utils from '../utils.js'
import { _contextid, ObjectID, createExpressError, getAgentClaim, parseDocumentID, idNegotiation, getAllVersions, getAllAncestors, getAllDescendants } from './utils.js'
import { _contextid, ObjectID, getAgentClaim, parseDocumentID, idNegotiation, getAllVersions, getAllAncestors, getAllDescendants } from './utils.js'

/**
* Public facing servlet to gather for all versions downstream from a provided `key object`.
Expand All @@ -23,15 +23,15 @@ const since = async function (req, res, next) {
try {
obj = await db.findOne({"$or":[{"_id": id}, {"__rerum.slug": id}]})
} catch (error) {
next(createExpressError(error))
next(utils.createExpressError(error))
return
}
if (null === obj) {
let err = {
message: `Cannot produce a history. There is no object in the database with id '${id}'. Check the URL.`,
status: 404
}
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
let all = await getAllVersions(obj)
Expand Down Expand Up @@ -60,15 +60,15 @@ const history = async function (req, res, next) {
try {
obj = await db.findOne({"$or":[{"_id": id}, {"__rerum.slug": id}]})
} catch (error) {
next(createExpressError(error))
next(utils.createExpressError(error))
return
}
if (null === obj) {
let err = {
message: `Cannot produce a history. There is no object in the database with id '${id}'. Check the URL.`,
status: 404
}
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
let all = await getAllVersions(obj)
Expand Down Expand Up @@ -103,9 +103,9 @@ const idHeadRequest = async function (req, res, next) {
"message": `No RERUM object with id '${id}'`,
"status": 404
}
next(createExpressError(err))
next(utils.createExpressError(err))
} catch (error) {
next(createExpressError(error))
next(utils.createExpressError(error))
}
}

Expand All @@ -128,9 +128,9 @@ const queryHeadRequest = async function (req, res, next) {
"message": `There are no objects in the database matching the query. Check the request body.`,
"status": 404
}
next(createExpressError(err))
next(utils.createExpressError(err))
} catch (error) {
next(createExpressError(error))
next(utils.createExpressError(error))
}
}

Expand All @@ -145,15 +145,15 @@ const sinceHeadRequest = async function (req, res, next) {
try {
obj = await db.findOne({"$or":[{"_id": id}, {"__rerum.slug": id}]})
} catch (error) {
next(createExpressError(error))
next(utils.createExpressError(error))
return
}
if (null === obj) {
let err = {
message: `Cannot produce a history. There is no object in the database with id '${id}'. Check the URL.`,
status: 404
}
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
let all = await getAllVersions(obj)
Expand Down Expand Up @@ -183,15 +183,15 @@ const historyHeadRequest = async function (req, res, next) {
try {
obj = await db.findOne({"$or":[{"_id": id}, {"__rerum.slug": id}]})
} catch (error) {
next(createExpressError(error))
next(utils.createExpressError(error))
return
}
if (null === obj) {
let err = {
message: "Cannot produce a history. There is no object in the database with this id. Check the URL.",
status: 404
}
next(createExpressError(err))
next(utils.createExpressError(err))
return
}
let all = await getAllVersions(obj)
Expand Down
Loading
Loading