Skip to content

Commit

Permalink
Require protocol to be set in endpoint URLs
Browse files Browse the repository at this point in the history
  • Loading branch information
webmaster128 committed Dec 19, 2023
1 parent 7ee2aa0 commit 852093c
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 39 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ and this project adheres to

[#1522]: https://github.com/cosmos/cosmjs/pull/1522

### Changed

- @cosmjs/tendermint-rpc: Require protocol to be set in endpoint URLs (https://,
http://, wss:// or ws://). Otherwise an error is raised instead of falling
back to ws://. ([#1527])

[#1527]: https://github.com/cosmos/cosmjs/pull/1527

## [0.32.1] - 2023-12-04

### Fixed
Expand Down
12 changes: 2 additions & 10 deletions packages/tendermint-rpc/src/comet38/comet38client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -885,14 +885,6 @@ describe("Comet38Client", () => {
it("can connect to a given url", async () => {
pendingWithoutTendermint();

// default connection
{
const client = await Comet38Client.connect(url);
const info = await client.abciInfo();
expect(info).toBeTruthy();
client.disconnect();
}

// http connection
{
const client = await Comet38Client.connect("http://" + url);
Expand All @@ -911,13 +903,13 @@ describe("Comet38Client", () => {
});

describe("With HttpClient", () => {
defaultTestSuite(() => new HttpClient(url), expected);
defaultTestSuite(() => new HttpClient("http://" + url), expected);
});

describe("With WebsocketClient", () => {
// don't print out WebSocket errors if marked pending
const onError = process.env.TENDERMINT_ENABLED ? console.error : () => 0;
const factory = (): WebsocketClient => new WebsocketClient(url, onError);
const factory = (): WebsocketClient => new WebsocketClient("ws://" + url, onError);
defaultTestSuite(factory, expected);
websocketTestSuite(factory, expected);
});
Expand Down
6 changes: 4 additions & 2 deletions packages/tendermint-rpc/src/rpcclients/httpbatchclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ export class HttpBatchClient implements RpcClient {
dispatchInterval: options.dispatchInterval ?? defaultHttpBatchClientOptions.dispatchInterval,
};
if (typeof endpoint === "string") {
// accept host.name:port and assume http protocol
this.url = hasProtocol(endpoint) ? endpoint : "http://" + endpoint;
if (!hasProtocol(endpoint)) {
throw new Error("Endpoint URL is missing a protocol. Expected 'https://' or 'http://'.");
}
this.url = endpoint;
} else {
this.url = endpoint.url;
this.headers = endpoint.headers;
Expand Down
6 changes: 4 additions & 2 deletions packages/tendermint-rpc/src/rpcclients/httpclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ export class HttpClient implements RpcClient {

public constructor(endpoint: string | HttpEndpoint) {
if (typeof endpoint === "string") {
// accept host.name:port and assume http protocol
this.url = hasProtocol(endpoint) ? endpoint : "http://" + endpoint;
if (!hasProtocol(endpoint)) {
throw new Error("Endpoint URL is missing a protocol. Expected 'https://' or 'http://'.");
}
this.url = endpoint;
} else {
this.url = endpoint.url;
this.headers = endpoint.headers;
Expand Down
29 changes: 28 additions & 1 deletion packages/tendermint-rpc/src/rpcclients/rpcclient.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createJsonRpcRequest } from "../jsonrpc";
import { defaultInstance } from "../testutil.spec";
import { HttpClient } from "./httpclient";
import { instanceOfRpcStreamingClient } from "./rpcclient";
import { hasProtocol, instanceOfRpcStreamingClient } from "./rpcclient";
import { WebsocketClient } from "./websocketclient";

function pendingWithoutTendermint(): void {
Expand Down Expand Up @@ -41,3 +41,30 @@ describe("RpcClient", () => {
wsClient.disconnect();
});
});

describe("hasProtocol", () => {
it("works", () => {
expect(hasProtocol("https://my.rpc.net")).toEqual(true);
expect(hasProtocol("https://my.rpc.net:443")).toEqual(true);
expect(hasProtocol("https://my.rpc.net:443/somewhere")).toEqual(true);
expect(hasProtocol("http://my.rpc.net")).toEqual(true);
expect(hasProtocol("http://my.rpc.net:443")).toEqual(true);
expect(hasProtocol("http://my.rpc.net:443/somewhere")).toEqual(true);
expect(hasProtocol("wss://my.rpc.net")).toEqual(true);
expect(hasProtocol("wss://my.rpc.net:443")).toEqual(true);
expect(hasProtocol("wss://my.rpc.net:443/somewhere")).toEqual(true);
expect(hasProtocol("ws://my.rpc.net")).toEqual(true);
expect(hasProtocol("ws://my.rpc.net:443")).toEqual(true);
expect(hasProtocol("ws://my.rpc.net:443/somewhere")).toEqual(true);
expect(hasProtocol("ws://my.rpc.net:443/somewhere")).toEqual(true);
expect(hasProtocol("file:///Users/joe/Library/CloudStorage/Notes.pdf")).toEqual(true);

// Protocols without double slash is not what we want to see here
expect(hasProtocol("tel:+1-201-555-0123")).toEqual(false);

// No protocols
expect(hasProtocol("")).toEqual(false);
expect(hasProtocol(" ")).toEqual(false);
expect(hasProtocol("/")).toEqual(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ function pendingWithoutTendermint(): void {
}

describe("WebsocketClient", () => {
const { blockTime, url: tendermintUrl } = defaultInstance;
const { blockTime, url } = defaultInstance;
const tendermintUrl = "ws://" + url;

it("can make a simple call", async () => {
pendingWithoutTendermint();
Expand Down
8 changes: 5 additions & 3 deletions packages/tendermint-rpc/src/rpcclients/websocketclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,13 @@ export class WebsocketClient implements RpcStreamingClient {
private readonly subscriptionStreams = new Map<string, Stream<SubscriptionEvent>>();

public constructor(baseUrl: string, onError: (err: any) => void = defaultErrorHandler) {
// accept host.name:port and assume ws protocol
if (!hasProtocol(baseUrl)) {
throw new Error("Base URL is missing a protocol. Expected 'ws://' or 'wss://'.");
}

// make sure we don't end up with ...//websocket
const path = baseUrl.endsWith("/") ? "websocket" : "/websocket";
const cleanBaseUrl = hasProtocol(baseUrl) ? baseUrl : "ws://" + baseUrl;
this.url = cleanBaseUrl + path;
this.url = baseUrl + path;

this.socket = new ReconnectingSocket(this.url);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -876,14 +876,6 @@ describe("Tendermint34Client", () => {
it("can connect to a given url", async () => {
pendingWithoutTendermint();

// default connection
{
const client = await Tendermint34Client.connect(url);
const info = await client.abciInfo();
expect(info).toBeTruthy();
client.disconnect();
}

// http connection
{
const client = await Tendermint34Client.connect("http://" + url);
Expand All @@ -902,13 +894,13 @@ describe("Tendermint34Client", () => {
});

describe("With HttpClient", () => {
defaultTestSuite(() => new HttpClient(url), expected);
defaultTestSuite(() => new HttpClient("http://" + url), expected);
});

describe("With WebsocketClient", () => {
// don't print out WebSocket errors if marked pending
const onError = process.env.TENDERMINT_ENABLED ? console.error : () => 0;
const factory = (): WebsocketClient => new WebsocketClient(url, onError);
const factory = (): WebsocketClient => new WebsocketClient("ws://" + url, onError);
defaultTestSuite(factory, expected);
websocketTestSuite(factory, expected);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -885,14 +885,6 @@ describe("Tendermint37Client", () => {
it("can connect to a given url", async () => {
pendingWithoutTendermint();

// default connection
{
const client = await Tendermint37Client.connect(url);
const info = await client.abciInfo();
expect(info).toBeTruthy();
client.disconnect();
}

// http connection
{
const client = await Tendermint37Client.connect("http://" + url);
Expand All @@ -911,13 +903,13 @@ describe("Tendermint37Client", () => {
});

describe("With HttpClient", () => {
defaultTestSuite(() => new HttpClient(url), expected);
defaultTestSuite(() => new HttpClient("http://" + url), expected);
});

describe("With WebsocketClient", () => {
// don't print out WebSocket errors if marked pending
const onError = process.env.TENDERMINT_ENABLED ? console.error : () => 0;
const factory = (): WebsocketClient => new WebsocketClient(url, onError);
const factory = (): WebsocketClient => new WebsocketClient("ws://" + url, onError);
defaultTestSuite(factory, expected);
websocketTestSuite(factory, expected);
});
Expand Down
1 change: 1 addition & 0 deletions packages/tendermint-rpc/src/testutil.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export interface ExpectedValues {
}

export interface TendermintInstance {
/** URL without protocol. Protocol will be added in tests. */
readonly url: string;
readonly version: string;
/** rough block time in ms */
Expand Down

0 comments on commit 852093c

Please sign in to comment.