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

Upgrading from 19.3.0 to 19.4.0 breaks fetch request #46287

Closed
zachsa opened this issue Jan 20, 2023 · 20 comments
Closed

Upgrading from 19.3.0 to 19.4.0 breaks fetch request #46287

zachsa opened this issue Jan 20, 2023 · 20 comments
Labels
fetch Issues and PRs related to the Fetch API

Comments

@zachsa
Copy link

zachsa commented Jan 20, 2023

Version

v19.4.0

Platform

inux ZACH-SP4 5.15.79.1-microsoft-standard-WSL2 #1 SMP Wed Nov 23 01:01:46 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

I have a simple fetch function that works fine using Node v19.3.0 (and earlier), but that breaks when I upgrade to Node v19.4.0:

import btoa from 'btoa'
import { FormData } from 'formdata-node'

const form = new FormData()
form.append('grant_type', 'client_credentials')
form.append('scope', SCOPES)

const { access_token, expires_in, scope, token_type } = await fetch(
  `${ORY_HYDRA}/oauth2/token`,
  {
    method: 'POST',
    headers: {
      Authorization: `Basic ${TOKEN}`,
    },
    body: form,
  }
).then(res => {
  if (res.status !== 200) {
    throw new Error(`${res.statusText} (${res.status})`)
  }
  return res.json()
})

const Authorization = [token_type, access_token].join(' ')

const res = await fetch(
  `SOME_SERVICE`,
  {
    method: 'GET',
    headers: {
      accept: 'application/json',
      Authorization,
    },
  }
)

console.log(data?.detail)

Using Node < v19.3.0, the request to SOME_SERVICE always authenticates successfully. Node Node v19.4.0 the data?.detail log is always Not authenticated even though I am able to get the access token from the initial request.

How often does it reproduce? Is there a required condition?

The above always occurs

What is the expected behavior?

It seems like the fetch implementation is changed in v19.4. Is this the case? At a guess it seems like something is being encoded differently between the two Node.js versions.

What do you see instead?

The fetch implementation seems to have changed

Additional information

No response

@bnoordhuis
Copy link
Member

Can you check v19.5.0? It contains several bug fixes that may be relevant.

If it still doesn't work, can you post a test case that doesn't depend on third-party modules? Thanks.

@bnoordhuis bnoordhuis added the fetch Issues and PRs related to the Fetch API label Jan 31, 2023
@zachsa
Copy link
Author

zachsa commented Jan 31, 2023

Thank you for the feedback. 19.5.0 also does not work.

I am using the Node.js global fetch object. what could be the cause of the issue (I'm not sure how to replicate this)?

@targos
Copy link
Member

targos commented Jan 31, 2023

Is there a redirect in your request? I suspect nodejs/undici#1780, which is related to the Authorization header.
/cc @nodejs/undici

@zachsa
Copy link
Author

zachsa commented Jan 31, 2023

I think there is. How can I check this?

@mcollina
Copy link
Member

mcollina commented Feb 1, 2023

Can you also include a demo server to reproduce your problem?

The spec says that the Authorization header should be removed if cross-origin redirects happen. Is this the case?

@zachsa
Copy link
Author

zachsa commented Feb 1, 2023

It may be - I'm checking if there are redirects occurring with the person who manages the ORY Hydra service

@zachsa
Copy link
Author

zachsa commented Feb 1, 2023

Which spec is it so that I can pass it on if necessary?

@ronag
Copy link
Member

ronag commented Feb 1, 2023

Which spec is it so that I can pass it on if necessary?

whatwg/fetch#1544

@targos
Copy link
Member

targos commented Feb 1, 2023

You can use Response.redirected to know if there was redirection(s).

@zachsa
Copy link
Author

zachsa commented Feb 2, 2023

You can use Response.redirected to know if there was redirection(s).

Thanks - yes. I can see that the res does get a redirect. However... in this case the redirect just adds a trailing slash to the URL. So:

fetch('https://domain.com/api/route?someparams...') gets redirected to Location: https://domain.com/api/route/?someparams...

and fetch('https://domain.com/api/route/?someparams') works

Seems very strict dropping the Authorization header in this case...

@KhafraDev
Copy link
Member

The authorization header isn't being dropped in your example because the port, hostname, and protocols are the same. Fetch doesn't check the path to ensure a url is from the same origin. Can you provide steps to reproduce the issue?

@zachsa
Copy link
Author

zachsa commented Feb 2, 2023

It would be difficult to setup an example - as far as I know the only way to do this would be to duplicate the setup of the 3rd party service. I have asked for an oauth client that could be used to replicate this (the server is public).

(Also there is a cloudflare proxy in the middle - not sure if that is a factor or what to check for)

@jimrandomh
Copy link

Also had a program break, on node 18.15.0. The script makes a POST with an Authorize header to https://api.roamresearch.com and receives an http 308 redirect to https://peer-65.api.roamresearch.com:3001. The fetch function follows the redirect, but doesn't send the Authorize header to the second server. It works if I use an older version of nodejs, the node-fetch library, or curl.

I suspect that this may have been done for security reasons, and that there may be an option somewhere to change this behavior, but unfortunately the issue is exacerbated by a second problem: fetch does not have any documentation inside of nodejs, just a link to the MDN documentation of the in-browser fetch function. That documentation refers to a bunch of origin- and cookie-related security options that clearly don't apply. So it's not clear what options nodejs's fetch has, or what they do.

@jimrandomh
Copy link

18.13.0: Works
18.14.0: Fails
18.15.0: Fails
18.16.0: Fails

@KhafraDev
Copy link
Member

Dropping the Authorization header on cross-origin redirects is expected, see whatwg/fetch@9004f4e.

@jimrandomh
Copy link

The spec isn't a sufficient reason, unless there is much better documentation and warning around this. This diverges from the behavior of very-recent versions and still-used polyfills, in a way that breaks things.

@KhafraDev
Copy link
Member

Fetch is marked as experimental, which means that "[t]he feature is not subject to semantic versioning rules. Non-backward compatible changes or removal may occur in any future release. Use of the feature is not recommended in production environments."1

Footnotes

  1. https://nodejs.org/dist/latest-v19.x/docs/api/documentation.html#stability-index

@jimrandomh
Copy link

jimrandomh commented Apr 16, 2023

I guess my disagreement here is more with the specification change itself, rather than with its implementation in nodejs, so I made an issue on the whatwg issue tracker. For my own use case, I've solved the problem by switching from nodejs's builtin fetch to the polyfill library node-fetch, but going forward I'm worried that library is going to make the same change, which would force me to either pin an outdated version or rewrite my code to use a non-fetch-like http library.

@bnoordhuis
Copy link
Member

Since this is spec conforming behavior, I'm going to go ahead and close this out as not-a-bug.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Apr 16, 2023
@zachsa
Copy link
Author

zachsa commented Apr 16, 2023

The authorization header isn't being dropped in your example because the port, hostname, and protocols are the same. Fetch doesn't check the path to ensure a url is from the same origin. Can you provide steps to reproduce the issue?

I think there may be a bug here. In my case I just changed the URL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fetch Issues and PRs related to the Fetch API
Projects
None yet
Development

No branches or pull requests

7 participants