diff --git a/.changeset/brown-chefs-own.md b/.changeset/brown-chefs-own.md new file mode 100644 index 00000000000..cf4553b32c5 --- /dev/null +++ b/.changeset/brown-chefs-own.md @@ -0,0 +1,7 @@ +--- +"wrangler": patch +--- + +fix: prevent infinite loop when fetching a list of results + +When fetching a list of results from cloudflare APIs (e.g. when fetching a list of keys in a kv namespace), the api returns a `cursor` that a consumer should use to get the next 'page' of results. It appears this cursor can also be a blank string (while we'd only account for it to be `undefined`). By only accounting for it to be `undefined`, we were infinitely looping through the same page of results and never terminating. This PR fixes it by letting it be a blank string (and `null`, for good measure) diff --git a/packages/wrangler/src/__tests__/kv.test.ts b/packages/wrangler/src/__tests__/kv.test.ts index 310858ad0f5..b9ee7e5fafc 100644 --- a/packages/wrangler/src/__tests__/kv.test.ts +++ b/packages/wrangler/src/__tests__/kv.test.ts @@ -863,21 +863,37 @@ describe("wrangler", () => { `); }); - it("should make multiple requests for paginated results", async () => { - // Create a lot of mock keys, so that the fetch requests will be paginated - const keys: string[] = []; - for (let i = 0; i < 550; i++) { - keys.push("key-" + i); - } - // Ask for the keys in pages of size 100. - const requests = mockKeyListRequest("some-namespace-id", keys, 100); - await runWrangler( - "kv:key list --namespace-id some-namespace-id --limit 100" - ); - expect(std.err).toMatchInlineSnapshot(`""`); - expect(JSON.parse(std.out).map((k) => k.name)).toEqual(keys); - expect(requests.count).toEqual(6); - }); + // We'll run the next test with variations on the cursor + // that's returned on cloudflare's API after all results + // have been drained. + for (const blankCursorValue of [undefined, null, ""] as [ + undefined, + null, + "" + ]) { + describe(`cursor - ${blankCursorValue}`, () => { + it("should make multiple requests for paginated results", async () => { + // Create a lot of mock keys, so that the fetch requests will be paginated + const keys: string[] = []; + for (let i = 0; i < 550; i++) { + keys.push("key-" + i); + } + // Ask for the keys in pages of size 100. + const requests = mockKeyListRequest( + "some-namespace-id", + keys, + 100, + blankCursorValue + ); + await runWrangler( + "kv:key list --namespace-id some-namespace-id --limit 100" + ); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(JSON.parse(std.out).map((k) => k.name)).toEqual(keys); + expect(requests.count).toEqual(6); + }); + }); + } it("should error if a given binding name is not in the configured kv namespaces", async () => { writeWranglerConfig(); @@ -1210,7 +1226,8 @@ function writeWranglerConfig() { export function mockKeyListRequest( expectedNamespaceId: string, expectedKeys: string[], - keysPerRequest = 1000 + keysPerRequest = 1000, + blankCursorValue: "" | undefined | null = undefined ) { const requests = { count: 0 }; // See https://api.cloudflare.com/#workers-kv-namespace-list-a-namespace-s-keys @@ -1231,7 +1248,7 @@ export function mockKeyListRequest( } else { const start = parseInt(query.get("cursor") ?? "0") || 0; const end = start + keysPerRequest; - const cursor = end < expectedKeyObjects.length ? end : undefined; + const cursor = end < expectedKeyObjects.length ? end : blankCursorValue; return createFetchResult( expectedKeyObjects.slice(start, end), true, diff --git a/packages/wrangler/src/cfetch/index.ts b/packages/wrangler/src/cfetch/index.ts index 0bfb103d7c6..c5b85f6fb87 100644 --- a/packages/wrangler/src/cfetch/index.ts +++ b/packages/wrangler/src/cfetch/index.ts @@ -106,5 +106,6 @@ function throwFetchError( } function hasCursor(result_info: unknown): result_info is { cursor: string } { - return (result_info as { cursor: string } | undefined)?.cursor !== undefined; + const cursor = (result_info as { cursor: string } | undefined)?.cursor; + return cursor !== undefined && cursor !== null && cursor !== ""; }