Add default user-agent + enable custom one

[voxpelli]

The RFC9110 HTTP Semantics spec says:

A user agent SHOULD send a User-Agent header field in each request unless specifically configured not to do so.

So I think we should.

Convention is that a library appends their user-agent to the one that's been set at the level above them, hence the user provided user-agent comes before the default one set in this module.

To provide context, the version number of this module + the home page of it is included in the User-Agent. That way an API can diagnose possibly faulty consumers and file an issue.

If someone wouldn't want to expose that info in a header, they can always set a custom User-Agent header directly in the http settings. Especially if #225 gets merged.

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[Uzlopak]

I dont like the idea to expose unnecessary information in security context. Also this is not a browser.

[voxpelli]

I dont like the idea to expose unnecessary information in security context.

Can you elaborate? It's a SHOULD in the HTTP spec and common good practice?

Also this is not a browser.

User agent is the term for any HTTP client, see: https://www.rfc-editor.org/rfc/rfc9110#name-user-agents

[Uzlopak]

I actually dont see any gain from this. Only exposing information. We also dont send in other plugins any user agent.

I think this is a semver major if we by default send user agent.

Imho by default dont send user agent is the best, but still i dont think it is good decision to add this feature.

[voxpelli]

@Uzlopak How about doing like undici and skip the version and the url and just include the name of the module? https://github.com/nodejs/undici/blob/111fd2388094270259814ca7ea5da7d07b2126f6/lib/fetch/index.js#L1346-L1348

Seems to be what node-fetch is also doing: https://github.com/node-fetch/node-fetch/blob/8b3320d2a7c07bce4afc6b2bf6c3bbddda85b01f/src/request.js#L278

got skips version but adds URL: https://github.com/sindresorhus/got/blob/944caa8ce8151d7b18ab17c4130b18d622c04ebe/source/core/options.ts#L714

Though got does add version in gh-got: https://github.com/sindresorhus/got/blob/944caa8ce8151d7b18ab17c4130b18d622c04ebe/documentation/examples/gh-got.js#L18 Probably because GitHub used to require that clients set a user-agent: https://stackoverflow.com/a/39912696/20667

[Uzlopak]

Yes, setting it to fixed value without require etc. is definetly better. I would prefer that any time.

[voxpelli]

Some more points of reference:

• curl defaults to eg. curl/7.54.1: https://everything.curl.dev/http/requests/user-agent
• hubot also includes version: https://github.com/hubotio/hubot/blob/9865cb2db4663006e0a0c5c506ffd3f3e3a3a3c6/src/robot.js#L748
• webtorrent includes version and reads it from package.json: https://github.com/webtorrent/webtorrent/blob/d61b59add1989ce2197759eecab7ba0a6418cdc4/lib/torrent.js#L59
• Some go even further and use universal-user-agent to also add in info about Node.js version and system architecture, such as eg. Octokit: https://github.com/octokit/request.js/blob/29edb28325de3a60442e15c7d45f20be9c4d350d/src/index.ts#L9

Yes, setting it to fixed value without require etc. is definetly better. I would prefer that any time.

@Uzlopak Is this as a reply to #226 (comment) ?

[Uzlopak]

I would set the user agent to 'fastify-oauth2'.

[voxpelli]

@Uzlopak Done ✔️ I can get behind that, especially considering that it appears to be the common practice in the npm ecosystem nowadays 👍

[voxpelli]

Rebased to do #226 (comment)

[mcollina]

lgtm

[Uzlopak]

So you can not truely override the useragent?

[Uzlopak]

Expected more something like

  let userAgent = {}
  switch (options.userAgent) {
    case false:
      break
    case undefined:
      userAgent['User-Agent'] = USER_AGENT
      break
    default:
      userAgent['User-Agent'] = options.userAgent
      break
  }

  const credentials = {
    ...options.credentials,
    http: {
      ...options.credentials.http,
      headers: {
        ...userAgent,
        ...options.credentials.http?.headers
      }
    }
  }

[voxpelli]

So you can not truely override the useragent?

You can, if you override the http settings

On other comment:

I prefer using const over let and avoid switch unless checking one bar against multiple values (as a missing break can cause havoc and it’s a bit unusual in that body of a case doesn’t come with its own scope, unless you add one, so it’s easy for people to make mistakes with)

[Uzlopak]

If I set userAgent to "Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 5.1; Trident/4.0)", then the plugin is changing the useragent to: "Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 5.1; Trident/4.0) fastify/oauth2"

So how can I override the useragent?

[voxpelli]

@Uzlopak This is what the RFC 9110 says, my emphasis:

The User-Agent field value consists of one or more product identifiers, each followed by zero or more comments (Section 5.6.5), which together identify the user agent software and its significant subproducts. By convention, the product identifiers are listed in decreasing order of their significance for identifying the user agent software. Each product identifier consists of a name and optional version.

This is what this PR up to now has done + I have added tests + comments on how to override it if one wants to.

But to avoid confusion I have now removed such prepending of the additional user-agent and instead outright replaces the default one. Removes possibly valuable information but I hope that fixes your concerns.

[Uzlopak]

LGTM