[NgoQuocViet2001]

Problem

#6073 wired the application etag setting into res.sendFile to close #2294. The assignment is unconditional:

// wire application etag option to send
opts.etag = this.app.enabled('etag');
var file = send(req, pathname, opts);

opts is the caller's own options object (var opts = options || {}), so this both overwrites an etag the caller passed and writes the setting onto their object.

send documents etag as an option (opts.etag !== undefined ? Boolean(opts.etag) : true), and res.sendFile's own JSDoc says "Other options are passed along to send", which every other option does. This one could not get through.

Measured over HTTP against master (5.2.1):

app etag ENABLED (default)
  res.sendFile(file, { etag: false })   ETag present   (expected absent)   <-- WRONG
  res.sendFile(file, {})                ETag present   (expected present)

app etag DISABLED
  res.sendFile(file, { etag: true })    ETag absent    (expected present)  <-- WRONG
  res.sendFile(file, {})                ETag absent    (expected absent)

res.download routes through res.sendFile, so it behaves the same way.

Fix

Assign only when the caller left it unset, and on a copy of their options:

opts = Object.create(opts);
// wire application etag option to send
opts.etag ??= this.app.enabled('etag');

This keeps #2294's behaviour intact: with no etag option, app.disable('etag') still turns it off, which is what that issue asked for ("app.disable('etag') sets sendFile()s options.etag to false for you"). It restores the per-call option that #6073's title advertises, and it no longer writes anything onto the caller's object.

Test plan

• Added three cases to test/res.sendFile.js beside the existing should disable the ETag function if requested: an explicit { etag: false } against the default-on app, an explicit { etag: true } against app.disable('etag'), and a check that the options object passed in comes back unchanged.
• Ran: npm test → 1263 passing, no failures. The two pre-existing ETag tests pass no etag option, so they are unaffected.
• Checked: reverting only lib/response.js fails the new tests.
• Ran: npm run lint → clean.

Note

The copy uses Object.create(opts) rather than { ...opts }. res.download passes its options to res.sendFile the same way (Object.create(opts) plus its own headers), so a spread would drop the root, dotfiles, maxAge and etag given to res.download. With a spread, 6 existing tests fail (4 in test/res.download.js, 2 in the downloads acceptance test).

[krzysdz]

Unconditionally overwriting the etag option probably isn't the best idea. There is code which uses etag: false with res.sendFile() that would stop working properly in Express 5.

This option probably should also be documented on the website. Currently it lists all options from Express's res.sendFile() and some, but not all that are used by send.

cc @wesleytodd @UlisesGascon (reviewers of #6073)

[NgoQuocViet2001]

Thanks for digging those up. They're all the etag: false on res.sendFile() case this fixes, and Express 4 respected it before send moved the option into Express.

Agreed on the docs too. The res.sendFile options table doesn't list etag at all right now. If this gets merged I can send a PR to expressjs.com adding it, with a note that it defaults to the app's etag setting.

[krzysdz]

Oops, this is still slightly wrong. If opts.etag is undefined/null (or does not exist) on the passed options object, this assignment will mutate the options argument and this modification will be visible outside this function.

Before setting the etag option a copy of opts should be made. For example (I can't make a suggestion on unmodified lines):

  opts = { ...opts }
  // wire application etag option to send
  opts.etag ??= this.app.enabled('etag');

This will set etag only on a copy of the argument and only if it does not exist or is null/undefined.

[colinandres885-boop]

• sendFile.[[2026-q1-quarterly-net-interest-margin.csv]()]()

[NgoQuocViet2001]

Thanks, fixed in 80318c4: the default is now set on a copy, and a new test checks that the options object passed to res.sendFile comes back unchanged.

I used opts = Object.create(opts) rather than { ...opts }, though. res.download passes its options to res.sendFile as Object.create(opts) with its own headers, so a spread keeps only headers and drops the root, dotfiles, maxAge and etag given to res.download. With a spread, 6 existing tests fail (4 in test/res.download.js, 2 in the downloads acceptance test), and res.download(file, { etag: false }) sends an ETag again.