diff --git a/README.md b/README.md index efb7178..1a0e891 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,19 @@ Cloud storage URLs in virtual chunk references are automatically translated: - `abfs://container@account.dfs.core.windows.net/path` → `https://account.blob.core.windows.net/container/path` +For S3, addressing follows the repo's virtual-chunk-container config (region, +endpoint, path-style), mirroring the Rust implementation. Buckets whose name +contains a dot must use path-style addressing; when the container config records +a region (repos written by Icechunk do), reads go straight to that regional +endpoint — which serves CORS and works in both Node and the browser, with no +extra configuration. + +If no region is known, dotted-name buckets fall back to the global endpoint, +whose region redirect (for buckets outside `us-east-1`) is resolved +automatically in Node but not in browsers — the cross-origin redirect carries no +CORS headers. Supply a [`fetchClient`](#virtual-chunk-authentication) that routes +to the regional endpoint or a CORS-enabled proxy in that case. + ### Repository For direct access to branches, tags, and checkouts. diff --git a/src/format/flatbuffers/repo-parser.ts b/src/format/flatbuffers/repo-parser.ts index ca30480..f0c16a4 100644 --- a/src/format/flatbuffers/repo-parser.ts +++ b/src/format/flatbuffers/repo-parser.ts @@ -19,17 +19,41 @@ import { const SUPPORTED_SPEC_VERSION = 2; +/** + * S3 store config carried by a virtual chunk container, mirroring Rust's + * `S3Options`. Drives endpoint/addressing when translating `s3://` chunk + * locations to HTTPS — notably `region`, which lets dotted-name buckets + * address their regional endpoint directly (required for browser CORS, since + * the global endpoint's region redirect carries no CORS headers). + */ +export interface S3ContainerConfig { + region?: string; + endpointUrl?: string; + forcePathStyle?: boolean; +} + +/** + * A virtual chunk container from the repo config, mirroring Rust's + * `VirtualChunkContainer`. `name` resolves `vcc://name/...` references; + * `urlPrefix` matches absolute chunk locations (longest prefix wins). + */ +export interface VirtualChunkContainer { + name: string | null; + urlPrefix: string; + s3?: S3ContainerConfig; +} + /** Parsed repo file with cached metadata */ export interface ParsedRepo { repo: FbsRepo; specVersion: number; snapshotsLength: number; // Cached for bounds checking /** - * Map of Virtual Chunk Container name → `url_prefix` for resolving - * relative `vcc://name/path` chunk locations. Empty when the repo has - * no named containers (unnamed/legacy entries are excluded). + * Virtual chunk containers from the repo config, sorted by descending + * `urlPrefix` length so the most specific container matches an absolute + * location first. Empty when the repo declares no containers. */ - virtualChunkContainers: Map; + virtualChunkContainers: VirtualChunkContainer[]; } /** @@ -73,19 +97,19 @@ export function parseRepo(data: Uint8Array): ParsedRepo { } /** - * Extract named Virtual Chunk Containers from the flexbuffers-encoded - * RepositoryConfig on the repo table. - * - * Only containers with a non-null `name` are included — unnamed/legacy - * containers cannot be referenced by `vcc://` URLs. + * Extract virtual chunk containers from the flexbuffers-encoded + * RepositoryConfig, including each container's S3 store config (region/ + * endpoint/addressing). Unnamed containers are included too — they still + * match absolute chunk locations by `urlPrefix` even though they can't be + * referenced by `vcc://name`. Returned sorted by descending `urlPrefix` + * length so the most specific container matches first. * - * Returns an empty map when the config is absent, malformed, or has no - * named containers. Parsing failures are swallowed because virtual chunk - * resolution is a best-effort path; callers that actually hit a `vcc://` - * location with no matching name will surface a clear error at fetch time. + * Returns an empty array when the config is absent or malformed. Parsing + * failures are swallowed because virtual chunk resolution is best-effort; a + * `vcc://` location with no matching name surfaces a clear error at fetch time. */ -function parseVirtualChunkContainers(repo: FbsRepo): Map { - const result = new Map(); +function parseVirtualChunkContainers(repo: FbsRepo): VirtualChunkContainer[] { + const result: VirtualChunkContainer[] = []; const configBytes = repo.configArray(); if (!configBytes || configBytes.length === 0) return result; @@ -106,14 +130,37 @@ function parseVirtualChunkContainers(repo: FbsRepo): Map { for (const container of Object.values(containers)) { if (!isRecord(container)) continue; - const { name, url_prefix: urlPrefix } = container; - if (typeof name !== "string" || typeof urlPrefix !== "string") continue; - result.set(name, urlPrefix); + const urlPrefix = container.url_prefix; + if (typeof urlPrefix !== "string") continue; + const name = typeof container.name === "string" ? container.name : null; + const s3 = parseS3Store(container.store); + result.push(s3 ? { name, urlPrefix, s3 } : { name, urlPrefix }); } + // Most specific (longest) url_prefix first, so absolute-location matching + // prefers the narrowest container. + result.sort((a, b) => b.urlPrefix.length - a.urlPrefix.length); return result; } +/** + * Parse the S3-family store config from a container's `store`. The `s3`, + * `s3_compatible`, and `tigris` ObjectStoreConfig variants all share Rust's + * `S3Options` shape; gcs/azure/http/local stores yield undefined. + */ +function parseS3Store(store: unknown): S3ContainerConfig | undefined { + if (!isRecord(store)) return undefined; + const s3 = store.s3 ?? store.s3_compatible ?? store.tigris; + if (!isRecord(s3)) return undefined; + const cfg: S3ContainerConfig = {}; + if (typeof s3.region === "string") cfg.region = s3.region; + if (typeof s3.endpoint_url === "string") cfg.endpointUrl = s3.endpoint_url; + if (typeof s3.force_path_style === "boolean") { + cfg.forcePathStyle = s3.force_path_style; + } + return cfg; +} + function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } diff --git a/src/reader/range-coalescer.ts b/src/reader/range-coalescer.ts index 31d469c..5279f90 100644 --- a/src/reader/range-coalescer.ts +++ b/src/reader/range-coalescer.ts @@ -56,6 +56,22 @@ function expectedRangeLength(range: RangeQuery): number { return "suffixLength" in range ? range.suffixLength : range.length; } +/** + * Map an S3 path-style global-endpoint region redirect to the regional URL to + * retry. For buckets outside `us-east-1`, `s3.amazonaws.com` answers 301 with + * no `Location` header (so `fetch` can't follow it) and reports the region in + * `x-amz-bucket-region`; this rebuilds the URL against `s3..amazonaws.com`. + * Returns null when the response isn't that redirect. + */ +function regionalS3RedirectUrl(url: string, response: Response): string | null { + if (response.status !== 301 && response.status !== 307) return null; + const region = response.headers.get("x-amz-bucket-region"); + if (!region) return null; + const globalPrefix = "https://s3.amazonaws.com/"; + if (!url.startsWith(globalPrefix)) return null; + return `https://s3.${region}.amazonaws.com/${url.slice(globalPrefix.length)}`; +} + /** * Build a minimal `AsyncReadable` that services every `getRange` by * fetching the configured URL with the requested byte range. The zarr @@ -63,18 +79,22 @@ function expectedRangeLength(range: RangeQuery): number { * converge on the same path and become eligible for range-merging. */ export function makeUrlStore(opts: MakeUrlStoreOptions): AsyncReadable { - const { url, fetchClient, conditionalHeaders } = opts; + const { fetchClient, conditionalHeaders } = opts; + // Hint shared across reads: once one read resolves an S3 regional redirect, + // later reads start from the regional host. Each read works off its own + // `requestUrl` copy, so this value is an optimization, not a source of truth. + let pinnedUrl = opts.url; - async function doFetch(init: RequestInit): Promise { + async function doFetch(target: string, init: RequestInit): Promise { return fetchClient - ? await fetchClient.fetch(url, init) - : await fetch(url, init); + ? await fetchClient.fetch(target, init) + : await fetch(target, init); } return { async get() { throw new Error( - `Virtual chunk URL store for ${url} only supports ranged reads`, + `Virtual chunk URL store for ${pinnedUrl} only supports ranged reads`, ); }, async getRange(_key, range, options) { @@ -86,16 +106,36 @@ export function makeUrlStore(opts: MakeUrlStoreOptions): AsyncReadable { ? `bytes=-${range.suffixLength}` : `bytes=${range.offset}-${range.offset + range.length - 1}`; - const response = await doFetch({ headers, signal: options?.signal }); + // Copy the endpoint locally so a concurrent read updating the shared pin + // can't change which URL this request detects and retries against. + let requestUrl = pinnedUrl; + let response = await doFetch(requestUrl, { + headers, + signal: options?.signal, + }); + + // Resolve a global-endpoint region redirect and retry once on the + // regional host, re-pinning it for later reads. + const regionalUrl = regionalS3RedirectUrl(requestUrl, response); + if (regionalUrl) { + // Free the discarded redirect body so Node can reuse the connection. + response.body?.cancel().catch(() => {}); + requestUrl = regionalUrl; + pinnedUrl = regionalUrl; + response = await doFetch(requestUrl, { + headers, + signal: options?.signal, + }); + } if (response.status === 412) { throw new Error( - `Virtual chunk at ${url} failed integrity check — data has been modified since snapshot was created`, + `Virtual chunk at ${requestUrl} failed integrity check — data has been modified since snapshot was created`, ); } if (response.status !== 200 && response.status !== 206) { throw new Error( - `Failed to fetch virtual chunk from ${url}: ${response.status} ${response.statusText}`, + `Failed to fetch virtual chunk from ${requestUrl}: ${response.status} ${response.statusText}`, ); } @@ -108,7 +148,7 @@ export function makeUrlStore(opts: MakeUrlStoreOptions): AsyncReadable { const expected = expectedRangeLength(range); if (data.length === expected) return data; throw new Error( - `Virtual range response size mismatch for ${url}: expected ${expected} bytes, got ${data.length}`, + `Virtual range response size mismatch for ${requestUrl}: expected ${expected} bytes, got ${data.length}`, ); } @@ -119,7 +159,7 @@ export function makeUrlStore(opts: MakeUrlStoreOptions): AsyncReadable { const end = range.offset + range.length; if (data.length >= end) return data.slice(range.offset, end); throw new Error( - `Virtual range request not honored for ${url}: need at least ${end} bytes for fallback slicing, got ${data.length}`, + `Virtual range request not honored for ${requestUrl}: need at least ${end} bytes for fallback slicing, got ${data.length}`, ); } // Suffix-length on a 200 fallback: take the trailing suffixLength bytes. @@ -127,7 +167,7 @@ export function makeUrlStore(opts: MakeUrlStoreOptions): AsyncReadable { return data.slice(data.length - range.suffixLength); } throw new Error( - `Virtual suffix range request not honored for ${url}: need at least ${range.suffixLength} bytes for fallback slicing, got ${data.length}`, + `Virtual suffix range request not honored for ${requestUrl}: need at least ${range.suffixLength} bytes for fallback slicing, got ${data.length}`, ); }, }; diff --git a/src/reader/session.ts b/src/reader/session.ts index b5f3789..e2b4df9 100644 --- a/src/reader/session.ts +++ b/src/reader/session.ts @@ -46,6 +46,10 @@ import { type AsyncReadable, type RangeCoalescingFn, } from "./range-coalescer.js"; +import type { + VirtualChunkContainer, + S3ContainerConfig, +} from "../format/flatbuffers/repo-parser.js"; /** Default byte-gap threshold for zarrita's range coalescer (matches its own default). */ const RANGE_COALESCE_SIZE = 32 * 1024; @@ -108,22 +112,22 @@ export class ReadSession { private nextFetchClientId = 1; private rangeCoalescerIds?: WeakMap; private nextRangeCoalescerId = 1; - /** VCC name → url_prefix map for resolving `vcc://` chunk locations. */ - private virtualChunkContainers: Map; + /** Virtual chunk containers (from repo config) for resolving chunk locations. */ + private virtualChunkContainers: VirtualChunkContainer[]; private constructor( storage: Storage, snapshot: Snapshot, specVersion: SpecVersion, maxManifestCacheSize: number = 100, - virtualChunkContainers?: Map, + virtualChunkContainers?: VirtualChunkContainer[], ) { this.storage = storage; this.snapshot = snapshot; this.specVersion = specVersion; this.manifestCache = new LRUCache(maxManifestCacheSize); this.manifestLoader = singleFlight(this.manifestCache); - this.virtualChunkContainers = virtualChunkContainers ?? new Map(); + this.virtualChunkContainers = virtualChunkContainers ?? []; } private getFetchClientKey(fetchClient: FetchClient | undefined): string { @@ -256,7 +260,7 @@ export class ReadSession { snapshotId: Uint8Array, options?: RequestOptions & { maxManifestCacheSize?: number; - virtualChunkContainers?: Map; + virtualChunkContainers?: VirtualChunkContainer[]; }, ): Promise { const { snapshot, specVersion } = await ReadSession.loadSnapshot( @@ -637,14 +641,15 @@ export class ReadSession { case "virtual": { // Virtual chunks reference external URLs // Expand any vcc://name/path → absolute URL, then translate s3:// etc. → HTTPS - const absoluteLocation = expandVccUrl( - payload.location, - this.virtualChunkContainers, - ); - const httpUrl = translateToHttpUrl( - absoluteLocation, - options?.azureAccount, - ); + const { url: absoluteLocation, container } = + resolveVirtualChunkLocation( + payload.location, + this.virtualChunkContainers, + ); + const httpUrl = translateToHttpUrl(absoluteLocation, { + azureAccount: options?.azureAccount, + s3: container?.s3, + }); const store = await this.getVirtualStoreForPayload( httpUrl, @@ -724,14 +729,15 @@ export class ReadSession { const absoluteStart = payload.offset + rangeStart; const expectedSize = rangeEnd - rangeStart; - const absoluteLocation = expandVccUrl( - payload.location, - this.virtualChunkContainers, - ); - const httpUrl = translateToHttpUrl( - absoluteLocation, - options?.azureAccount, - ); + const { url: absoluteLocation, container } = + resolveVirtualChunkLocation( + payload.location, + this.virtualChunkContainers, + ); + const httpUrl = translateToHttpUrl(absoluteLocation, { + azureAccount: options?.azureAccount, + s3: container?.s3, + }); const store = await this.getVirtualStoreForPayload( httpUrl, @@ -823,52 +829,108 @@ function compareUtf8Bytes(a: string, b: string): number { const VCC_SCHEME = "vcc://"; /** - * Expand a `vcc://name/relative/path` chunk location to an absolute URL using - * the repo's Virtual Chunk Container map. Pass-through for any location that - * doesn't start with `vcc://`. + * Resolve a virtual chunk location to an absolute URL and its matching + * container. `vcc://name/...` references resolve by container name; absolute + * locations match the container whose `urlPrefix` is a prefix (the list is + * pre-sorted longest-first, so the most specific wins). The matched + * container's S3 store config drives endpoint/region selection in + * `translateToHttpUrl`. * - * When the map is empty (e.g. a `ReadSession` constructed without a - * `Repository`), vcc:// locations pass through unchanged so a caller's - * `fetchClient` can still handle them. When the map is non-empty but the - * referenced name is missing, we throw — that signals a real config / - * manifest mismatch the caller should surface. + * Pass-through with no container when the location isn't `vcc://` and no + * prefix matches, or when there are no containers at all (e.g. a `ReadSession` + * built without a `Repository`, where a caller's `fetchClient` resolves + * `vcc://` itself). * - * Persisted repo configs may contain legacy/migrated prefixes without a - * trailing slash. Normalize those before joining so `vcc://name/path` - * resolves under the configured container root instead of being appended to - * the last prefix segment. + * Persisted configs may carry legacy prefixes without a trailing slash; + * normalize before joining so `vcc://name/path` resolves under the container + * root rather than against the last prefix segment. * - * @throws Error when the URL is malformed or the name is unknown despite a - * populated container map. + * @throws Error when a `vcc://` URL is malformed, or names an unknown + * container despite a populated container list. */ -export function expandVccUrl( +export function resolveVirtualChunkLocation( location: string, - containers: Map, -): string { - if (!location.startsWith(VCC_SCHEME)) return location; - - const rest = location.slice(VCC_SCHEME.length); - const slash = rest.indexOf("/"); - if (slash === -1) { - throw new Error( - `Invalid vcc:// URL "${location}": missing "/" after container name`, - ); + containers: VirtualChunkContainer[], +): { url: string; container?: VirtualChunkContainer } { + if (location.startsWith(VCC_SCHEME)) { + const rest = location.slice(VCC_SCHEME.length); + const slash = rest.indexOf("/"); + if (slash === -1) { + throw new Error( + `Invalid vcc:// URL "${location}": missing "/" after container name`, + ); + } + const name = rest.slice(0, slash); + const relativePath = rest.slice(slash + 1); + const container = containers.find((c) => c.name === name); + if (!container) { + if (containers.length === 0) return { url: location }; + throw new Error( + `Unknown virtual chunk container "${name}" referenced by ${location}`, + ); + } + return { + url: ensureTrailingSlash(container.urlPrefix) + relativePath, + container, + }; } - const name = rest.slice(0, slash); - const relativePath = rest.slice(slash + 1); - const urlPrefix = containers.get(name); - if (urlPrefix === undefined) { - if (containers.size === 0) return location; - throw new Error( - `Unknown virtual chunk container "${name}" referenced by ${location}`, - ); - } + // Match on a normalized (trailing-slash) prefix so a legacy prefix like + // `s3://bucket` can't claim a sibling such as `s3://bucket2/key`. + const container = containers.find((c) => + location.startsWith(ensureTrailingSlash(c.urlPrefix)), + ); + return { url: location, container }; +} + +/** Append a trailing `/` if absent, so prefix matches respect a path boundary. */ +function ensureTrailingSlash(prefix: string): string { + return prefix.endsWith("/") ? prefix : `${prefix}/`; +} - const normalizedPrefix = urlPrefix.endsWith("/") - ? urlPrefix - : `${urlPrefix}/`; - return normalizedPrefix + relativePath; +/** + * Build an HTTPS URL for an `s3://bucket/key` location, honoring the + * container's S3 store config (mirrors how Rust builds the request): + * + * - `endpointUrl` (S3-compatible / Tigris) overrides the host, path-style. + * - dotted bucket names force path-style — virtual-hosted breaks TLS cert + * validation; `forcePathStyle` forces it for any bucket. + * - `region`, when known, targets the regional endpoint directly. This is + * required for browser reads of dotted buckets: the global endpoint's + * region redirect carries no CORS headers, so the browser can't follow it. + * Without a region we use the global endpoint and `makeUrlStore` resolves + * the redirect at fetch time (works server-side only). + */ +function buildS3HttpUrl( + bucket: string, + key: string, + s3?: S3ContainerConfig, +): string { + if (s3?.endpointUrl) { + const base = s3.endpointUrl.replace(/\/+$/, ""); + return `${base}/${bucket}/${key}`; + } + const region = s3?.region; + const pathStyle = s3?.forcePathStyle === true || bucket.includes("."); + if (pathStyle) { + const host = region + ? `s3.${region}.${awsEndpointSuffix(region)}` + : "s3.amazonaws.com"; + return `https://${host}/${bucket}/${key}`; + } + const host = region + ? `${bucket}.s3.${region}.${awsEndpointSuffix(region)}` + : `${bucket}.s3.amazonaws.com`; + return `https://${host}/${key}`; +} + +/** + * AWS partition endpoint suffix for an S3 region. China (`cn-*`) uses + * `amazonaws.com.cn`; GovCloud uses the commercial suffix, and ISO partitions + * are air-gapped (not web-reachable), so the commercial suffix covers them. + */ +function awsEndpointSuffix(region: string): string { + return region.startsWith("cn-") ? "amazonaws.com.cn" : "amazonaws.com"; } /** @@ -885,31 +947,24 @@ export function expandVccUrl( * the container name (not the account). The account must be supplied separately * via the azureAccount parameter. * - * Note: S3 URLs use virtual-hosted style for simple bucket names, but fall back to - * path-style for buckets containing dots (which break SSL certificate validation). - * For buckets in specific regions, S3 will redirect to the correct endpoint. + * Note: S3 addressing follows the container's store config (region, endpoint, + * path-style); see `buildS3HttpUrl`. When no region is known, dotted-name + * buckets use the global path-style endpoint, whose region redirect + * `makeUrlStore` resolves at fetch time (server only). */ -function translateToHttpUrl(url: string, azureAccount?: string): string { - // S3: s3://bucket/key → https://bucket.s3.amazonaws.com/key - // For buckets with dots, use path-style: https://s3.amazonaws.com/bucket/key +function translateToHttpUrl( + url: string, + options?: { azureAccount?: string; s3?: S3ContainerConfig }, +): string { + const azureAccount = options?.azureAccount; + // S3: addressing is driven by the container's S3 store config — see + // buildS3HttpUrl for the virtual-hosted vs path-style and region rules. if (url.startsWith("s3://")) { const rest = url.slice(5); // Remove 's3://' const slashIndex = rest.indexOf("/"); - if (slashIndex === -1) { - // Just bucket, no key - const bucket = rest; - if (bucket.includes(".")) { - return `https://s3.amazonaws.com/${bucket}/`; - } - return `https://${bucket}.s3.amazonaws.com/`; - } - const bucket = rest.slice(0, slashIndex); - const key = rest.slice(slashIndex + 1); - // Use path-style for buckets with dots (virtual-hosted fails SSL validation) - if (bucket.includes(".")) { - return `https://s3.amazonaws.com/${bucket}/${key}`; - } - return `https://${bucket}.s3.amazonaws.com/${key}`; + const bucket = slashIndex === -1 ? rest : rest.slice(0, slashIndex); + const key = slashIndex === -1 ? "" : rest.slice(slashIndex + 1); + return buildS3HttpUrl(bucket, key, options?.s3); } // GCS: gs://bucket/key or gcs://bucket/key → https://storage.googleapis.com/bucket/key diff --git a/tests/format/flatbuffers/repo-parser.test.ts b/tests/format/flatbuffers/repo-parser.test.ts index 894f2b5..f20c2e9 100644 --- a/tests/format/flatbuffers/repo-parser.test.ts +++ b/tests/format/flatbuffers/repo-parser.test.ts @@ -97,25 +97,47 @@ describe("repo-parser", () => { }); describe("virtualChunkContainers", () => { - it("is empty when the only container has no name", () => { - // test-repo-v2 is created via `ic.VirtualChunkContainer("s3://testbucket/", store)` - // with no `name=` argument, so its single container is unnamed (name: null) - // and MUST be excluded from the vcc:// resolution map. + it("includes the unnamed container with its parsed S3 store config", () => { + // test-repo-v2's single container is unnamed (name: null) but carries an + // S3 store config (region/endpoint/addressing) we use to build requests. const repoData = readFileSync(join(TEST_REPO_V2_PATH, "repo")); const repo = parseRepo(repoData); - expect(repo.virtualChunkContainers.size).toBe(0); + expect(repo.virtualChunkContainers).toHaveLength(1); + const c = repo.virtualChunkContainers[0]; + expect(c.name).toBeNull(); + expect(c.urlPrefix).toBe("s3://testbucket/"); + expect(c.s3).toEqual({ + region: "us-east-1", + endpointUrl: "http://localhost:4200", + forcePathStyle: true, + }); }); - it("includes every named container from a migrated v2 repo", () => { + it("includes every container from a migrated v2 repo with parsed S3 config", () => { // test-repo-v2-migrated was produced by upgrading a v1 repo, which seeds // the config with the built-in named containers for each scheme. const repoData = readFileSync(join(TEST_REPO_V2_MIGRATED_PATH, "repo")); const repo = parseRepo(repoData); - expect(repo.virtualChunkContainers.get("s3")).toBe("s3://testbucket"); - expect(repo.virtualChunkContainers.get("gcs")).toBe("gcs"); - expect(repo.virtualChunkContainers.get("az")).toBe("az"); - expect(repo.virtualChunkContainers.get("tigris")).toBe("tigris"); - expect(repo.virtualChunkContainers.get("file")).toBe("file"); + const byName = new Map( + repo.virtualChunkContainers.map((c) => [c.name, c]), + ); + + expect(byName.get("s3")?.urlPrefix).toBe("s3://testbucket"); + expect(byName.get("s3")?.s3).toEqual({ + region: "us-east-1", + endpointUrl: "http://localhost:4200", + forcePathStyle: true, + }); + // Tigris uses S3Options too: endpoint + addressing, region unset (null). + expect(byName.get("tigris")?.s3).toEqual({ + endpointUrl: "https://fly.storage.tigris.dev", + forcePathStyle: false, + }); + // Non-S3 stores carry no S3 config. + expect(byName.get("gcs")?.urlPrefix).toBe("gcs"); + expect(byName.get("gcs")?.s3).toBeUndefined(); + expect(byName.get("az")?.s3).toBeUndefined(); + expect(byName.get("file")?.s3).toBeUndefined(); }); }); diff --git a/tests/reader/range-coalescer.test.ts b/tests/reader/range-coalescer.test.ts index 7f9ce13..353beeb 100644 --- a/tests/reader/range-coalescer.test.ts +++ b/tests/reader/range-coalescer.test.ts @@ -18,10 +18,15 @@ function toArrayBuffer(data: Uint8Array): ArrayBuffer { ) as ArrayBuffer; } -function mockFetchResponse(status: number, data: Uint8Array): Response { +function mockFetchResponse( + status: number, + data: Uint8Array, + headers: Record = {}, +): Response { return { status, statusText: status === 206 ? "Partial Content" : "OK", + headers: { get: (name: string) => headers[name.toLowerCase()] ?? null }, arrayBuffer: vi.fn().mockResolvedValue(toArrayBuffer(data)), } as unknown as Response; } @@ -65,6 +70,124 @@ describe("range coalescer adapters", () => { fetchSpy.mockRestore(); }); + it("retries the regional endpoint when the S3 global endpoint 301-redirects", async () => { + const globalUrl = + "https://s3.amazonaws.com/us-west-2.opendata.source.coop/data.bin"; + const regionalUrl = + "https://s3.us-west-2.amazonaws.com/us-west-2.opendata.source.coop/data.bin"; + const body = new Uint8Array([1, 2, 3]); + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + mockFetchResponse(301, new Uint8Array(), { + "x-amz-bucket-region": "us-west-2", + }), + ) + .mockResolvedValueOnce(mockFetchResponse(206, body)); + const store = makeUrlStore({ url: globalUrl }); + + const result = await store.getRange("/", { offset: 0, length: 3 }); + + expect(result).toEqual(body); + expect(fetchSpy).toHaveBeenNthCalledWith(1, globalUrl, expect.anything()); + expect(fetchSpy).toHaveBeenNthCalledWith(2, regionalUrl, expect.anything()); + + fetchSpy.mockRestore(); + }); + + it("pins the regional endpoint for later reads after one redirect", async () => { + const globalUrl = + "https://s3.amazonaws.com/eu-central-1.example.bucket/data.bin"; + const regionalUrl = + "https://s3.eu-central-1.amazonaws.com/eu-central-1.example.bucket/data.bin"; + const body = new Uint8Array([4, 5, 6]); + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + mockFetchResponse(301, new Uint8Array(), { + "x-amz-bucket-region": "eu-central-1", + }), + ) + .mockResolvedValue(mockFetchResponse(206, body)); + const store = makeUrlStore({ url: globalUrl }); + + await store.getRange("/", { offset: 0, length: 3 }); + await store.getRange("/", { offset: 0, length: 3 }); + + // First read redirects (global → regional); second read goes straight to regional. + expect(fetchSpy).toHaveBeenCalledTimes(3); + expect(fetchSpy).toHaveBeenNthCalledWith(3, regionalUrl, expect.anything()); + + fetchSpy.mockRestore(); + }); + + it("cancels the discarded redirect body before retrying", async () => { + const globalUrl = "https://s3.amazonaws.com/dotted.bucket/data.bin"; + const regionalUrl = + "https://s3.eu-west-1.amazonaws.com/dotted.bucket/data.bin"; + const cancel = vi.fn().mockResolvedValue(undefined); + const redirect = { + status: 301, + statusText: "Moved Permanently", + headers: { + get: (name: string) => + name.toLowerCase() === "x-amz-bucket-region" ? "eu-west-1" : null, + }, + body: { cancel }, + arrayBuffer: vi.fn().mockResolvedValue(new ArrayBuffer(0)), + } as unknown as Response; + const body = new Uint8Array([9]); + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce(redirect) + .mockResolvedValueOnce(mockFetchResponse(206, body)); + const store = makeUrlStore({ url: globalUrl }); + + const result = await store.getRange("/", { offset: 0, length: 1 }); + + expect(result).toEqual(body); + expect(cancel).toHaveBeenCalledTimes(1); + expect(fetchSpy).toHaveBeenNthCalledWith(2, regionalUrl, expect.anything()); + + fetchSpy.mockRestore(); + }); + + it("retries both reads when concurrent requests hit the global endpoint", async () => { + const globalUrl = + "https://s3.amazonaws.com/us-west-2.opendata.source.coop/data.bin"; + const regionalUrl = + "https://s3.us-west-2.amazonaws.com/us-west-2.opendata.source.coop/data.bin"; + const body = new Uint8Array([1, 2, 3]); + // Mock by target URL, not call order: the global endpoint always 301s, + // the regional endpoint always serves. This reproduces two in-flight + // reads both starting against the global host before either resolves. + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockImplementation(async (input) => { + const target = String(input); + if (target === globalUrl) { + return mockFetchResponse(301, new Uint8Array(), { + "x-amz-bucket-region": "us-west-2", + }); + } + if (target === regionalUrl) return mockFetchResponse(206, body); + throw new Error(`unexpected fetch URL: ${target}`); + }); + const store = makeUrlStore({ url: globalUrl }); + + // Both reads snapshot the global URL before either resolves its redirect; + // each must detect and retry against the regional host independently. + const [a, b] = await Promise.all([ + store.getRange("/", { offset: 0, length: 3 }), + store.getRange("/", { offset: 0, length: 3 }), + ]); + + expect(a).toEqual(body); + expect(b).toEqual(body); + + fetchSpy.mockRestore(); + }); + it("rejects URL offset 206 responses with mismatched body length", async () => { const url = "https://example.com/data.bin"; const fetchSpy = vi diff --git a/tests/reader/session.test.ts b/tests/reader/session.test.ts index 8925167..6e583b3 100644 --- a/tests/reader/session.test.ts +++ b/tests/reader/session.test.ts @@ -19,6 +19,7 @@ import type { NodeSnapshot, ArrayNodeData, } from "../../src/format/flatbuffers/types.js"; +import type { VirtualChunkContainer } from "../../src/format/flatbuffers/repo-parser.js"; // ESM equivalent of __dirname const __dirname = dirname(fileURLToPath(import.meta.url)); @@ -62,7 +63,7 @@ function createMockSession(options: { nodes?: NodeSnapshot[]; storage?: MockStorage; specVersion?: SpecVersion; - virtualChunkContainers?: Map; + virtualChunkContainers?: VirtualChunkContainer[]; }): ReadSession { const mockSnapshot: Snapshot = { id: createMockSnapshotId(1) as any, @@ -81,12 +82,33 @@ function createMockSession(options: { session.manifestCache = new Map(); session.manifestLoader = singleFlight(session.manifestCache); session.nextFetchClientId = 1; - session.virtualChunkContainers = - options.virtualChunkContainers ?? new Map(); + session.virtualChunkContainers = options.virtualChunkContainers ?? []; return session; } +/** A fetchClient whose fetch resolves a 206 with `length` bytes. */ +function mockVirtualFetchClient(length = 7) { + return { + fetch: vi.fn().mockResolvedValue({ + ok: true, + status: 206, + arrayBuffer: vi.fn().mockResolvedValue(new Uint8Array(length).buffer), + } as any), + }; +} + +function virtualPayload(location: string, length = 7) { + return { + type: "virtual" as const, + location, + offset: 0, + length, + checksumEtag: null, + checksumLastModified: 0, + }; +} + /** Helper to create a group node */ function createGroupNode(path: string, userData: object = {}): NodeSnapshot { return { @@ -665,9 +687,9 @@ describe("ReadSession", () => { const session = createMockSession({ nodes: [], - virtualChunkContainers: new Map([ - ["my-data", "https://example.com/data/"], - ]), + virtualChunkContainers: [ + { name: "my-data", urlPrefix: "https://example.com/data/" }, + ], }) as any; const payload = { @@ -690,7 +712,9 @@ describe("ReadSession", () => { it("throws a clear error when a vcc:// URL references an unknown container", async () => { const session = createMockSession({ nodes: [], - virtualChunkContainers: new Map([["known", "https://example.com/"]]), + virtualChunkContainers: [ + { name: "known", urlPrefix: "https://example.com/" }, + ], }) as any; const payload = { @@ -870,6 +894,122 @@ describe("ReadSession", () => { "azureAccount option is required", ); }); + + it("uses the container region for dotted-name s3:// buckets (regional path-style)", async () => { + const fetchClient = mockVirtualFetchClient(); + const session = createMockSession({ + nodes: [], + virtualChunkContainers: [ + { + name: null, + urlPrefix: "s3://us-west-2.example/", + s3: { region: "us-west-2" }, + }, + ], + }) as any; + + await session.fetchChunkPayload( + virtualPayload("s3://us-west-2.example/a/b.bin"), + { fetchClient }, + ); + + expect(fetchClient.fetch).toHaveBeenCalledWith( + "https://s3.us-west-2.amazonaws.com/us-west-2.example/a/b.bin", + expect.any(Object), + ); + }); + + it("falls back to the global endpoint for dotted buckets without a container region", async () => { + const fetchClient = mockVirtualFetchClient(); + const session = createMockSession({ + nodes: [], + virtualChunkContainers: [ + { name: null, urlPrefix: "s3://dotted.example/" }, + ], + }) as any; + + await session.fetchChunkPayload( + virtualPayload("s3://dotted.example/a/b.bin"), + { fetchClient }, + ); + + expect(fetchClient.fetch).toHaveBeenCalledWith( + "https://s3.amazonaws.com/dotted.example/a/b.bin", + expect.any(Object), + ); + }); + + it("uses the regional virtual-hosted host for simple buckets with a region", async () => { + const fetchClient = mockVirtualFetchClient(); + const session = createMockSession({ + nodes: [], + virtualChunkContainers: [ + { + name: null, + urlPrefix: "s3://simple/", + s3: { region: "eu-west-1" }, + }, + ], + }) as any; + + await session.fetchChunkPayload(virtualPayload("s3://simple/a/b.bin"), { + fetchClient, + }); + + expect(fetchClient.fetch).toHaveBeenCalledWith( + "https://simple.s3.eu-west-1.amazonaws.com/a/b.bin", + expect.any(Object), + ); + }); + + it("uses the China partition suffix (amazonaws.com.cn) for cn- regions", async () => { + const fetchClient = mockVirtualFetchClient(); + const session = createMockSession({ + nodes: [], + virtualChunkContainers: [ + { + name: null, + urlPrefix: "s3://cn.bucket/", + s3: { region: "cn-north-1" }, + }, + ], + }) as any; + + await session.fetchChunkPayload( + virtualPayload("s3://cn.bucket/a/b.bin"), + { + fetchClient, + }, + ); + + expect(fetchClient.fetch).toHaveBeenCalledWith( + "https://s3.cn-north-1.amazonaws.com.cn/cn.bucket/a/b.bin", + expect.any(Object), + ); + }); + + it("routes to a custom endpoint_url (path-style) for S3-compatible containers", async () => { + const fetchClient = mockVirtualFetchClient(); + const session = createMockSession({ + nodes: [], + virtualChunkContainers: [ + { + name: null, + urlPrefix: "s3://mybucket/", + s3: { endpointUrl: "http://localhost:9000", forcePathStyle: true }, + }, + ], + }) as any; + + await session.fetchChunkPayload(virtualPayload("s3://mybucket/a/b.bin"), { + fetchClient, + }); + + expect(fetchClient.fetch).toHaveBeenCalledWith( + "http://localhost:9000/mybucket/a/b.bin", + expect.any(Object), + ); + }); }); describe("fetchChunkPayloadRange", () => { diff --git a/tests/reader/vcc-expansion.test.ts b/tests/reader/vcc-expansion.test.ts index f6387a3..ea3a604 100644 --- a/tests/reader/vcc-expansion.test.ts +++ b/tests/reader/vcc-expansion.test.ts @@ -1,70 +1,119 @@ import { describe, it, expect } from "vitest"; -import { expandVccUrl } from "../../src/reader/session.js"; +import { resolveVirtualChunkLocation } from "../../src/reader/session.js"; +import type { VirtualChunkContainer } from "../../src/format/flatbuffers/repo-parser.js"; -describe("expandVccUrl", () => { - const containers = new Map([ - ["my-data", "s3://mybucket/some/prefix/"], - ["gcs-data", "gs://other/"], - ["no-slash", "s3://bucket/key"], - ["migrated-s3", "s3://testbucket"], - ]); +describe("resolveVirtualChunkLocation", () => { + const containers: VirtualChunkContainer[] = [ + { + name: "my-data", + urlPrefix: "s3://mybucket/some/prefix/", + s3: { region: "us-west-2" }, + }, + { name: "gcs-data", urlPrefix: "gs://other/" }, + { name: "no-slash", urlPrefix: "s3://bucket/key" }, + { name: "migrated-s3", urlPrefix: "s3://testbucket" }, + // Unnamed container — matched by url_prefix only, never by vcc:// name. + { + name: null, + urlPrefix: "s3://us-west-2.example/", + s3: { region: "us-west-2" }, + }, + ]; - it("passes through absolute s3:// URLs unchanged", () => { - expect(expandVccUrl("s3://bucket/key", containers)).toBe("s3://bucket/key"); + it("passes through unmatched absolute s3:// URLs with no container", () => { + const r = resolveVirtualChunkLocation("s3://unmatched/key", containers); + expect(r).toEqual({ url: "s3://unmatched/key" }); }); - it("passes through https:// URLs unchanged", () => { - expect(expandVccUrl("https://example.com/x.nc", containers)).toBe( - "https://example.com/x.nc", + it("matches an absolute s3:// URL to its container by url_prefix", () => { + const r = resolveVirtualChunkLocation( + "s3://us-west-2.example/data/chunk.bin", + containers, ); + expect(r.url).toBe("s3://us-west-2.example/data/chunk.bin"); + expect(r.container?.name).toBeNull(); + expect(r.container?.s3).toEqual({ region: "us-west-2" }); + }); + + it("requires a path boundary, so a non-slash prefix can't claim a sibling bucket", () => { + const legacy: VirtualChunkContainer[] = [ + { name: null, urlPrefix: "s3://testbucket", s3: { region: "us-east-1" } }, + ]; + // Sibling bucket must NOT match the boundary-less prefix. + expect( + resolveVirtualChunkLocation("s3://testbucket2/key", legacy).container, + ).toBeUndefined(); + // The real bucket still matches (prefix is normalized to `s3://testbucket/`). + expect( + resolveVirtualChunkLocation("s3://testbucket/key", legacy).container?.s3, + ).toEqual({ region: "us-east-1" }); }); - it("expands vcc://name/path to url_prefix + path", () => { - expect(expandVccUrl("vcc://my-data/chunks/abc.nc", containers)).toBe( - "s3://mybucket/some/prefix/chunks/abc.nc", + it("prefers the most specific (longest) url_prefix match", () => { + const overlapping: VirtualChunkContainer[] = [ + { name: null, urlPrefix: "s3://b/", s3: { region: "us-east-1" } }, + { name: null, urlPrefix: "s3://b/deep/", s3: { region: "eu-west-1" } }, + ].sort((a, b) => b.urlPrefix.length - a.urlPrefix.length); + const r = resolveVirtualChunkLocation("s3://b/deep/x", overlapping); + expect(r.container?.s3).toEqual({ region: "eu-west-1" }); + }); + + it("passes through https:// URLs unchanged", () => { + expect( + resolveVirtualChunkLocation("https://example.com/x.nc", containers).url, + ).toBe("https://example.com/x.nc"); + }); + + it("expands vcc://name/path to url_prefix + path and returns the container", () => { + const r = resolveVirtualChunkLocation( + "vcc://my-data/chunks/abc.nc", + containers, ); + expect(r.url).toBe("s3://mybucket/some/prefix/chunks/abc.nc"); + expect(r.container?.s3).toEqual({ region: "us-west-2" }); }); it("normalizes url_prefix values that lack a trailing slash", () => { - expect(expandVccUrl("vcc://no-slash/tail", containers)).toBe( - "s3://bucket/key/tail", - ); + expect( + resolveVirtualChunkLocation("vcc://no-slash/tail", containers).url, + ).toBe("s3://bucket/key/tail"); }); it("normalizes migrated VCC prefixes before expansion", () => { - expect(expandVccUrl("vcc://migrated-s3/path/to/chunk", containers)).toBe( - "s3://testbucket/path/to/chunk", - ); + expect( + resolveVirtualChunkLocation("vcc://migrated-s3/path/to/chunk", containers) + .url, + ).toBe("s3://testbucket/path/to/chunk"); }); it("handles relative paths with nested slashes", () => { - expect(expandVccUrl("vcc://gcs-data/a/b/c", containers)).toBe( - "gs://other/a/b/c", - ); + expect( + resolveVirtualChunkLocation("vcc://gcs-data/a/b/c", containers).url, + ).toBe("gs://other/a/b/c"); }); it("allows an empty relative path", () => { - expect(expandVccUrl("vcc://my-data/", containers)).toBe( + expect(resolveVirtualChunkLocation("vcc://my-data/", containers).url).toBe( "s3://mybucket/some/prefix/", ); }); - it("throws on unknown container name when the map is populated", () => { - expect(() => expandVccUrl("vcc://missing/x", containers)).toThrow( - /Unknown virtual chunk container "missing"/, - ); + it("throws on unknown container name when the list is populated", () => { + expect(() => + resolveVirtualChunkLocation("vcc://missing/x", containers), + ).toThrow(/Unknown virtual chunk container "missing"/); }); it("throws when the vcc:// URL has no slash after the name", () => { - expect(() => expandVccUrl("vcc://my-data", containers)).toThrow( - /missing "\/" after container name/, - ); + expect(() => + resolveVirtualChunkLocation("vcc://my-data", containers), + ).toThrow(/missing "\/" after container name/); }); - it("passes vcc:// through unchanged when the container map is empty", () => { - // Preserves pre-patch behavior for callers using ReadSession.open() - // directly with a fetchClient that resolves vcc:// itself. - expect(expandVccUrl("vcc://anything/x", new Map())).toBe( + it("passes vcc:// through unchanged when the container list is empty", () => { + // Preserves behavior for callers using ReadSession.open() directly with a + // fetchClient that resolves vcc:// itself. + expect(resolveVirtualChunkLocation("vcc://anything/x", []).url).toBe( "vcc://anything/x", ); }); diff --git a/tests/reader/virtual-coalescing.test.ts b/tests/reader/virtual-coalescing.test.ts index 88d130e..cf18dd2 100644 --- a/tests/reader/virtual-coalescing.test.ts +++ b/tests/reader/virtual-coalescing.test.ts @@ -87,6 +87,7 @@ function createMockSession(options: { storage?: MockStorage } = {}): any { session.manifestLoader = singleFlight(session.manifestCache); session.nextFetchClientId = 1; session.nextRangeCoalescerId = 1; + session.virtualChunkContainers = []; return session; } diff --git a/tests/zarrita-integration.test.ts b/tests/zarrita-integration.test.ts index 9c617c5..f27b543 100644 --- a/tests/zarrita-integration.test.ts +++ b/tests/zarrita-integration.test.ts @@ -496,7 +496,9 @@ describe("Zarrita Integration", () => { const originalFetch = globalThis.fetch; vi.spyOn(globalThis, "fetch").mockImplementation(async (input, init) => { const url = typeof input === "string" ? input : input.toString(); - if (url.includes("testbucket.s3.amazonaws.com")) { + // The v2 repo config routes this container to its configured + // endpoint_url (a path-style S3-compatible host), not AWS. + if (url.includes("localhost:4200/testbucket")) { return new Response(chunkBytes.buffer.slice(0), { status: 206, headers: { "Content-Type": "application/octet-stream" },