Federation domain validation (#1393)

* move domain validation to FederationServer.createForDomain and constructor

* add tests for domain validation

Author: Ryang-21

CHANGELOG.md

@@ -15,7 +15,7 @@ A breaking change will get clearly marked in this log.
 * Fixed bigint-to-U32/I32 conversion in `Spec` using `Number(val)` instead of `val as number` (a no-op for bigints) ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
 * Fixed missing template literal `$` in two `Spec` error messages that were not interpolated ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
 * WASM custom section parser: when a section was skipped (invalid name length), the offset was not advanced, causing an infinite loop or incorrect parsing of subsequent sections ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
-* `FederationServer.resolve` now validates domains per RFC 1035, rejecting malformed domains. Port numbers are also accepted in the domain ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
+* `FederationServer.createForDomain` and the `FederationServer` constructor now validate domains per RFC 1035, rejecting malformed domains; port numbers are also accepted. This may be breaking for callers that previously omitted `domain` or passed an invalid domain ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
 * `FederationServer` URL mutation: `resolveAddress`, `resolveAccountId`, and `resolveTransactionId` mutated the shared `serverURL` by appending query params on each call. Fixed by cloning the URL before modifying ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
 * `CallBuilder.stream()` URL mutation: `stream()` mutated the shared `this.url` by adding query params, corrupting the builder for subsequent calls. Fixed by cloning the URL ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
 * `AssembledTransaction` restore path: when `buildWithOp` was used and automatic state restoration was needed, the rebuild incorrectly reconstructed the operation via `contract.call()` instead of reusing the original operation ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).

src/federation/server.ts

@@ -7,6 +7,7 @@ import { Resolver } from "../stellartoml";
 
 import { Api } from "./api";
 import { httpClient } from "../http-client";
+import { validateDomain } from "./utils";
 
 /** @module Federation */
 
@@ -91,16 +92,6 @@ export class FederationServer {
       return Promise.reject(new Error("Invalid Stellar address"));
     }
 
-    // Validate domain per RFC 1035 (as required by SEP-0002): each dot-separated
-    // label must start with a letter, end with a letter or digit, and contain only
-    // letters, digits, or hyphens.
-    if (
-      !/^(?:[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?\.)*[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?(?::\d+)?$/.test(
-        domain,
-      )
-    ) {
-      return Promise.reject(new Error("Invalid domain in Stellar address"));
-    }
     const federationServer = await FederationServer.createForDomain(
       domain,
       opts,
@@ -135,6 +126,7 @@ export class FederationServer {
     domain: string,
     opts: Api.Options = {},
   ): Promise<FederationServer> {
+    validateDomain(domain);
     const tomlObject = await Resolver.resolve(domain, opts);
     if (!tomlObject.FEDERATION_SERVER) {
       return Promise.reject(
@@ -149,9 +141,9 @@ export class FederationServer {
     domain: string,
     opts: Api.Options = {},
   ) {
-    // TODO `domain` regexp
     this.serverURL = URI(serverURL);
     this.domain = domain;
+    validateDomain(domain);
 
     const allowHttp =
       typeof opts.allowHttp === "undefined"

src/federation/utils.ts

@@ -0,0 +1,14 @@
+export function validateDomain(domain: string): void {
+  // Validate domain per RFC 1035 with an optional port (as required by SEP-0002): each dot-separated
+  // label must start with a letter, end with a letter or digit, and contain only
+  // letters, digits, or hyphens.
+  if (
+    !/^(?:[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?\.)*[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?(?::\d+)?$/.test(
+      domain,
+    )
+  ) {
+    throw new Error(
+      "The provided domain is invalid. Ensure that the domain adheres to RFC 1035",
+    );
+  }
+}

test/unit/federation_server.test.ts

@@ -27,6 +27,12 @@ describe("federation-server.js tests", () => {
       ).toThrow(/Cannot connect to insecure federation server/);
     });
 
+    it("throws error for invalid domain", () => {
+      expect(
+        () => new Server("https://acme.com:1337/federation", "-stellar.org"),
+      ).toThrow(/The provided domain is invalid/);
+    });
+
     it("allow insecure server when opts.allowHttp flag is set", () => {
       expect(
         () =>
@@ -170,6 +176,13 @@ FEDERATION_SERVER="https://api.stellar.org/federation"
       expect(federationServer["domain"]).toEqual("acme.com");
     });
 
+    it("fails for invalid domain before requesting stellar.toml", async () => {
+      await expect(Server.createForDomain("-acme.com")).rejects.toThrow(
+        /The provided domain is invalid/,
+      );
+      expect(mockHttpClient).not.toHaveBeenCalled();
+    });
+
     it("fails when stellar.toml does not contain federation server info", async () => {
       mockHttpClient.mockImplementation((url: string) => {
         if (url.includes("https://acme.com/.well-known/stellar.toml")) {