Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change default "user-agent" from undici #2306

Closed
balazsorban44 opened this issue Oct 2, 2023 · 12 comments · Fixed by #2310
Closed

Change default "user-agent" from undici #2306

balazsorban44 opened this issue Oct 2, 2023 · 12 comments · Fixed by #2310

Comments

@balazsorban44
Copy link
Contributor

balazsorban44 commented Oct 2, 2023

What is the problem this feature will solve?

Currently, Node.js uses undici for its fetch implementation. This sets undici as the default user agent.

From Node.js' perspective, undici is an implementation detail and so it should have its own user agent header.

For context at Next.js, we stopped polyfilling globalThis.fetch in favor of the platform-provided alternative (previously using node-fetch), and started getting reports where users did not know where the requests with the undici user agent were coming from.

I've tested the current behavior in different runtimes, here is Node.js, Bun and Deno for comparison:

  • Node.js node node.mjs prints: undici:
// node.mjs
import { createServer } from 'http'
const server = createServer((req, res) => {
  console.log(req.headers['user-agent'])
  res.end()
}).listen(3000)
await fetch('http://localhost:3000')
server.close()
process.exit(0)
  • Bun bun bun.js prints: Bun/1.0.2:
// bun.js
const server = Bun.serve({
  port: 3000,
  fetch: (req) => console.log(req.headers.get('user-agent')),
})
await fetch('http://localhost:3000')
server.stop()
  • Deno deno run --allow-net deno.js prints: Deno/1.37.1:
// deno.js
Deno.serve({ port: 3000 }, (request) => {
  console.log(request.headers.get('user-agent'))
  return new Response()
})
await fetch('http://localhost:3000')
Deno.exit(0)

What is the feature you are proposing to solve the problem?

I propose that the default user agent in Node.js changes to the format used by Deno, and Bun for consistency.

Example: Node/18.18.0

What alternatives have you considered?

Related: nodejs/node#43852 nodejs/node#43851

@Ethan-Arrowood
Copy link
Collaborator

I'm 👍 for this change. Undici should maybe be dynamic and still return undici when called directly from Undici, but then when in Node.js it uses Node/<Version>. Not sure how to implement that exactly off the top of my head though.

@mscdex
Copy link

mscdex commented Oct 2, 2023

I'd rather have User-Agent be opt-in, but at the very least it should not contain version information.

@Ethan-Arrowood
Copy link
Collaborator

What would the opt-in mechanism be?

I also agree -- for lower level http apis. Fetch is high-enough level that I believe its okay to be making default decisions like user-agent header.

@KhafraDev
Copy link
Member

If httpRequest’s header list does not contain User-Agent, then user agents should append (User-Agent, default User-Agent value) to httpRequest’s header list.

spec-wise we have to add a user-agent header.

@balazsorban44
Copy link
Contributor Author

balazsorban44 commented Oct 2, 2023

we have to add

I'd rather have User-Agent be opt-in

Keep in mind, it's already there, its value is just confusing.

What would be the argument for not adding a version number? Most clients I can think of has one. 🤔

@mscdex
Copy link

mscdex commented Oct 2, 2023

What would be the argument for not adding a version number?

At least for security reasons.

@KhafraDev
Copy link
Member

Keep in mind that browsers are actually removing info from the user agent headers, for the reason mentioned above. https://developer.chrome.com/blog/user-agent-reduction-android-model-and-version/

@balazsorban44 balazsorban44 changed the title Change default "user-agent" to Node/version Change default "user-agent" from undici Oct 3, 2023
@benjamingr
Copy link
Member

I'm +1 to changing it to simply "node" or something similar

@Ethan-Arrowood
Copy link
Collaborator

Based on the way browsers are going let's settle on switching it to "node" or "nodejs". Since that is significantly more recognizable than "undici"

@KhafraDev
Copy link
Member

I don't have an opinion, anything is fine with me

@KhafraDev KhafraDev transferred this issue from nodejs/node Oct 4, 2023
@mcollina
Copy link
Member

mcollina commented Oct 4, 2023

I would rather not have one, but the spec mandate us. node is better than undici because it's 3 bytes less.

Who wants to do the honors?

@Ethan-Arrowood
Copy link
Collaborator

On it 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants