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

Library erroneously URI-encodes usersname and password #2093

Closed
1 of 2 tasks
JoshuaWise opened this issue Jul 28, 2022 · 4 comments
Closed
1 of 2 tasks

Library erroneously URI-encodes usersname and password #2093

JoshuaWise opened this issue Jul 28, 2022 · 4 comments

Comments

@JoshuaWise
Copy link

JoshuaWise commented Jul 28, 2022

Describe the bug

When Got builds an Authorization header for basic auth, it URI-encodes the username and password. This is not warranted by any spec, and it breaks requests that use special characters in their username or password.

Actual behavior

Got URI-encodes the username and password used in the Authorization header for basic auth.

Expected behavior

Got should send whatever username or password is specified by the user, without modification.

Code to reproduce

const client = Got.extend({
  username: process.env.AUTH_USER,
  password: process.env.AUTH_PASS,
});

await client.get(url);

Checklist

  • I have read the documentation.
  • I have tried my code with the latest version of Node.js and Got.
@szmarczak
Copy link
Collaborator

got/source/core/options.ts

Lines 1842 to 1883 in e032b60

get username(): string {
const url = this._internals.url as URL;
const value = url ? url.username : this._internals.username;
return decodeURIComponent(value);
}
set username(value: string) {
assert.string(value);
const url = this._internals.url as URL;
const fixedValue = encodeURIComponent(value);
if (url) {
url.username = fixedValue;
} else {
this._internals.username = fixedValue;
}
}
get password(): string {
const url = this._internals.url as URL;
const value = url ? url.password : this._internals.password;
return decodeURIComponent(value);
}
set password(value: string) {
assert.string(value);
const url = this._internals.url as URL;
const fixedValue = encodeURIComponent(value);
if (url) {
url.password = fixedValue;
} else {
this._internals.password = fixedValue;
}
}

const {headers, username, password} = options;

got/source/core/index.ts

Lines 1060 to 1063 in e032b60

if (username || password) {
const credentials = Buffer.from(`${username}:${password}`).toString('base64');
headers.authorization = `Basic ${credentials}`;
}

show otherwise. Can you please post an actual code to reproduce the issue? The one you posted isn't valid.

@JoshuaWise
Copy link
Author

I can reproduce the bug in v11.8.5, but not v12.3.0. I can't upgrade to version 12 because using ES modules would require an entire refactor of our build system.

@JoshuaWise
Copy link
Author

JoshuaWise commented Jul 28, 2022

Here's how to reproduce:

'use strict';
const http = require('http');
const Got = require('got');

const client = Got.extend({
	username: 'admin',
	password: 'special_chars_^#@',
});

const server = http.createServer((req, res) => {
	const auth = req.headers.authorization;
	if (!auth || !auth.startsWith('Basic ')) {
		return res.writeHead(401).end();
	}

	const base64Creds = auth.slice(6);
	const plainCreds = Buffer.from(base64Creds, 'base64').toString();
	const [username, password] = plainCreds.split(':');

	console.log('username: %s\npassword: %s', username, password);

	return res.writeHead(204).end();
});

server.listen(0, () => {
	const { port } = server.address();

	client.get(`http://localhost:${port}`).finally(() => server.close());
});

It outputs:

username: admin
password: special_chars_%5E%23%40

@szmarczak
Copy link
Collaborator

got/source/core/index.ts

Lines 1690 to 1702 in 5e17bb7

// Update `username`
if (options.username === '') {
options.username = options.url.username;
} else {
options.url.username = options.username;
}
// Update `password`
if (options.password === '') {
options.password = options.url.password;
} else {
options.url.password = options.password;
}

In Got v11 we don't do any special handling. The bug is in upstream, see nodejs/node#31450

Duplicate of #1169

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

No branches or pull requests

2 participants