Skip to content

Commit

Permalink
fix: pass user credentials to events http client (#6179)
Browse files Browse the repository at this point in the history
  • Loading branch information
nflaig committed Dec 12, 2023
1 parent 3a43dcb commit 5b38552
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 18 deletions.
7 changes: 3 additions & 4 deletions packages/api/src/beacon/client/events.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {ChainForkConfig} from "@lodestar/config";
import {Api, BeaconEvent, routesData, getEventSerdes} from "../routes/events.js";
import {stringifyQuery} from "../../utils/client/format.js";
import {stringifyQuery, urlJoin} from "../../utils/client/format.js";
import {getEventSource} from "../../utils/client/eventSource.js";
import {HttpStatusCode} from "../../utils/client/httpStatusCode.js";

Expand All @@ -13,8 +13,7 @@ export function getClient(config: ChainForkConfig, baseUrl: string): Api {
return {
eventstream: async (topics, signal, onEvent) => {
const query = stringifyQuery({topics});
// TODO: Use a proper URL formatter
const url = `${baseUrl}${routesData.eventstream.url}?${query}`;
const url = `${urlJoin(baseUrl, routesData.eventstream.url)}?${query}`;
// eslint-disable-next-line @typescript-eslint/naming-convention
const EventSource = await getEventSource();
const eventSource = new EventSource(url);
Expand Down Expand Up @@ -58,4 +57,4 @@ export function getClient(config: ChainForkConfig, baseUrl: string): Api {
}

// https://github.com/EventSource/eventsource/blob/82e034389bd2c08d532c63172b8e858c5b185338/lib/eventsource.js#L143
type EventSourceError = {status: number; message: string};
type EventSourceError = {status?: number; message: string};
3 changes: 1 addition & 2 deletions packages/api/src/utils/client/httpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ export class HttpClient implements IHttpClient {
private readonly urlsScore: number[];

get baseUrl(): string {
// Don't leak username/password to caller
return new URL(this.urlsOpts[0].baseUrl).origin;
return this.urlsOpts[0].baseUrl;
}

/**
Expand Down
10 changes: 0 additions & 10 deletions packages/api/test/unit/client/httpClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,6 @@ describe("httpClient json client", () => {
await httpClient.json(testRoute);
});

it("should not leak user credentials in baseUrl getter", () => {
const url = new URL("http://localhost");
url.username = "user";
url.password = "password";
const httpClient = new HttpClient({baseUrl: url.toString()});

expect(httpClient.baseUrl.includes(url.username)).toBe(false);
expect(httpClient.baseUrl.includes(url.password)).toBe(false);
});

it("should handle aborting request with timeout", async () => {
const {baseUrl} = await getServer({
...testRoute,
Expand Down
6 changes: 4 additions & 2 deletions packages/validator/src/services/chainHeaderTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ export class ChainHeaderTracker {
) {}

start(signal: AbortSignal): void {
void this.api.events.eventstream([EventType.head], signal, this.onHeadUpdate);
this.logger.verbose("Subscribed to head event");
this.logger.verbose("Subscribing to head event");
this.api.events
.eventstream([EventType.head], signal, this.onHeadUpdate)
.catch((e) => this.logger.error("Failed to subscribe to head event", {}, e));
}

getCurrentChainHead(slot: Slot): Root | null {
Expand Down

0 comments on commit 5b38552

Please sign in to comment.