Skip to content

Reject reserved names in morgan.token() - #385

Open
seethinajayadileep wants to merge 1 commit into
expressjs:masterfrom
seethinajayadileep:fix/reject-reserved-token-names
Open

seethinajayadileep wants to merge 1 commit into
expressjs:masterfrom
seethinajayadileep:fix/reject-reserved-token-names

Conversation

@seethinajayadileep

Copy link
Copy Markdown

Summary

morgan.token() stores callbacks on the morgan export object. Registering a token named token, format, or compile therefore overwrote morgan's own API methods. After morgan.token('token', ...), later morgan.token(...) calls failed with TypeError: ... is not a function, which matches issue #265.

Per maintainer guidance on that issue, 1.x should reject those reserved names with a clear error (and document them). Allowing arbitrary names is left for a future 2.0 redesign.

Changes

  • Throw TypeError when registering a reserved token name
  • Document reserved names in the README
  • Add regression tests (reject reserved names; keep morgan.token usable after a rejected registration)

Test plan

  • ./node_modules/.bin/mocha --exit test/morgan.js (96 pass)
  • NO_COLOR=1 ./node_modules/.bin/mocha --exit test/noColor.js (6 pass)

Fixes #265

Registering a token named token/format/compile overwrote morgan's own
API methods because tokens are stored on the export object. Throw a
TypeError instead, document the reserved names, and add regression tests.

Fixes expressjs#265

Signed-off-by: JAYA DILEEP <seethinajayadileep@hotmail.com>
@itzzadi04

Copy link
Copy Markdown

Tested this locally against the reproduction in #265.

On master, registering a token named token is accepted, and later morgan.token() calls silently do nothing (morgan.uuid ends up undefined).

On this branch, morgan.token('token', ...) throws morgan.token cannot use reserved name "token". compile and format are rejected the same way.

I also listed the functions exposed on morgan. compile, format and token are the ones morgan itself relies on, so the reserved list looks complete. Built-in tokens and formats stay overridable, which seems intentional.

The README note is accurate and sits well next to the existing "same name overwrites" sentence.

npm test and npm run lint both pass on this branch. Looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding a token named token causes all subsequent tokens to break.

3 participants