From a9a80de3789bc4b0c1e9e5ab9087dc7e01d3889f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 17:43:30 +0000 Subject: [PATCH] fix(connector-rest): a baseUrl that carries a query string sends each action to its own path buildUrl parses the base and lifts its query off before joining the path, then puts the base parameters back ahead of the action's own (inline path query, then `query`); on a clash the action's value wins. A base without a query sends the same URL as before, and a path still only lands under the base's origin. The dogfood carry-forward pin gains a path-bearing dispatch. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5oht6tMAsnKGYsgbiT --- .changeset/28736-rest-base-url-query.md | 12 +++ .../connector-rest/src/rest-connector.test.ts | 75 +++++++++++++++++++ .../connector-rest/src/rest-connector.ts | 32 +++++++- ...or-entry-and-url-hardening.dogfood.test.ts | 4 +- 4 files changed, 118 insertions(+), 5 deletions(-) create mode 100644 .changeset/28736-rest-base-url-query.md diff --git a/.changeset/28736-rest-base-url-query.md b/.changeset/28736-rest-base-url-query.md new file mode 100644 index 00000000000..de23e62bf18 --- /dev/null +++ b/.changeset/28736-rest-base-url-query.md @@ -0,0 +1,12 @@ +--- +'@objectstack/connector-rest': patch +--- + +fix(connector-rest): a `baseUrl` that carries a query string sends each action to its own path + +Clause-②: no +Metadata-protocol: none + +A REST connector whose `baseUrl` carries a fixed query parameter (`https://api.example.com/v1?region=eu`, `?api-version=2024-01`) used to put every action's path inside that query: the base and the path were joined as text, so `/items` became `…/v1?region=eu/items` and the request went to `/v1`. The request URL is now built from the parsed base: the path joins the base's path with one slash between, the base's query parameters stay the query, and the action's own parameters (written inline in `path`, or in `query`, an api-key `paramName` included) come after them. **On a clash the action's value wins**: it is the more specific intent, and refusing would break a working action the day its base gains the parameter. + +A base without a query sends exactly the URL it sent before, and a path still only ever lands under the base's origin (a path like `//other.example` or an absolute URL stays a path segment). A base fragment keeps the place the text join gave it. diff --git a/packages/connectors/connector-rest/src/rest-connector.test.ts b/packages/connectors/connector-rest/src/rest-connector.test.ts index ccb1fcf3b50..a82f35b2ca5 100644 --- a/packages/connectors/connector-rest/src/rest-connector.test.ts +++ b/packages/connectors/connector-rest/src/rest-connector.test.ts @@ -89,6 +89,81 @@ describe('createRestConnector — request action', () => { }); }); +// ─── request URL: a base URL that carries its own query ────────────── + +describe('createRestConnector — base URL with a query string', () => { + /** The URL one `request` dispatch sent, for a base URL and an action input. */ + async function sentUrl(baseUrl: string, input: Record, auth?: Parameters[0]['auth']) { + const { impl, calls } = stubFetch(); + const { handlers } = createRestConnector({ baseUrl, auth, fetchImpl: impl }); + await handlers.request(input, {}); + expect(calls).toHaveLength(1); + return calls[0].url; + } + + it.each([ + ['a path', 'https://api.example.com/v1?region=eu', '/items', 'https://api.example.com/v1/items?region=eu'], + ['a path without its leading slash', 'https://api.example.com/v1?region=eu', 'items', 'https://api.example.com/v1/items?region=eu'], + ['a base with a trailing slash', 'https://api.example.com/v1/?region=eu', '/items', 'https://api.example.com/v1/items?region=eu'], + ['a base with no path', 'https://api.example.com?api-version=2024-01', '/items', 'https://api.example.com/items?api-version=2024-01'], + ['several base parameters', 'https://api.example.com/v1?region=eu&api-version=2024-01', '/items/7', 'https://api.example.com/v1/items/7?region=eu&api-version=2024-01'], + ['an empty path', 'https://api.example.com/v1?region=eu', '', 'https://api.example.com/v1?region=eu'], + ])('%s: the path joins the base path and the base query stays the query', async (_label, baseUrl, path, expected) => { + expect(await sentUrl(baseUrl, { path })).toBe(expected); + }); + + it('merges the action query after the base query', async () => { + expect(await sentUrl('https://api.example.com/v1?region=eu', { path: '/items', query: { page: 2 } })) + .toBe('https://api.example.com/v1/items?region=eu&page=2'); + }); + + it('a clash with the base parameter: the action query wins', async () => { + // `query` replaces the value where the parameter already stands. + expect(await sentUrl('https://api.example.com/v1?region=eu&api-version=1', { path: '/items', query: { region: 'us' } })) + .toBe('https://api.example.com/v1/items?region=us&api-version=1'); + }); + + it('a clash with the base parameter: a query written inline in the path wins too', async () => { + expect(await sentUrl('https://api.example.com/v1?region=eu&api-version=1', { path: '/items?region=us&page=2' })) + .toBe('https://api.example.com/v1/items?api-version=1®ion=us&page=2'); + }); + + it('an api-key query parameter rides beside the base query', async () => { + expect(await sentUrl('https://api.example.com/v1?region=eu', { path: '/me' }, { type: 'api-key', key: 'k-1', paramName: 'api_key' })) + .toBe('https://api.example.com/v1/me?region=eu&api_key=k-1'); + }); + + it('a base fragment keeps the place the text join gave it', async () => { + expect(await sentUrl('https://api.example.com/v1?region=eu#frag', { path: '/items' })) + .toBe('https://api.example.com/v1?region=eu#frag/items'); + expect(await sentUrl('https://api.example.com/v1#frag', { path: '/items' })) + .toBe('https://api.example.com/v1#frag/items'); + }); + + it.each([ + ['no query', 'https://api.example.com/v1'], + ['a query', 'https://api.example.com/v1?region=eu'], + ])('a path never leaves the base origin (base with %s)', async (_label, baseUrl) => { + for (const path of ['//evil.example/x', 'https://evil.example/x', '\\\\evil.example/x', '/@evil.example/x', '/../../x']) { + const sent = new URL(await sentUrl(baseUrl, { path })); + expect(sent.origin, path).toBe('https://api.example.com'); + expect(sent.username, path).toBe(''); + expect(sent.search, path).toBe(new URL(baseUrl).search); + } + }); + + it('a base without a query sends the URL the text join sent', async () => { + const cases: Array<[string, string, string]> = [ + ['https://api.example.com/v1', '/items?page=2', 'https://api.example.com/v1/items?page=2'], + ['https://api.example.com/v1/', 'items', 'https://api.example.com/v1/items'], + ['https://api.example.com/v1', '//evil.example/x', 'https://api.example.com/v1//evil.example/x'], + ['https://api.example.com/v1', 'https://evil.example/x', 'https://api.example.com/v1/https://evil.example/x'], + ['https://api.example.com', '', 'https://api.example.com/'], + ]; + for (const [baseUrl, path, expected] of cases) expect(await sentUrl(baseUrl, { path }), `${baseUrl} + ${path}`).toBe(expected); + }); +}); + // ─── auth injection ────────────────────────────────────────────────── describe('createRestConnector — static auth', () => { diff --git a/packages/connectors/connector-rest/src/rest-connector.ts b/packages/connectors/connector-rest/src/rest-connector.ts index a0721192b9a..9180629bef4 100644 --- a/packages/connectors/connector-rest/src/rest-connector.ts +++ b/packages/connectors/connector-rest/src/rest-connector.ts @@ -100,11 +100,37 @@ export interface RestConnectorBundle { >; } -/** Build the request URL from base + path + query, encoding query params. */ +/** + * Build the request URL from base + path + query, encoding query params. + * + * The base is parsed first and its own query is lifted off before the path is + * joined: a text join of `https://api.example.com/v1?region=eu` and `/items` + * reads `…/v1?region=eu/items`, which sends every path-bearing action to `/v1` + * with the path inside the `region` value. The path is then joined onto the + * base's path with one slash between, exactly as before, so a base without a + * query yields the same URL byte for byte, and a path can only ever land under + * the base's origin: it is appended after the base's path, never resolved + * against it, so `//other.example` or an absolute URL stays a path segment. + * + * The base's query parameters come back first, then the action's own: those + * written inline in `path`, then `query` (auth's query parameter included). + * **On a clash the action's value wins** — it is the more specific intent, and + * refusing would break a working action the day its base gains the parameter. + * A base fragment is left where the text join put it, as before. + */ function buildUrl(baseUrl: string, path: string, query?: RestRequestInput['query']): string { - const base = baseUrl.replace(/\/+$/, ''); + const base = new URL(baseUrl); + const baseParams = [...base.searchParams]; + base.search = ''; const suffix = path ? (path.startsWith('/') ? path : `/${path}`) : ''; - const url = new URL(base + suffix); + const url = new URL(base.toString().replace(/\/+$/, '') + suffix); + if (baseParams.length > 0) { + const own = [...url.searchParams]; + const ownKeys = new Set(own.map(([key]) => key)); + const merged = new URLSearchParams(baseParams.filter(([key]) => !ownKeys.has(key))); + for (const [key, value] of own) merged.append(key, value); + url.search = merged.toString(); + } if (query) { for (const [key, value] of Object.entries(query)) { if (value !== undefined && value !== null) url.searchParams.set(key, String(value)); diff --git a/packages/qa/dogfood/test/connector-meta-door-entry-and-url-hardening.dogfood.test.ts b/packages/qa/dogfood/test/connector-meta-door-entry-and-url-hardening.dogfood.test.ts index 870594c4c02..11ac81af6d9 100644 --- a/packages/qa/dogfood/test/connector-meta-door-entry-and-url-hardening.dogfood.test.ts +++ b/packages/qa/dogfood/test/connector-meta-door-entry-and-url-hardening.dogfood.test.ts @@ -267,9 +267,9 @@ describe('connector metadata doors: undeclared entry keys refused; URL credentia expect(atRest.providerConfig.baseUrl).toBe(carriedUrl); await waitReady(CARRIED); - // No request path: the rest provider appends a path to the base URL as - // text, so the base URL's own query stays the query only without one. + // The base URL's own query stays the query with and without a request path. expect(await dispatch(CARRIED, '')).toBe(`/v1?api_key=${PLACEHOLDERS.carried}®ion=eu`); + expect(await dispatch(CARRIED, '/probe')).toBe(`/v1/probe?api_key=${PLACEHOLDERS.carried}®ion=eu`); }); }); });