nodejs / undici

An HTTP/1.1 client, written from scratch for Node.js
https://nodejs.github.io/undici
MIT License
6.1k stars 531 forks source link

ProxyAgent ignores credentials #1674

Open negezor opened 2 years ago

negezor commented 2 years ago

Bug Description

I was expecting the classic behavior of passing a proxy URL to be parsed apart. However, ProxyAgent ignores the auth part.

Reproducible By

import { ProxyAgent, request } from 'undici';

const PROXY_URL = 'http://USERNAME:PASSWORD@HOST';

const dispatcher = new ProxyAgent(PROXY_URL);

const response = await request('https://google.com', {
    dispatcher
});
// HTTP 407 Proxy Authentication Required

console.log('response', response);

Expected Behavior

Server response result

Logs & Screenshots

/home/negezor/projects/repro/node_modules/undici/lib/proxy-agent.js:67
            callback(new RequestAbortedError('Proxy response !== 200 when HTTP Tunneling'))
                     ^

RequestAbortedError [AbortError]: Proxy response !== 200 when HTTP Tunneling
    at Client.connect (/home/negezor/projects/repro/node_modules/undici/lib/proxy-agent.js:67:22)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  code: 'UND_ERR_ABORTED'
}

Environment

Node.js v18.10.0 Undici v5.10.0

Additional context

This is easy to fix, however this is non-obvious behavior.

import { ProxyAgent, request } from 'undici';

const PROXY_URL = 'http://USERNAME:PASSWORD@HOST';

const url = new URL(PROXY_URL);
const dispatcher = new ProxyAgent({
    uri: PROXY_URL,
    auth: Buffer.from(`${url.username}:${url.password}`).toString('base64')
});

const response = await request('https://google.com', {
    dispatcher
});

console.log('response', response);
// Full response
mcollina commented 2 years ago

cc @RafaelGSS

RafaelGSS commented 2 years ago

That's expected. The ProxyAgent uses HTTP Tunneling to perform a request. Would you like to open a PR to clarify it in the documentation?

julien-f commented 6 months ago

How is that expected?

Even when using tunneling (using CONNECT method), you still have to do a normal HTTP request to the proxy and you can use the proxy-authenticate to authenticate against it.

Am I missing anything?

Using credentials in the URL does work natively with cURL.

metcoder95 commented 6 months ago

The ProxyAgent parses the URL accordingly to WHATWG URL spec, and it respects the username:password if no opts.auth is provided; do you have a reproducible example?

julien-f commented 6 months ago

My bad, it's indeed working correctly, sorry for the noise.

The problem I had was that it does not handle username without a :password, while it is accepted by Node's URL class and appears to be valid if I understand the Wikipedia page:

userinfo subcomponent [...] may consist of a user name and an optional password preceded by a colon (:)

-- https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Syntax

Uzlopak commented 6 months ago

Wouldnt it be solved by setting password to an empty string?

metcoder95 commented 6 months ago

Yeah, indeed won't work as the ProxyAgent seems not designed to handle that scenario; but as @Uzlopak said, that's a valid workaround.

The spec for Basic Auth scheme does states about separating the username and password by a colon, being the password a possible empty text.

It is up to the server decide what to do on those cases. A PR for supporting can be welcomed from my side

julien-f commented 6 months ago

Regardless of the presence of :, it does not appear to work natively when the password is empty.

My current work-around:

const uri = new URL(proxy);
const token =
  "Basic " +
  Buffer.from(`${uri.username}:${uri.password}`).toString("base64");
const dispatcher = new ProxyAgent({ token, uri });

Unfortunately I don't have time to investigate in undici code for a PR.