Improve URL validation and fail on startup

ChainSafe · Aug 14, 2023 · 9aa412b · 9aa412b
1 parent 7aa238e
commit 9aa412b
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 5 deletions.
diff --git 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);

diff --git 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
@@ -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<R, T = unknown>(url: string, json: T, opts?: ReqOpts): Promise<R> {
-    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);