The authorization header is improperly encoded

[bruno-xo7]

Describe the bug

Actual behavior

When I use the username/password with got, It seems that the combo 'username:password' is encoded with encodeURI() before to be encode in base64 for the basic auth. This first encoding is not necessary due to the base64 encoding and brings problems. So when the encodeURI() change a value like '@' to '%40', the basic authentification doesn't work.

Expected behavior

Just encode the combo 'username:password' with base64 encoding.

Code to reproduce

const got = require('got');

(async () => {
  const response = await got.head("http://example.com/", {username: "user@", password: "password@"});
  console.info("Response:", response.client._httpMessage._header);
}) ();

We see that the Authorization header is invalid

Checklist

• I have read the documentation.
• I have tried my code with the latest version of Node.js and Got.

[szmarczak]

It's a Node.js issue:

const https = require('https');
const getStream = require('get-stream');

const url = new URL('https://httpbin.org/anything');
url.username = 'user@';
url.password = 'password';

https.get(url, async response => {
    const body = await getStream(response);
    console.log(JSON.parse(body));
});

I'm not sure whether it's a bug or valid behavior...

[szmarczak]

Reference: https://tools.ietf.org/html/rfc2617 (haven't looked at it yet)

[szmarczak]

Took a quick look and found this:

      user-pass   = userid ":" password
      userid      = *<TEXT excluding ":">
      password    = *TEXT

[szmarczak]

nodejs/node#31439

[bruno-xo7]

also nodejs/node#31450

The both issues are still open in Nodejs.

I did this workaround on my side until Nodejs fixes the issue:

  const headers = {
    "Authorization": "Basic " + Buffer.from(`user@:password@`).toString("base64")
  };
  await got(encodeURI("http://example.com/"), { headers });

I built the Authorization header instead of filling username and password members of the options objects.

If Nodejs only fixes it on the new version, maybe Got can fix the issue on their side to avoid the bug on old Nodejs versions.

[szmarczak]

maybe Got can fix the issue on their side

We could implement a workaround, but it's not the proper solution. The bug is not a big one, so I'd rather wait until it's fixed on Node.js side. But if you really need this badly, you can send a PR :)

[kamikazechaser]

@szmarczak Let the library encode it properly, internally. I feel this is a major issue since the library has the options.username and options.password option.

[andyburke]

Got caught by this. Feeling agreement with @kamikazechaser: got could essentially make this work correctly by handling the username/password itself.

[legopin]

At the very least there should be documentation linking to this issue, took me a while to suspect what went wrong
https://github.com/sindresorhus/got/blob/main/documentation/2-options.md#username

[szmarczak]

Let's fix this. I don't think Node.js will fix this anytime soon, given how much time we have already waited. Furthermore, both Google Chrome and Firefox don't support (which is weird) username and password in URL in fetch.

[szmarczak]

Actually we just have to wait for a Node.js release: nodejs/node#39310 🎉

[szmarczak]

I'll fix this later today in Got :)

[AbdelrahmanHafez]

@szmarczak
Would a PR to merge this fix into v11 be welcome?
I just spent 3 hours trying to fix issues caused by this bug and I would love to prevent other people from having the same issue.

[Joe-Palmer]

I would very much welcome this fix in v11