From cfbf25da3da208696b1f9f6de5db4635a70b01f3 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sun, 9 Aug 2026 21:12:42 -0400 Subject: [PATCH 1/2] fix(share): do not flag embedded deck.gl rows as unloadable Two follow-ups to the share-readiness check in #1812, from review comments that landed after that PR merged. A non-GeoJSON deck.gl visualization (arc, heatmap, hexagon built from a CSV) keeps its rows in `source.data` as an array. `isPlainObject` excludes arrays, so `carriesOwnData` missed those layers, they fell through to the reference walk, and a layer whose data travels inside the project file was reported as "no source" or, when `sourcePath` still held the original CSV name, as a local file. An array `data` now counts as embedded, like an inline FeatureCollection. Only a string `data` is a URL. Separately, a rejected ranged GET no longer condemns the host. `Range` is CORS-safelisted only for a simple byte range, and an older webview may preflight it and get no matching `Access-Control-Allow-Headers` back. The HEAD that preceded the retry already proved the host answers and lets this origin read the response, so a rejection there is about the ranged request rather than the host: the probe falls back to the HEAD's verdict instead of reporting "a browser cannot fetch this host" for a host whose plain GET a renderer would fetch fine. Refs #1671. --- .../src/lib/share-readiness.ts | 25 ++++++++-- tests/share-readiness.test.ts | 49 +++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/apps/geolibre-desktop/src/lib/share-readiness.ts b/apps/geolibre-desktop/src/lib/share-readiness.ts index 536d6b0bb4..ede4e55622 100644 --- a/apps/geolibre-desktop/src/lib/share-readiness.ts +++ b/apps/geolibre-desktop/src/lib/share-readiness.ts @@ -377,7 +377,12 @@ function carriesOwnData(layer: GeoLibreLayer, embeddedLayerIds?: ReadonlySet { assert.equal(refs[0].probeUrl, "https://tiles.example.com/tileset.json"); }); + it("skips a deck.gl visualization whose rows are inlined as an array", () => { + const refs = collectShareSources({ + layers: [ + layer({ + id: "a", + name: "Arcs from CSV", + type: "deckgl-viz", + source: { type: "deckgl-viz", data: [{ lat: 1, lon: 2 }] }, + // Set when the layer is built from a local file, and not a reference + // a recipient needs: the rows travel in `source.data`. + sourcePath: "/home/me/flows.csv", + }), + ], + }); + assert.deepEqual(refs, []); + }); + it("reports a query-backed layer that names no reference at all", () => { const refs = collectShareSources({ layers: [ @@ -332,6 +349,38 @@ describe("probeShareSources", () => { ]); }); + it("falls back to the HEAD verdict when only the ranged GET is rejected", async () => { + const refs = collectShareSources({ + layers: [ + layer({ id: "a", name: "A", type: "cog", source: { url: "https://s3.example.com/a.tif" } }), + ], + }); + // HEAD answers 405, so the host is up and readable cross-origin; the ranged + // GET is rejected on its own (an older webview preflighting `Range`). That + // must not turn a working host into a "blocked" verdict. + const fn = (async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "HEAD") return new Response(null, { status: 405 }); + throw new TypeError("Failed to fetch"); + }) as unknown as typeof fetch; + const { refs: probed } = await probeShareSources(refs, { fetchImpl: fn }); + assert.equal(probed[0].status, "reachable"); + }); + + it("keeps a HEAD 403 credentialed when the ranged GET is also rejected", async () => { + const refs = collectShareSources({ + layers: [ + layer({ id: "a", name: "A", type: "cog", source: { url: "https://s3.example.com/a.tif" } }), + ], + }); + const fn = (async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "HEAD") return new Response(null, { status: 403 }); + throw new TypeError("Failed to fetch"); + }) as unknown as typeof fetch; + const { refs: probed } = await probeShareSources(refs, { fetchImpl: fn }); + assert.equal(probed[0].status, "credentialed"); + assert.equal(probed[0].reason, "auth-required"); + }); + it("reads an opaque browser rejection as browser-blocked", async () => { const refs = collectShareSources({ layers: [ From 0ee143fa20385b5e42ee3254b1c6aa44d613d7a6 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sun, 9 Aug 2026 21:23:19 -0400 Subject: [PATCH 2/2] Assert the ranged GET ran in the fallback tests Both fallback tests rejected every non-HEAD request but never checked that a GET was attempted, so they would have kept passing if the retry were dropped entirely: a bare HEAD 405 already reads as reachable and a bare 403 as credentialed. They now record each attempt through a shared helper and assert HEAD followed by GET with `Range: bytes=0-0`. Verified by deleting the retry locally, which takes the suite from 26 passing to 3 failing instead of 1. --- tests/share-readiness.test.ts | 37 +++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/tests/share-readiness.test.ts b/tests/share-readiness.test.ts index a4a6b38228..f5a073a02f 100644 --- a/tests/share-readiness.test.ts +++ b/tests/share-readiness.test.ts @@ -45,6 +45,23 @@ function fakeFetch(routes: Record) { return { fn, calls }; } +/** + * Answers HEAD with `headStatus` and rejects the ranged GET, recording both + * attempts so a test can prove the retry actually ran. + */ +function rejectingRangedGet(headStatus: number) { + const attempts: { method?: string; range?: string }[] = []; + const fn = (async (_input: RequestInfo | URL, init?: RequestInit) => { + attempts.push({ + method: init?.method, + range: (init?.headers as Record | undefined)?.Range, + }); + if (init?.method === "HEAD") return new Response(null, { status: headStatus }); + throw new TypeError("Failed to fetch"); + }) as unknown as typeof fetch; + return { fn, attempts }; +} + describe("isPrivateHostname", () => { it("recognizes loopback, private ranges, and reserved suffixes", () => { for (const host of [ @@ -358,12 +375,15 @@ describe("probeShareSources", () => { // HEAD answers 405, so the host is up and readable cross-origin; the ranged // GET is rejected on its own (an older webview preflighting `Range`). That // must not turn a working host into a "blocked" verdict. - const fn = (async (_input: RequestInfo | URL, init?: RequestInit) => { - if (init?.method === "HEAD") return new Response(null, { status: 405 }); - throw new TypeError("Failed to fetch"); - }) as unknown as typeof fetch; + const { fn, attempts } = rejectingRangedGet(405); const { refs: probed } = await probeShareSources(refs, { fetchImpl: fn }); assert.equal(probed[0].status, "reachable"); + // Without this the test would still pass if the retry were dropped + // entirely, since a bare HEAD 405 also reads as reachable. + assert.deepEqual(attempts, [ + { method: "HEAD", range: undefined }, + { method: "GET", range: "bytes=0-0" }, + ]); }); it("keeps a HEAD 403 credentialed when the ranged GET is also rejected", async () => { @@ -372,13 +392,14 @@ describe("probeShareSources", () => { layer({ id: "a", name: "A", type: "cog", source: { url: "https://s3.example.com/a.tif" } }), ], }); - const fn = (async (_input: RequestInfo | URL, init?: RequestInit) => { - if (init?.method === "HEAD") return new Response(null, { status: 403 }); - throw new TypeError("Failed to fetch"); - }) as unknown as typeof fetch; + const { fn, attempts } = rejectingRangedGet(403); const { refs: probed } = await probeShareSources(refs, { fetchImpl: fn }); assert.equal(probed[0].status, "credentialed"); assert.equal(probed[0].reason, "auth-required"); + assert.deepEqual(attempts, [ + { method: "HEAD", range: undefined }, + { method: "GET", range: "bytes=0-0" }, + ]); }); it("reads an opaque browser rejection as browser-blocked", async () => {