Check for a single match in every iteration of interpolation search (#8122)

* Check for a single match in every iteration of interpolation search

* Add interpolation search fallback to binary

Author: GregBrimble

.changeset/gorgeous-papayas-trade.md

@@ -0,0 +1,5 @@
+---
+"@cloudflare/workers-shared": patch
+---
+
+fix: interpolation search method now checks for a single match at the beginning of every iteration

packages/workers-shared/asset-worker/src/assets-manifest.ts

@@ -88,24 +88,36 @@ export const interpolationSearch = (
 	if (arr.byteLength === 0) {
 		return false;
 	}
+	// top and tail the search space
 	let low = 0;
 	let high = arr.byteLength / ENTRY_SIZE - 1;
-	if (high === low) {
-		const current = new Uint8Array(arr.buffer, arr.byteOffset, PATH_HASH_SIZE);
-		if (current.byteLength !== searchValue.byteLength) {
-			throw new TypeError(
-				"Search value and current value are of different lengths"
-			);
-		}
-		const cmp = compare(current, searchValue);
-		if (cmp === 0) {
-			return new Uint8Array(arr.buffer, arr.byteOffset, ENTRY_SIZE);
-		} else {
-			return false;
-		}
-	}
 	const searchValueNumber = uint8ArrayToNumber(searchValue);
 	while (low <= high) {
+		if (low === high) {
+			// search space has been reduced to a single element
+			const current = new Uint8Array(
+				arr.buffer,
+				arr.byteOffset + low * ENTRY_SIZE,
+				PATH_HASH_SIZE
+			);
+			if (current.byteLength !== searchValue.byteLength) {
+				throw new TypeError(
+					"Search value and current value are of different lengths"
+				);
+			}
+			const cmp = compare(current, searchValue);
+			if (cmp === 0) {
+				// and it's a match!
+				return new Uint8Array(
+					arr.buffer,
+					arr.byteOffset + low * ENTRY_SIZE,
+					ENTRY_SIZE
+				);
+			} else {
+				// and it's not a match!
+				return false;
+			}
+		}
 		const lowValue = new Uint8Array(
 			arr.buffer,
 			arr.byteOffset + low * ENTRY_SIZE,
@@ -119,17 +131,19 @@ export const interpolationSearch = (
 		const lowValueNumber = uint8ArrayToNumber(lowValue);
 		const highValueNumber = uint8ArrayToNumber(highValue);
 		const denominator = highValueNumber - lowValueNumber;
-		if (denominator < 0n) {
-			return false;
+		if (denominator <= 0n) {
+			throw new TypeError("Search space is unordered");
 		}
 		const numerator = searchValueNumber - lowValueNumber;
 		if (numerator < 0n) {
+			// the lowest value in our search space is smaller than the search value (so it can't be in the search space)
 			return false;
 		}
 		const mid = Math.floor(
 			Number(BigInt(low) + (BigInt(high - low) * numerator) / denominator)
 		);
 		if (mid < low || mid > high) {
+			// our next guess is either entirely out of range of the search space or we've already searched it
 			return false;
 		}
 		const current = new Uint8Array(
@@ -150,8 +164,10 @@ export const interpolationSearch = (
 				ENTRY_SIZE
 			);
 		} else if (cmp < 0) {
+			// our estimate was too low so our search space now becomes mid+1 -> high
 			low = mid + 1;
 		} else {
+			// our estimate was too high so our search space now becomes low -> mid-1
 			high = mid - 1;
 		}
 	}

packages/workers-shared/asset-worker/src/index.ts

@@ -240,7 +240,12 @@ export default class extends WorkerEntrypoint<Env> {
 		try {
 			const assetsManifest = new AssetsManifest(this.env.ASSETS_MANIFEST);
 			if (searchMethod === "interpolation") {
-				return await assetsManifest.getWithInterpolationSearch(pathname);
+				try {
+					return await assetsManifest.getWithInterpolationSearch(pathname);
+				} catch (e) {
+					analytics.setData({ manifestReadMethod: "binary-fallback" });
+					return await assetsManifest.getWithBinarySearch(pathname);
+				}
 			} else {
 				return await assetsManifest.getWithBinarySearch(pathname);
 			}

packages/workers-shared/asset-worker/tests/assets-manifest.test.ts

@@ -236,6 +236,18 @@ describe("search methods", async () => {
 				expect(foundEntry).toBe(false);
 			}
 		});
+
+		it("throws an error if the search value is longer than the current value", async () => {
+			const { manifest } = await makeManifestOfLength(10);
+			expect(() => {
+				binarySearch(
+					new Uint8Array(manifest, HEADER_SIZE),
+					new Uint8Array(PATH_HASH_SIZE + 1).fill(127)
+				);
+			}).toThrowErrorMatchingInlineSnapshot(
+				`[TypeError: Search value and current value are of different lengths]`
+			);
+		});
 	});
 
 	describe("interpolation search", () => {
@@ -339,6 +351,57 @@ describe("search methods", async () => {
 			}
 		});
 
+		it("throws an error if the search value is longer than the current value", async () => {
+			const { manifest } = await makeManifestOfLength(10);
+			expect(() => {
+				interpolationSearch(
+					new Uint8Array(manifest, HEADER_SIZE),
+					new Uint8Array(PATH_HASH_SIZE + 1).fill(127)
+				);
+			}).toThrowErrorMatchingInlineSnapshot(
+				`[TypeError: Search value and current value are of different lengths]`
+			);
+		});
+
+		it("throws an error if the search space is out of order", async () => {
+			const smallEntry = {
+				path: "/small",
+				pathHashBytes: new Uint8Array(PATH_HASH_SIZE),
+				contentHash: "00000000000000000000000000000000",
+			};
+			const largeEntry = {
+				path: "/large",
+				pathHashBytes: new Uint8Array(PATH_HASH_SIZE).fill(255),
+				contentHash: "ffffffffffffffffffffffffffffffff",
+			};
+			const entries = [smallEntry, largeEntry];
+			entries.sort((a, b) => 0 - compare(a.pathHashBytes, b.pathHashBytes)); // inverted order!
+
+			const assetManifestBytes = new Uint8Array(
+				HEADER_SIZE + entries.length * ENTRY_SIZE
+			);
+
+			for (const [i, { pathHashBytes, contentHash }] of entries.entries()) {
+				const contentHashBytes = hexToBytes(contentHash);
+				const entryOffset = HEADER_SIZE + i * ENTRY_SIZE;
+
+				assetManifestBytes.set(pathHashBytes, entryOffset + PATH_HASH_OFFSET);
+				assetManifestBytes.set(
+					contentHashBytes,
+					entryOffset + CONTENT_HASH_OFFSET
+				);
+			}
+
+			expect(() => {
+				interpolationSearch(
+					new Uint8Array(assetManifestBytes.buffer, HEADER_SIZE),
+					smallEntry.pathHashBytes
+				);
+			}).toThrowErrorMatchingInlineSnapshot(
+				`[TypeError: Search space is unordered]`
+			);
+		});
+
 		it("doesn't throw 'RangeError: Division by zero' with extreme manifests", async () => {
 			const smallEntry = {
 				path: "/small",