fix(res.sendFile): treat the app etag setting as a default, not an override

[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 undefined:

if (opts.etag === undefined) {
  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 stops clobbering the caller's object when they did supply one.

Test plan

• Added two 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, and an explicit { etag: true } against app.disable('etag').
• Ran: mocha test/ → 1154 passing, no failures. The two pre-existing ETag tests pass no etag option, so they are unaffected.
• Checked: reverting only lib/response.js fails both new tests.
• Ran: npm run lint → clean.

Note

With no etag option supplied, the setting is still written onto the caller's object, as before. Narrowing that further means copying opts, which felt like a separate change from the behaviour fix — happy to include it if you would rather.

[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)

[krzysdz]

I don't think that expanding this comment is necessary. I know that LLMs love comments, but not every line of code needs 3 lines of comments to explain what it does.

[krzysdz]

Suggested change

	if (opts.etag === undefined) {
	opts.etag = this.app.enabled('etag');
	}
	opts.etag ??= this.app.enabled('etag');

This would slightly change logic (overwrite undefined and null), but why not write it the easier way (boolean is expected anyway). Nullish coalescing assignment was designed for such use case.

[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.