diff --git a/packages/api/src/utils/client/httpClient.ts b/packages/api/src/utils/client/httpClient.ts index 6015d9b6715f..54a7ca8d4e42 100644 --- a/packages/api/src/utils/client/httpClient.ts +++ b/packages/api/src/utils/client/httpClient.ts @@ -1,4 +1,4 @@ -import {ErrorAborted, Logger, TimeoutError} from "@lodestar/utils"; +import {ErrorAborted, Logger, TimeoutError, isValidHttpUrl, toBase64} from "@lodestar/utils"; import {ReqGeneric, RouteDef} from "../index.js"; import {ApiClientResponse, ApiClientSuccessResponse} from "../../interfaces.js"; import {fetch, isFetchError} from "./fetch.js"; @@ -123,8 +123,15 @@ export class HttpClient implements IHttpClient { // opts.baseUrl is equivalent to `urls: [{baseUrl}]` // unshift opts.baseUrl to urls, without mutating opts.urls - for (const urlOrOpts of [...(baseUrl ? [baseUrl] : []), ...urls]) { + for (const [i, urlOrOpts] of [...(baseUrl ? [baseUrl] : []), ...urls].entries()) { const urlOpts: URLOpts = typeof urlOrOpts === "string" ? {baseUrl: urlOrOpts, ...allUrlOpts} : urlOrOpts; + + if (!urlOpts.baseUrl) { + throw Error(`HttpClient.urls[${i}] is empty or undefined: ${urlOpts.baseUrl}`); + } + if (!isValidHttpUrl(urlOpts.baseUrl)) { + throw Error(`HttpClient.urls[${i}] must be a valid URL: ${urlOpts.baseUrl}`); + } // De-duplicate by baseUrl, having two baseUrls with different token or timeouts does not make sense if (!this.urlsOpts.some((opt) => opt.baseUrl === urlOpts.baseUrl)) { this.urlsOpts.push(urlOpts); @@ -269,7 +276,7 @@ export class HttpClient implements IHttpClient { const timer = this.metrics?.requestTime.startTimer({routeId}); try { - const url = urlJoin(baseUrl, opts.url) + (opts.query ? "?" + stringifyQuery(opts.query) : ""); + const url = new URL(urlJoin(baseUrl, opts.url) + (opts.query ? "?" + stringifyQuery(opts.query) : "")); const headers = extraHeaders && opts.headers ? {...extraHeaders, ...opts.headers} : opts.headers || extraHeaders || {}; @@ -279,6 +286,14 @@ export class HttpClient implements IHttpClient { if (bearerToken && headers["Authorization"] === undefined) { headers["Authorization"] = `Bearer ${bearerToken}`; } + if (url.username || url.password) { + if (headers["Authorization"] === undefined) { + headers["Authorization"] = `Basic ${toBase64(`${url.username}:${url.password}`)}`; + } + // Remove the username and password from the URL + url.username = ""; + url.password = ""; + } this.logger?.debug("HttpClient request", {routeId}); @@ -291,7 +306,7 @@ export class HttpClient implements IHttpClient { if (!res.ok) { const errBody = await res.text(); - throw new HttpError(`${res.statusText}: ${getErrorMessage(errBody)}`, res.status, url); + throw new HttpError(`${res.statusText}: ${getErrorMessage(errBody)}`, res.status, url.toString()); } const streamTimer = this.metrics?.streamTime.startTimer({routeId}); diff --git a/packages/api/test/unit/client/httpClient.test.ts b/packages/api/test/unit/client/httpClient.test.ts index e3cfb6062f3d..be6006d9e375 100644 --- a/packages/api/test/unit/client/httpClient.test.ts +++ b/packages/api/test/unit/client/httpClient.test.ts @@ -135,6 +135,22 @@ describe("httpClient json client", () => { } }); + it("should set user credentials in URL as Authorization header", async () => { + const {baseUrl} = await getServer({ + ...testRoute, + handler: async (req) => { + expect(req.headers.authorization).to.equal("Basic dXNlcjpwYXNzd29yZA=="); + return {}; + }, + }); + const url = new URL(baseUrl); + url.username = "user"; + url.password = "password"; + const httpClient = new HttpClient({baseUrl: url.toString()}); + + await httpClient.json(testRoute); + }); + it("should handle aborting request with timeout", async () => { const {baseUrl} = await getServer({ ...testRoute, diff --git a/packages/api/test/unit/client/httpClientFallback.test.ts b/packages/api/test/unit/client/httpClientFallback.test.ts index bf929cf854a2..2c0846d00148 100644 --- a/packages/api/test/unit/client/httpClientFallback.test.ts +++ b/packages/api/test/unit/client/httpClientFallback.test.ts @@ -8,7 +8,7 @@ describe("httpClient fallback", () => { // Using fetchSub instead of actually setting up servers because there are some strange // race conditions, where the server stub doesn't count the call in time before the test is over. - const fetchStub = Sinon.stub<[string], ReturnType>(); + const fetchStub = Sinon.stub<[URL], ReturnType>(); let httpClient: HttpClient; @@ -21,10 +21,10 @@ describe("httpClient fallback", () => { const serverErrors = new Map(); // With baseURLs above find the server index associated with that URL - function getServerIndex(url: string): number { - const i = baseUrls.findIndex((baseUrl) => url.startsWith(baseUrl)); + function getServerIndex(url: URL): number { + const i = baseUrls.findIndex((baseUrl) => url.toString().startsWith(baseUrl)); if (i < 0) { - throw Error(`fetch called with unknown url ${url}`); + throw Error(`fetch called with unknown url ${url.toString()}`); } return i; } diff --git a/packages/api/test/unit/client/httpClientOptions.test.ts b/packages/api/test/unit/client/httpClientOptions.test.ts index c60c9c628cc9..1bad7c69a406 100644 --- a/packages/api/test/unit/client/httpClientOptions.test.ts +++ b/packages/api/test/unit/client/httpClientOptions.test.ts @@ -67,4 +67,20 @@ describe("HTTPClient options", () => { {baseUrl: baseUrl2, bearerToken: bearerToken2}, ]); }); + + it("Throw if empty baseUrl", () => { + expect(() => new HttpClient({baseUrl: ""})).to.throw(Error); + }); + + it("Throw if invalid baseUrl", () => { + expect(() => new HttpClient({baseUrl: "invalid"})).to.throw(Error); + }); + + it("Throw if empty value in urls option", () => { + expect(() => new HttpClient({urls: [""]})).to.throw(Error); + }); + + it("Throw if invalid value in urls option", () => { + expect(() => new HttpClient({urls: ["invalid"]})).to.throw(Error); + }); }); diff --git a/packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts b/packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts index 694848c14cf9..59454f8c2f9e 100644 --- a/packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts +++ b/packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts @@ -1,5 +1,5 @@ import {fetch} from "@lodestar/api"; -import {ErrorAborted, TimeoutError, retry} from "@lodestar/utils"; +import {ErrorAborted, TimeoutError, isValidHttpUrl, retry} from "@lodestar/utils"; import {IGauge, IHistogram} from "../../metrics/interface.js"; import {IJson, RpcPayload} from "../interface.js"; import {encodeJwtToken} from "./jwt.js"; @@ -92,6 +92,9 @@ export class JsonRpcHttpClient implements IJsonRpcHttpClient { if (!url) { throw Error(`JsonRpcHttpClient.urls[${i}] is empty or undefined: ${url}`); } + if (!isValidHttpUrl(url)) { + throw Error(`JsonRpcHttpClient.urls[${i}] must be a valid URL: ${url}`); + } } this.jwtSecret = opts?.jwtSecret; @@ -188,8 +191,6 @@ export class JsonRpcHttpClient implements IJsonRpcHttpClient { * Fetches JSON and throws detailed errors in case the HTTP request is not ok */ private async fetchJsonOneUrl(url: string, json: T, opts?: ReqOpts): Promise { - if (!url) throw Error(`Empty or undefined JSON RPC HTTP client url: ${url}`); - const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), opts?.timeout ?? this.opts?.timeout ?? REQUEST_TIMEOUT); diff --git a/packages/beacon-node/src/monitoring/service.ts b/packages/beacon-node/src/monitoring/service.ts index a6142aeac957..66ed14ee42d0 100644 --- a/packages/beacon-node/src/monitoring/service.ts +++ b/packages/beacon-node/src/monitoring/service.ts @@ -210,7 +210,7 @@ export class MonitoringService { private parseMonitoringEndpoint(endpoint: string): URL { if (!endpoint) { - throw new Error("Monitoring endpoint must be provided"); + throw new Error(`Monitoring endpoint is empty or undefined: ${endpoint}`); } try { @@ -226,7 +226,7 @@ export class MonitoringService { return url; } catch { - throw new Error("Monitoring endpoint must be a valid URL"); + throw new Error(`Monitoring endpoint must be a valid URL: ${endpoint}`); } } } diff --git a/packages/beacon-node/test/unit/monitoring/service.test.ts b/packages/beacon-node/test/unit/monitoring/service.test.ts index 53ec4df355e8..e698e6609959 100644 --- a/packages/beacon-node/test/unit/monitoring/service.test.ts +++ b/packages/beacon-node/test/unit/monitoring/service.test.ts @@ -58,14 +58,16 @@ describe("monitoring / service", () => { }); it("should throw an error if monitoring endpoint is not provided", () => { - expect(() => new MonitoringService("beacon", {endpoint: ""}, {register, logger})).to.throw( - "Monitoring endpoint must be provided" + const endpoint = ""; + expect(() => new MonitoringService("beacon", {endpoint}, {register, logger})).to.throw( + `Monitoring endpoint is empty or undefined: ${endpoint}` ); }); it("should throw an error if monitoring endpoint is not a valid URL", () => { - expect(() => new MonitoringService("beacon", {endpoint: "invalid"}, {register, logger})).to.throw( - "Monitoring endpoint must be a valid URL" + const endpoint = "invalid"; + expect(() => new MonitoringService("beacon", {endpoint}, {register, logger})).to.throw( + `Monitoring endpoint must be a valid URL: ${endpoint}` ); }); diff --git a/packages/cli/src/cmds/validator/keymanager/impl.ts b/packages/cli/src/cmds/validator/keymanager/impl.ts index 6dabec8ac862..824ed8f124e6 100644 --- a/packages/cli/src/cmds/validator/keymanager/impl.ts +++ b/packages/cli/src/cmds/validator/keymanager/impl.ts @@ -15,7 +15,8 @@ import { } from "@lodestar/api/keymanager"; import {Interchange, SignerType, Validator} from "@lodestar/validator"; import {ServerApi} from "@lodestar/api"; -import {getPubkeyHexFromKeystore, isValidatePubkeyHex, isValidHttpUrl} from "../../../util/format.js"; +import {isValidHttpUrl} from "@lodestar/utils"; +import {getPubkeyHexFromKeystore, isValidatePubkeyHex} from "../../../util/format.js"; import {parseFeeRecipient} from "../../../util/index.js"; import {DecryptKeystoresThreadPool} from "./decryptKeystores/index.js"; import {IPersistedKeysBackend} from "./interface.js"; diff --git a/packages/cli/src/cmds/validator/signers/index.ts b/packages/cli/src/cmds/validator/signers/index.ts index ea654b8ba221..a441d14099d5 100644 --- a/packages/cli/src/cmds/validator/signers/index.ts +++ b/packages/cli/src/cmds/validator/signers/index.ts @@ -4,9 +4,9 @@ import {deriveEth2ValidatorKeys, deriveKeyFromMnemonic} from "@chainsafe/bls-key import {toHexString} from "@chainsafe/ssz"; import {interopSecretKey} from "@lodestar/state-transition"; import {externalSignerGetKeys, Signer, SignerType} from "@lodestar/validator"; -import {LogLevel, Logger} from "@lodestar/utils"; +import {LogLevel, Logger, isValidHttpUrl} from "@lodestar/utils"; import {defaultNetwork, GlobalArgs} from "../../../options/index.js"; -import {assertValidPubkeysHex, isValidHttpUrl, parseRange, YargsError} from "../../../util/index.js"; +import {assertValidPubkeysHex, parseRange, YargsError} from "../../../util/index.js"; import {getAccountPaths} from "../paths.js"; import {IValidatorCliArgs} from "../options.js"; import {PersistedKeysBackend} from "../keymanager/persistedKeys.js"; diff --git a/packages/cli/src/util/format.ts b/packages/cli/src/util/format.ts index a959ec028e11..e673fa4d4408 100644 --- a/packages/cli/src/util/format.ts +++ b/packages/cli/src/util/format.ts @@ -56,17 +56,6 @@ export function assertValidPubkeysHex(pubkeysHex: string[]): void { } } -export function isValidHttpUrl(urlStr: string): boolean { - let url; - try { - url = new URL(urlStr); - } catch (_) { - return false; - } - - return url.protocol === "http:" || url.protocol === "https:"; -} - /** * Parses a file to get a list of bootnodes for a network. * Bootnodes file is expected to contain bootnode ENR's concatenated by newlines, or commas for diff --git a/packages/utils/src/base64.ts b/packages/utils/src/base64.ts new file mode 100644 index 000000000000..696a8288a928 --- /dev/null +++ b/packages/utils/src/base64.ts @@ -0,0 +1,9 @@ +const hasBufferFrom = typeof Buffer !== "undefined" && typeof Buffer.from === "function"; + +export function toBase64(value: string): string { + return hasBufferFrom ? Buffer.from(value).toString("base64") : btoa(value); +} + +export function fromBase64(value: string): string { + return hasBufferFrom ? Buffer.from(value, "base64").toString("utf8") : atob(value); +} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index a09c615fbf2b..8e622b310c31 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -1,5 +1,6 @@ export * from "./yaml/index.js"; export * from "./assert.js"; +export * from "./base64.js"; export * from "./bytes.js"; export * from "./err.js"; export * from "./errors.js"; @@ -14,6 +15,7 @@ export * from "./sleep.js"; export * from "./sort.js"; export * from "./timeout.js"; export {RecursivePartial, bnToNum} from "./types.js"; +export * from "./validation.js"; export * from "./verifyMerkleBranch.js"; export * from "./promise.js"; export * from "./waitFor.js"; diff --git a/packages/utils/src/validation.ts b/packages/utils/src/validation.ts new file mode 100644 index 000000000000..0b47a70e2340 --- /dev/null +++ b/packages/utils/src/validation.ts @@ -0,0 +1,10 @@ +export function isValidHttpUrl(urlStr: string): boolean { + let url; + try { + url = new URL(urlStr); + } catch (_) { + return false; + } + + return url.protocol === "http:" || url.protocol === "https:"; +} diff --git a/packages/utils/test/unit/base64.test.ts b/packages/utils/test/unit/base64.test.ts new file mode 100644 index 000000000000..38ccd77bafe8 --- /dev/null +++ b/packages/utils/test/unit/base64.test.ts @@ -0,0 +1,15 @@ +import "../setup.js"; +import {expect} from "chai"; +import {toBase64, fromBase64} from "../../src/index.js"; + +describe("toBase64", () => { + it("should encode UTF-8 string as base64 string", () => { + expect(toBase64("user:password")).to.be.equal("dXNlcjpwYXNzd29yZA=="); + }); +}); + +describe("fromBase64", () => { + it("should decode UTF-8 string from base64 string", () => { + expect(fromBase64("dXNlcjpwYXNzd29yZA==")).to.be.equal("user:password"); + }); +});