Handle Bad Content-Type Headers RESTfully

[thehabes]

Closes #245
Closes #246
Closes #248
Closes #256

Summary

Requests with wrong or missing Content-Type headers were causing 500 errors and connection resets instead of proper HTTP error responses. This was because express.json() left req.body unparsed, and controllers crashed accessing undefined properties — dropping the TCP connection before any response was sent.

Changes

• app.js — Restricted express.json() to only parse application/json and application/ld+json bodies. This also fixes a latent issue where application/ld+json bodies (core to a JSON-LD API) were never being parsed by the JSON middleware. Removed unnecessary express.urlencoded() middleware (this is a JSON API, not a form-processing app).
• controllers/utils.js — Fixed createExpressError() where the switch (err.code) block set statusCode/statusMessage correctly, but unconditional fallback lines immediately overwrote both values (createExpressError overwrites err.code switch values unconditionally #256). Restructured to set defaults in the initializer, then override for err.code === 11000.
• rest.js — Added validateContentType middleware that runs before API controllers:
  • Skips validation for methods without bodies (GET, HEAD, OPTIONS, DELETE)
  • Skips validation for /release endpoints (uses Slug header, not a JSON body)
  • Allows text/plain for /search endpoints
  • Requires application/json or application/ld+json for all other write endpoints (POST, PUT, PATCH)
  • Returns 415 Unsupported Media Type for missing or unsupported Content-Type
  • Added 415 case to the messenger error handler
  • Fixed pre-existing bug in messenger's 403 handler where err.message was used instead of error.message, causing the "Forbidden" text to be silently dropped from the response
• routes/api-routes.js — Mounted the middleware on /api after compatibility rewrites but before API route handlers
• routes/static.js — Removed unnecessary express.urlencoded() middleware
• routes/__tests__/contentType.test.js — 15 tests covering valid types, rejected types, case insensitivity, method skipping (GET, DELETE), PUT validation, and endpoint-specific rules (search, release)
• routes/__tests__/history.test.js, since.test.js — Increased Jest timeout to 10s to fix intermittent test timeouts
• routes/__tests__/*.test.js — Removed express.urlencoded() from all test app setups to match production configuration

[Copilot]

Pull request overview

Improves REST correctness for requests with missing/incorrect Content-Type headers to avoid controller crashes/connection resets, and aligns error handling with appropriate HTTP responses.

Changes:

• Restricts JSON body parsing to application/json and application/ld+json, and introduces /api Content-Type validation middleware returning 415 for unsupported/missing types.
• Fixes createExpressError() so the 11000 duplicate-key mapping to 409 isn’t overwritten by unconditional fallbacks.
• Updates/extends Jest coverage around Content-Type handling and adjusts timeouts/removes unused urlencoded middleware from several route tests.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
routes/static.js	Removes unused urlencoded parsing from static router.
routes/api-routes.js	Mounts validateContentType middleware on /api before API handlers.
routes/tests/update.test.js	Removes unused urlencoded middleware in test harness.
routes/tests/unset.test.js	Removes unused urlencoded middleware in test harness.
routes/tests/since.test.js	Increases Jest timeout; removes unused urlencoded middleware in test harness.
routes/tests/set.test.js	Removes unused urlencoded middleware in test harness.
routes/tests/release.test.js	Removes unused urlencoded middleware in test harness.
routes/tests/query.test.js	Removes unused urlencoded middleware in test harness.
routes/tests/patch.test.js	Removes unused urlencoded middleware in test harness.
routes/tests/overwrite-optimistic-locking.test.txt	Removes unused urlencoded middleware in test harness snippet.
routes/tests/id.test.js	Removes unused urlencoded middleware in test harness.
routes/tests/history.test.js	Increases Jest timeout; removes unused urlencoded middleware in test harness.
routes/tests/delete.test.js	Removes unused urlencoded middleware in test harness.
routes/tests/create.test.js	Removes unused urlencoded middleware in test harness.
routes/tests/contentType.test.js	Adds focused tests for Content-Type validation behavior and endpoint-specific rules.
routes/tests/bulkUpdate.test.js	Removes unused urlencoded middleware in test harness.
routes/tests/bulkCreate.test.js	Removes unused urlencoded middleware in test harness.
rest.js	Adds validateContentType; fixes 403 message handling; adds 415 case to messenger.
controllers/utils.js	Refactors createExpressError() defaulting logic and preserves 11000 -> 409 mapping.
app.js	Restricts JSON parsing to JSON + JSON-LD; removes urlencoded middleware.

[Copilot]

rest.js now imports createExpressError from controllers/utils.js. That module imports database/index.js, which eagerly calls connect() on import, so simply importing rest.js (e.g. in routes/__tests__/contentType.test.js) can trigger an unwanted MongoDB connection attempt. Since messenger already understands plain {statusCode, statusMessage} objects, consider removing this import and calling next({ statusCode: 415, statusMessage: ... }) (or moving createExpressError to a side-effect-free module) to keep REST middleware usable without DB side effects.

[Copilot]

createExpressError() no longer provides a generic message for MongoDB errors with err.code (other than 11000). Because controllers often pass raw MongoServerError objects (which include code and message) into createExpressError, this change can leak internal DB error details to clients via rest.messenger. Consider restoring the prior behavior for non-11000 err.code values (e.g., set a generic 500 message when err.code is present and no explicit statusMessage was provided), while keeping the 11000 -> 409 mapping.

Suggested change

	let error = {
	statusCode: err.statusCode ?? err.status ?? 500,
	statusMessage: err.statusMessage ?? err.message ?? "There was an error that prevented this request from completing successfully."
	}
	if (err.code === 11000) {
	error.statusMessage = `The id provided already exists. Please use a different _id or Slug.`
	error.statusCode = 409
	const explicitStatusMessage = err.statusMessage
	let error = {
	statusCode: err.statusCode ?? err.status ?? 500,
	statusMessage: explicitStatusMessage ?? err.message ?? "There was an error that prevented this request from completing successfully."
	}
	if (err.code === 11000) {
	error.statusMessage = `The id provided already exists. Please use a different _id or Slug.`
	error.statusCode = 409
	} else if (err.code != null && explicitStatusMessage == null) {
	error.statusCode = 500
	error.statusMessage = "There was an error that prevented this request from completing successfully."