fix: detect explicit port from URL string; add Vitest tests
The previous fix fell back to the scheme's default port (443/80) when url.port was empty, but url.port is empty for *both* 'wss://host:443' and 'wss://host' — causing the validation to wrongly accept a URL with no port at all. Fix: inspect the raw authority segment of the URL string to check whether a ':port' component is actually present, regardless of whether that port is the scheme default. Add gateway-form.test.ts covering: - explicit non-default ports (accepted) - explicit default ports :443 / :80 (accepted — regression case) - missing port (rejected) - wrong scheme (rejected) - invalid URL (rejected) - whitespace trimming Closes #148
This commit is contained in:
committed by
Abhimanyu Saharan
parent
fae681747d
commit
c13915bbdf
@@ -13,14 +13,66 @@ const mockedGatewaysStatusApiV1GatewaysStatusGet = vi.mocked(
|
||||
);
|
||||
|
||||
describe("validateGatewayUrl", () => {
|
||||
it("requires ws/wss with an explicit port", () => {
|
||||
expect(validateGatewayUrl("https://gateway.example")).toBe(
|
||||
"Gateway URL must start with ws:// or wss://.",
|
||||
);
|
||||
expect(validateGatewayUrl("ws://gateway.example")).toBe(
|
||||
it("accepts ws:// with explicit non-default port", () => {
|
||||
expect(validateGatewayUrl("ws://localhost:18789")).toBeNull();
|
||||
});
|
||||
|
||||
it("accepts wss:// with explicit non-default port", () => {
|
||||
expect(validateGatewayUrl("wss://gateway.example.com:8443")).toBeNull();
|
||||
});
|
||||
|
||||
it("accepts wss:// with explicit default port 443", () => {
|
||||
expect(validateGatewayUrl("wss://devbot.tailcc2080.ts.net:443")).toBeNull();
|
||||
});
|
||||
|
||||
it("accepts ws:// with explicit default port 80", () => {
|
||||
expect(validateGatewayUrl("ws://localhost:80")).toBeNull();
|
||||
});
|
||||
|
||||
it("accepts URLs with a path after the port", () => {
|
||||
expect(validateGatewayUrl("wss://host.example.com:443/gateway")).toBeNull();
|
||||
});
|
||||
|
||||
it("trims surrounding whitespace before validating", () => {
|
||||
expect(validateGatewayUrl(" wss://host:443 ")).toBeNull();
|
||||
});
|
||||
|
||||
it("rejects empty string", () => {
|
||||
expect(validateGatewayUrl("")).toBe("Gateway URL is required.");
|
||||
});
|
||||
|
||||
it("rejects wss:// with no port at all", () => {
|
||||
expect(validateGatewayUrl("wss://gateway.example.com")).toBe(
|
||||
"Gateway URL must include an explicit port.",
|
||||
);
|
||||
expect(validateGatewayUrl("ws://gateway.example:18789")).toBeNull();
|
||||
});
|
||||
|
||||
it("rejects ws:// with no port at all", () => {
|
||||
expect(validateGatewayUrl("ws://localhost")).toBe(
|
||||
"Gateway URL must include an explicit port.",
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects https:// scheme", () => {
|
||||
expect(validateGatewayUrl("https://gateway.example.com:443")).toBe(
|
||||
"Gateway URL must start with ws:// or wss://.",
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects http:// scheme", () => {
|
||||
expect(validateGatewayUrl("http://localhost:8080")).toBe(
|
||||
"Gateway URL must start with ws:// or wss://.",
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects completely invalid URL", () => {
|
||||
expect(validateGatewayUrl("not-a-url")).toBe(
|
||||
"Enter a valid gateway URL including port.",
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects URL with only whitespace", () => {
|
||||
expect(validateGatewayUrl(" ")).toBe("Gateway URL is required.");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -4,6 +4,30 @@ export const DEFAULT_WORKSPACE_ROOT = "~/.openclaw";
|
||||
|
||||
export type GatewayCheckStatus = "idle" | "checking" | "success" | "error";
|
||||
|
||||
/**
|
||||
* Returns true only when the URL string contains an explicit ":port" segment.
|
||||
*
|
||||
* JavaScript's URL API sets `.port` to "" for *both* an omitted port and a
|
||||
* port that equals the scheme's default (e.g. 443 for wss:). We therefore
|
||||
* inspect the raw host+port token from the URL string instead.
|
||||
*/
|
||||
function hasExplicitPort(urlString: string): boolean {
|
||||
try {
|
||||
const { hostname } = new URL(urlString);
|
||||
// Extract the authority portion (between // and the first / ? or #)
|
||||
const withoutScheme = urlString.slice(urlString.indexOf("//") + 2);
|
||||
const authority = withoutScheme.split(/[/?#]/)[0];
|
||||
// authority is either "host", "host:port", or "[ipv6]:port"
|
||||
// Remove a leading IPv6 bracket group before checking for ":"
|
||||
const withoutIPv6 = authority.startsWith("[")
|
||||
? authority.slice(authority.indexOf("]") + 1)
|
||||
: authority.slice(hostname.length);
|
||||
return withoutIPv6.startsWith(":") && /^:\d+$/.test(withoutIPv6);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export const validateGatewayUrl = (value: string) => {
|
||||
const trimmed = value.trim();
|
||||
if (!trimmed) return "Gateway URL is required.";
|
||||
@@ -12,10 +36,7 @@ export const validateGatewayUrl = (value: string) => {
|
||||
if (url.protocol !== "ws:" && url.protocol !== "wss:") {
|
||||
return "Gateway URL must start with ws:// or wss://.";
|
||||
}
|
||||
// url.port is empty for default ports (80 for ws:, 443 for wss:) — allow those
|
||||
const defaultPorts: Record<string, string> = { "ws:": "80", "wss:": "443" };
|
||||
const effectivePort = url.port || defaultPorts[url.protocol] || "";
|
||||
if (!effectivePort) {
|
||||
if (!hasExplicitPort(trimmed)) {
|
||||
return "Gateway URL must include an explicit port.";
|
||||
}
|
||||
return null;
|
||||
|
||||
Reference in New Issue
Block a user