sindresorhus / got

🌐 Human-friendly and powerful HTTP request library for Node.js
MIT License
14.27k stars 935 forks source link

Equivalent POST request works with request-promise but not with Got #1730

Closed woacademy closed 3 years ago

woacademy commented 3 years ago

Describe the bug

I'd like to convert a piece of code that logs into a website and retrieves some data from request-promise to got. The login information is not important (node index.js foo bar) as the request-promise sample will fail but the got request will hang for a long time and seemingly never resolve. The expected response is a 302, with the location header being used to verify login success.

Actual behavior

Request false
[...]

Expected behavior

Request false
Got false

Code to reproduce

const request = require('request-promise');
const got = require('got');
const tough = require('tough-cookie');
const FormData = require('form-data');

const loginWithRequest = async (username, password) => {
  const jar = request.jar();
  await request('https://www.unlimitedhorizon.co.uk', {jar: jar});

  const response = await request.post('https://www.unlimitedhorizon.co.uk/webapp/signin?wicket:interface=:0:wrap:inner:signInPanel:signInForm::IFormSubmitListener::', {
    simple: false, // Don't error on 302.
    resolveWithFullResponse: true, // Include response headers.
    jar: jar,
    form: {
      'username': username,
      'password': password,
    }
  });

  return (response.headers.location == 'dashboard/admin');
};

const loginWithGot = async (username, password) => {
  const jar = new tough.CookieJar();
  await got('https://www.unlimitedhorizon.co.uk', {cookieJar: jar});

  const form = new FormData();
  form.append('username', username);
  form.append('password', password);

  const response = await got.post('https://www.unlimitedhorizon.co.uk/webapp/signin?wicket:interface=:0:wrap:inner:signInPanel:signInForm::IFormSubmitListener::', {
    cookieJar: jar,
    body: form
  });

  // Not adapted to got yet but gets stuck before this point.
  return (response.headers.location == 'dashboard/admin');
};

(async () => {
  console.log('Request', await loginWithRequest(process.argv[2], process.argv[3]));
  console.log('Got', await loginWithGot(process.argv[2], process.argv[3]))
})();

Checklist

szmarczak commented 3 years ago

If you set a timeout, the request times out. Can you try the main branch?

npm install "sindresorhus/got#62305d77d3428b5c714d21b4bbee68cc75b5f787"
woacademy commented 3 years ago

Can you try the main branch?

$ npm install "sindresorhus/got#62305d77d3428b5c714d21b4bbee68cc75b5f787"
[...]
> got@11.8.0 build
> del-cli dist && tsc

test/stream.ts:490:35 - error TS2769: No overload matches this call.
  The last overload gave the following error.
    Type 'Readable' is not assignable to type 'string | Readable | Buffer | undefined'.
      Type 'Readable' is missing the following properties from type 'Readable': readableEncoding, readableEnded, readableObjectMode, pipe, and 8 more.

490  const response = await got.post({body});
                                      ~~~~

  source/core/options.ts:1058:6
    1058  get body(): string | Buffer | Readable | undefined {
              ~~~~
    The expected type comes from property 'body' which is declared here on type 'OptionsInit'
  source/types.ts:171:2
    171  (options: OptionsInit): CancelableRequest | Request;
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    The last overload is declared here.

Found 1 error.

error Command failed with exit code 1.
szmarczak commented 3 years ago

In that case try cloning the repo and builiding it from there.

npm install && npm run build
woacademy commented 3 years ago

@szmarczak this gives me the same error, even when building the commit you referenced. The same error can be seen in the builds on GitHub.

woacademy commented 3 years ago

Managed to install got from HEAD with the latest commit and it still hangs when running this sample. A timeout can be set but that doesn't resolve the issue of request working on got not.

szmarczak commented 3 years ago

It's very likely that the issue is with FormData. Replacing FormData with string gives me 401 as expected. Continuing investigation.

szmarczak commented 3 years ago

Looks like a FormData bug: https://runkit.com/szmarczak/60bded210d856c0013ab2b6b

Passing it to PassThrough first does the trick.

woacademy commented 3 years ago

Looks like a FormData bug: https://runkit.com/szmarczak/60bded210d856c0013ab2b6b

Passing it to PassThrough first does the trick.

Thank you for looking into this, appreciate it! This does seem to get one step closer but the 401 is unexpected, request comes back with a 302 which I'd expect as that's what I see when executing this with a browser. To be clear the response code should be 302 regardless of failure or success. Any ideas?

szmarczak commented 3 years ago
...
    followRedirect: false,
...
szmarczak commented 3 years ago

https://github.com/sindresorhus/got#followredirect

woacademy commented 3 years ago

https://github.com/sindresorhus/got#followredirect

Thank you. Everything is working as expected now, unfortunately when passing the correct credentials request is able to login, and got isn't (with console.log(response.headers.location);):

$ node index.js foo bar
dashboard/admin
Request true
?wicket:interface=:0::::
Got false

I realise you won't be able to help me diagnose that part but could you describe how to pass it as a string like you mentioned earlier?

It's very likely that the issue is with FormData. Replacing FormData with string gives me 401 as expected.

szmarczak commented 3 years ago

could you describe how to pass it as a string like you mentioned earlier?

Simply pass a string to body. body: 'data'

I realise you won't be able to help me diagnose that part

You can use Wireshark or any other tool to debug this.

szmarczak commented 3 years ago

Note that in the request example you're using application/x-www-form-urlencoded but in the Got example you're using multipart/form-data. Got accepts the first as well, using the form option.