node icon indicating copy to clipboard operation
node copied to clipboard

esm(network imports): expose `User-Agent` header

Open Fyko opened this issue 3 years ago • 1 comments

What is the problem this feature will solve?

We're building a registry server to serve our JavaScript packages in a variety of runtimes -- Node.js, Deno and the browser. Both the browser and Deno include a User-Agent header, but Node doesn't.

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

I propose altering the GET functions to include the user-agent header. https://github.com/nodejs/node/blob/5e3d003113739672b4f56a0fa4e8109833461f3a/lib/internal/modules/esm/fetch_module.js#L47-L69

However, this poses some security concerns. Should it be exposed by default? Should there be another flag? node --experimental-network-imports --experimental-network-imports-expose-ua ./foo.mjs.

Fyko avatar Jul 15 '22 17:07 Fyko

FYI we're considering replacing fetch_module with fetch

JakobJingleheimer avatar Jul 15 '22 20:07 JakobJingleheimer

The ask is basically to add this?

diff --git a/lib/internal/modules/esm/fetch_module.js b/lib/internal/modules/esm/fetch_module.js
index d79e5a5eb99..7a52ae4c985 100644
--- a/lib/internal/modules/esm/fetch_module.js
+++ b/lib/internal/modules/esm/fetch_module.js
@@ -124,6 +124,7 @@ function fetchWithRedirects(parsed) {
   const result = (async () => {
     const req = handler(parsed, {
       headers: { Accept: '*/*' },
+      'User-Agent': 'node',
     });
     // Note that `once` is used here to handle `error` and that it hits the
     // `finally` on network error/timeout.

Pull request welcome, I think? Adding the version might leak a little too much info but then again, maybe not.

edit: I missed #43852 already exists and met some fierce opposition.

bnoordhuis avatar Mar 18 '23 10:03 bnoordhuis

The ask is basically to add this?

diff --git a/lib/internal/modules/esm/fetch_module.js b/lib/internal/modules/esm/fetch_module.js
index d79e5a5eb99..7a52ae4c985 100644
--- a/lib/internal/modules/esm/fetch_module.js
+++ b/lib/internal/modules/esm/fetch_module.js
@@ -124,6 +124,7 @@ function fetchWithRedirects(parsed) {
   const result = (async () => {
     const req = handler(parsed, {
       headers: { Accept: '*/*' },
+      'User-Agent': 'node',
     });
     // Note that `once` is used here to handle `error` and that it hits the
     // `finally` on network error/timeout.

Pull request welcome, I think? Adding the version might leak a little too much info but then again, maybe not.

edit: I missed #43852 already exists and met some fierce opposition.

I would suggest something more informative + align with deno user agent: Node/{version}

It would help servers polyfill features if not supported on current version of nodejs, etc

renhiyama avatar Jul 10 '23 16:07 renhiyama

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] avatar Jan 07 '24 01:01 github-actions[bot]

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] avatar Feb 07 '24 01:02 github-actions[bot]