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

v3 incorrectly strips out GET params [?] #749

Closed
dzek69 opened this issue Mar 15, 2020 · 4 comments
Closed

v3 incorrectly strips out GET params [?] #749

dzek69 opened this issue Mar 15, 2020 · 4 comments
Labels

Comments

@dzek69
Copy link
Contributor

dzek69 commented Mar 15, 2020

Example code, use Node 13.2+:

index.mjs

import express from "express";
import f from "node-fetch";
const fetch = f.default; // Native ESM

const app = express();
app.get("*", (req, res) => {
    console.log("srv", req.url)
    res.send("yo");
});
app.listen(5001);

(async () => {
    const res = await fetch("http://127.0.0.1:5001/api/getStuff?a=tra").then(r => r.text())
    console.log(res);
})().catch(e => {
    console.error(e);
});

With 3.0.0-beta.4 version of node-fetch you'll see just srv /api/getStuff in the console
With 2.6.0 you will see srv /api/getStuff?a=tra - as expected

So it looks like somehow GET query params gets stripped out.

@KhafraDev
Copy link
Contributor

Can't reproduce on Windows 10/Node v13.9.0 with your example code. Tried it a bit and then modified the script to run a couple thousand times. ->

(node:7816) ExperimentalWarning: The ESM module loader is experimental.
srv /api/getStuff?a=tra
yo

@dzek69
Copy link
Contributor Author

dzek69 commented Mar 17, 2020

I have prepared a repository:
https://github.com/dzek69/node-fetch-bug-demo

Use yarn to keep the same versions as me.
Run yarn start to test on your local node version
Run yarn docker to run in docker with node 13.10.1
Run yarn docker:12 to run in docker with node 12

I have also built and published images on docker hub:
dzek69/node-fetch-bug:12 -- node-fetch v3 + node 12
dzek69/node-fetch-bug:13 -- node-fetch v3 + node 13
dzek69/node-fetch-bug:v2 -- node-fetch v2 + node 13 -- this works correctly, because it's v2 version

In every environment this behaves the same for me (that's why I've build docker images so you can see it too if this somehow runs correctly on your machine)

If I run docker images on my Linux machine it behave the same as when run from windows:

dzek@srv:~$ docker run dzek69/node-fetch-bug:12
Unable to find image 'dzek69/node-fetch-bug:12' locally
12: Pulling from dzek69/node-fetch-bug
c9b1b535fdd9: Pull complete
6ad7c27326e7: Pull complete
968d8b4948da: Pull complete
a1ec837a6331: Pull complete
f462b809bd5c: Pull complete
bc393c91984a: Pull complete
Digest: sha256:5fdeca44d4d8a8e75b8ea6e90625357be8e6e1992c332599cdea030784953c59
Status: Downloaded newer image for dzek69/node-fetch-bug:12
v12.16.1
yarn run v1.22.0
$ node index.js
server /api/getStuff INCORRECT URL
client this is the result
Done in 0.17s.
dzek@srv:~$ docker run dzek69/node-fetch-bug:13
Unable to find image 'dzek69/node-fetch-bug:13' locally
13: Pulling from dzek69/node-fetch-bug
c9b1b535fdd9: Already exists
8488f113df73: Pull complete
09953e135439: Pull complete
b1863e3df3d5: Pull complete
4225ec739664: Pull complete
7ed84526c057: Pull complete
Digest: sha256:0d0b8d45fdcd20e57c133efa10b1045d11907ce905b0789b0969a40ef563a22f
Status: Downloaded newer image for dzek69/node-fetch-bug:13
v13.10.1
yarn run v1.22.0
$ node index.js
server /api/getStuff INCORRECT URL
client this is the result
Done in 0.19s.
dzek@srv:~$ docker run dzek69/node-fetch-bug:v2
Unable to find image 'dzek69/node-fetch-bug:v2' locally
v2: Pulling from dzek69/node-fetch-bug
c9b1b535fdd9: Already exists
8488f113df73: Already exists
09953e135439: Already exists
b1863e3df3d5: Already exists
4225ec739664: Already exists
033c42270e9e: Pull complete
Digest: sha256:da39a56fd3c2b75673096795ad7fbfa7324db3f5b55ab69de0db105a70e7d2e5
Status: Downloaded newer image for dzek69/node-fetch-bug:v2
v13.10.1
yarn run v1.22.0
$ node index.js
server /api/getStuff?a=tra this is OK
client this is the result
Done in 0.18s.

Please take another look at it

@gp-slick-coder
Copy link

node-fetch does not consider query strings

https://api.exchangeratesapi.io/latest?base=USD&symbols=EUR

will result to

https://api.exchangeratesapi.io/latest

@xxczaki
Copy link
Member

xxczaki commented Apr 13, 2020

Fixed by #759

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

Successfully merging a pull request may close this issue.

4 participants