feat(app): warn when HEAD route is declared after GET route on the same path

[santusht06]

Problem

In Express, declaring an app.get(path, ...) route automatically equips that route layer with an implicit HEAD fallback.

If a developer subsequently declares a custom app.head(path, ...) route on the exact same path:

app.get('/api/resource', handleGet);   // 1. GET declared first
app.head('/api/resource', handleHead); // 2. Custom HEAD declared second

When a client sends a HEAD request, the router evaluates layers in order of declaration. Because the preceding GET layer matches all HEAD requests as a fallback, the handleGet handler runs and the custom handleHead route is silently shadowed and never executed (Issue #5004).

Solution

Inside app[method] in lib/application.js, when method === 'head', check if the router stack already contains an existing route for the same path with methods.get = true.

If detected, emit an ExpressWarning using process.emitWarning alerting the developer to declare HEAD routes before GET routes to ensure they execute properly.

Changes Made

• lib/application.js: Added check and warning emission when app.head(path) is registered after an existing app.get(path).
• test/app.head.js:
  • Added test verifying warning emission when HEAD is declared after GET on the same path.
  • Added test verifying no warning is emitted when HEAD is declared before GET on the same path.
  • Added test verifying no warning is emitted when HEAD and GET are declared on different paths.

Test Plan

• Run npm test (all 1,259 tests pass cleanly).
• Run npm run lint (passes with 0 errors).

Fixes #5004

[kilisamemarisaaa]

Could this also cover Router registrations? The runnable reproduction in #5004 uses express.Router(), but this check only runs in app[method], so Router#head never reaches it.

I reproduced this at 5e94a42b5238489cbdb5d1ee19d9ba655f6b128c with Express 5.2.1. The direct application sequence emits one warning, while the equivalent router sequence emits none and a HEAD request is still handled by the GET handler:

directAppWarnings: 1
routerWarnings: 0
routerHeadHandler: GET

The added app tests pass (6/6 for test/app.head.js) and lint is clean, but they do not exercise the linked issue's registration path. Since this PR does not change the router implementation, #5004's reproduction remains unchanged. Please add a regression test using express.Router() and cover that path (which may require a corresponding change in the router dependency), or narrow the PR scope rather than closing #5004.

[santusht06]

Thanks for the review and great catch, @kilisamemarisaaa!

I have updated the PR in commit 4358fd4 to also wrap Router.prototype.head in lib/express.js so that both direct app.head() and express.Router().head() declarations emit the diagnostic warning when declared after an existing GET route on the same path.

I also added regression tests in test/Router.js covering:

• Warning emission when Router.head() is declared after Router.get() on the same path.
• Verification that no warning is emitted when Router.head() is declared before Router.get() (the recommended order).

All 1,261 tests pass cleanly across the full suite.

[kilisamemarisaaa]

Thanks for addressing the Router path. I re-ran the original reproduction on 4358fd4f: it now emits one warning, and the full suite is 1,265/1,265 with lint passing.

I found two remaining cases before this fully covers the behavior:

1. The wrapper in lib/express.js mutates the shared router package prototype for the entire process. This minimal sequence shows the side effect:

const Router = require('router')
const originalHead = Router.prototype.head
require('express')

console.log(Router.prototype.head !== originalHead) // true

A standalone Router() created from that first import then emits an ExpressWarning when registering GET followed by HEAD, even though it is not an Express router. Could this be implemented in the router dependency itself (and consumed through a version update), or scoped to routers created through Express without patching the dependency's global prototype?

2. The strict layer.route.path === path comparison misses supported array paths:

router.get(['/a', '/b'], getHandler)
router.head('/a', headHandler)

I sent HEAD /a on this head; getHandler handled it and zero warnings were emitted. An array-path regression test would keep the warning aligned with the actual shadowing behavior.

[santusht06]

Thanks again for the thorough review, @kilisamemarisaaa!

I have addressed both items in commit eb49bfa:

1. Prototype Isolation: Replaced global Router.prototype.head mutation with an expressRouter factory in lib/express.js. Standalone require('router') instances and prototype remain completely untouched and unmutated.
2. Array Path Support: Added pathsOverlap utility in lib/utils.js to correctly identify shadowing when routes are declared using array paths (e.g., router.get(['/a', '/b']) followed by router.head('/a')).
3. Added Regression Tests:
   • Tested that standalone require('router') prototype is not mutated and emits 0 warnings.
   • Tested that Router.get(['/a', '/b']) + Router.head('/a') emits ExpressWarning.
   • Tested that app.get(['/a', '/b']) + app.head('/a') emits ExpressWarning.

All 1,264 tests across the entire suite pass cleanly with 0 lint errors.