From 1255beb992e5ab33932a4ecb6966a022be253803 Mon Sep 17 00:00:00 2001 From: Jeremy Maitin-Shepard Date: Wed, 13 Nov 2024 13:01:58 -0800 Subject: [PATCH 1/2] fix: use HEAD request rather than bytes=-N suffix requests Fixes https://github.com/google/neuroglancer/issues/651 --- src/kvstore/special/index.ts | 170 +++++++++++++++++------------------ 1 file changed, 84 insertions(+), 86 deletions(-) diff --git a/src/kvstore/special/index.ts b/src/kvstore/special/index.ts index 22423b00e..ac9ee232a 100644 --- a/src/kvstore/special/index.ts +++ b/src/kvstore/special/index.ts @@ -23,7 +23,7 @@ import type { } from "#src/kvstore/index.js"; import { composeByteRangeRequest } from "#src/kvstore/index.js"; import { uncancelableToken } from "#src/util/cancellation.js"; -import { HttpError, isNotFoundError } from "#src/util/http_request.js"; +import { isNotFoundError } from "#src/util/http_request.js"; import type { SpecialProtocolCredentialsProvider } from "#src/util/special_protocol_request.js"; import { cancellableFetchSpecialOk } from "#src/util/special_protocol_request.js"; @@ -85,97 +85,95 @@ class SpecialProtocolKvStore implements ReadableKvStore { const { cancellationToken = uncancelableToken } = options; let { byteRange: byteRangeRequest } = options; const url = this.baseUrl + key; - for (let attempt = 0; ; ++attempt) { - try { - const requestInit: RequestInit = {}; - const rangeHeader = getRangeHeader(byteRangeRequest); - if (rangeHeader !== undefined) { - requestInit.headers = { range: rangeHeader }; - requestInit.cache = byteRangeCacheMode; + + try { + // The HTTP spec supports suffixLength requests directly via "Range: + // bytes=-N" requests, which avoids the need for a separate HEAD request. + // However, per + // https://fetch.spec.whatwg.org/#cors-safelisted-request-header a suffix + // length byte range request header will always trigger an OPTIONS preflight + // request, which would otherwise be avoided. This negates the benefit of + // using a suffixLength request directly. Additionally, some servers such as + // the npm http-server package and https://uk1s3.embassy.ebi.ac.uk/ do not + // correctly handle suffixLength requests or do not correctly handle CORS + // preflight requests. To avoid those issues, always just issue a separate + // HEAD request to determine the length. + let totalSize: number | undefined; + if ( + byteRangeRequest !== undefined && + "suffixLength" in byteRangeRequest + ) { + const totalSize = await this.getObjectLength(url, options); + byteRangeRequest = composeByteRangeRequest( + { offset: 0, length: totalSize }, + byteRangeRequest, + ).outer; + } + const requestInit: RequestInit = {}; + const rangeHeader = getRangeHeader(byteRangeRequest); + if (rangeHeader !== undefined) { + requestInit.headers = { range: rangeHeader }; + requestInit.cache = byteRangeCacheMode; + } + const { response, data } = await cancellableFetchSpecialOk( + this.credentialsProvider, + url, + requestInit, + async (response) => ({ + response, + data: await response.arrayBuffer(), + }), + cancellationToken, + ); + let byteRange: ByteRange | undefined; + if (response.status === 206) { + const contentRange = response.headers.get("content-range"); + if (contentRange === null) { + // Content-range should always be sent, but some buggy servers don't + // send it. + if (byteRangeRequest !== undefined) { + byteRange = { + offset: byteRangeRequest.offset, + length: data.byteLength, + }; + } else { + throw new Error( + "Unexpected HTTP 206 response when no byte range specified.", + ); + } } - const { response, data } = await cancellableFetchSpecialOk( - this.credentialsProvider, - url, - requestInit, - async (response) => ({ - response, - data: await response.arrayBuffer(), - }), - cancellationToken, - ); - let byteRange: ByteRange | undefined; - let totalSize: number | undefined; - if (response.status === 206) { - const contentRange = response.headers.get("content-range"); - if (contentRange === null) { - if (byteRangeRequest !== undefined) { - if ("suffixLength" in byteRangeRequest) { - const objectSize = await this.getObjectLength(url, options); - byteRange = { - offset: objectSize - byteRangeRequest.suffixLength, - length: Number(response.headers.get("content-length")), - }; - } else { - byteRange = { - offset: byteRangeRequest.offset, - length: data.byteLength, - }; - } - } else { - throw new Error( - "Unexpected HTTP 206 response when no byte range specified.", - ); - } + if (contentRange !== null) { + const m = contentRange.match(/bytes ([0-9]+)-([0-9]+)\/([0-9]+|\*)/); + if (m === null) { + throw new Error( + `Invalid content-range header: ${JSON.stringify(contentRange)}`, + ); } - if (contentRange !== null) { - const m = contentRange.match( - /bytes ([0-9]+)-([0-9]+)\/([0-9]+|\*)/, + const beginPos = parseInt(m[1], 10); + const endPos = parseInt(m[2], 10); + if (endPos !== beginPos + data.byteLength - 1) { + throw new Error( + `Length in content-range header ${JSON.stringify( + contentRange, + )} does not match content length ${data.byteLength}`, ); - if (m === null) { - throw new Error( - `Invalid content-range header: ${JSON.stringify(contentRange)}`, - ); - } - const beginPos = parseInt(m[1], 10); - const endPos = parseInt(m[2], 10); - if (endPos !== beginPos + data.byteLength - 1) { - throw new Error( - `Length in content-range header ${JSON.stringify( - contentRange, - )} does not match content length ${data.byteLength}`, - ); - } - totalSize = m[3] === "*" ? undefined : parseInt(m[3], 10); - byteRange = { offset: beginPos, length: data.byteLength }; } + if (m[3] !== "*") { + totalSize = parseInt(m[3], 10); + } + byteRange = { offset: beginPos, length: data.byteLength }; } - if (byteRange === undefined) { - byteRange = { offset: 0, length: data.byteLength }; - totalSize = data.byteLength; - } - return { data: new Uint8Array(data), dataRange: byteRange, totalSize }; - } catch (e) { - if ( - attempt === 0 && - e instanceof HttpError && - e.status === 416 && - options.byteRange !== undefined && - "suffixLength" in options.byteRange - ) { - // Some servers, such as the npm http-server package, do not support suffixLength - // byte-range requests. - const contentLengthNumber = await this.getObjectLength(url, options); - byteRangeRequest = composeByteRangeRequest( - { offset: 0, length: contentLengthNumber }, - byteRangeRequest, - ).outer; - continue; - } - if (isNotFoundError(e)) { - return undefined; - } - throw e; } + if (byteRange === undefined) { + byteRange = { offset: 0, length: data.byteLength }; + totalSize = data.byteLength; + } + return { data: new Uint8Array(data), dataRange: byteRange, totalSize }; + } catch (e) { + if (isNotFoundError(e)) { + return undefined; + } + throw e; } } } From 39272cdd398459bcbc03bf2f18affa9ebc2e340d Mon Sep 17 00:00:00 2001 From: Jeremy Maitin-Shepard Date: Wed, 13 Nov 2024 21:57:45 -0800 Subject: [PATCH 2/2] fix(package.json): fix node.js version requirement Fixes https://github.com/google/neuroglancer/issues/643. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 9a9fe3a88..3ac3f3e7b 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "url": "git+https://github.com/google/neuroglancer.git" }, "engines": { - "node": ">=20.10 <21 || >=21.2" + "node": ">=20.11 <21 || >=21.2" }, "browserslist": [ "last 2 Chrome versions",