From aa705a782d838d540b916daa60ca09eaa86603ab Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Mon, 4 Mar 2024 11:05:47 -0800 Subject: [PATCH 01/39] chore: limit body parameters to the types used --- packages/verified-fetch/src/types.ts | 2 ++ packages/verified-fetch/src/utils/responses.ts | 10 ++++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/verified-fetch/src/types.ts b/packages/verified-fetch/src/types.ts index 4a235e1a..f46515d7 100644 --- a/packages/verified-fetch/src/types.ts +++ b/packages/verified-fetch/src/types.ts @@ -1 +1,3 @@ export type RequestFormatShorthand = 'raw' | 'car' | 'tar' | 'ipns-record' | 'dag-json' | 'dag-cbor' | 'json' | 'cbor' + +export type SupportedBodyTypes = string | ArrayBuffer | Blob | ReadableStream | null diff --git a/packages/verified-fetch/src/utils/responses.ts b/packages/verified-fetch/src/utils/responses.ts index a5963706..1a6d74c4 100644 --- a/packages/verified-fetch/src/utils/responses.ts +++ b/packages/verified-fetch/src/utils/responses.ts @@ -1,3 +1,5 @@ +import type { SupportedBodyTypes } from '../types.js' + function setField (response: Response, name: string, value: string | boolean): void { Object.defineProperty(response, name, { enumerable: true, @@ -23,7 +25,7 @@ export interface ResponseOptions extends ResponseInit { redirected?: boolean } -export function okResponse (url: string, body?: BodyInit | null, init?: ResponseOptions): Response { +export function okResponse (url: string, body?: SupportedBodyTypes, init?: ResponseOptions): Response { const response = new Response(body, { ...(init ?? {}), status: 200, @@ -40,7 +42,7 @@ export function okResponse (url: string, body?: BodyInit | null, init?: Response return response } -export function notSupportedResponse (url: string, body?: BodyInit | null, init?: ResponseInit): Response { +export function notSupportedResponse (url: string, body?: SupportedBodyTypes, init?: ResponseInit): Response { const response = new Response(body, { ...(init ?? {}), status: 501, @@ -54,7 +56,7 @@ export function notSupportedResponse (url: string, body?: BodyInit | null, init? return response } -export function notAcceptableResponse (url: string, body?: BodyInit | null, init?: ResponseInit): Response { +export function notAcceptableResponse (url: string, body?: SupportedBodyTypes, init?: ResponseInit): Response { const response = new Response(body, { ...(init ?? {}), status: 406, @@ -67,7 +69,7 @@ export function notAcceptableResponse (url: string, body?: BodyInit | null, init return response } -export function badRequestResponse (url: string, body?: BodyInit | null, init?: ResponseInit): Response { +export function badRequestResponse (url: string, body?: SupportedBodyTypes, init?: ResponseInit): Response { const response = new Response(body, { ...(init ?? {}), status: 400, From 089ae240893cd7b63dc0a6b24d17c5d690d4d735 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Mon, 4 Mar 2024 11:15:35 -0800 Subject: [PATCH 02/39] chore: add response-header helper and tests --- .../src/utils/response-headers.ts | 31 +++++++++++ .../test/utils/response-headers.spec.ts | 55 +++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 packages/verified-fetch/src/utils/response-headers.ts create mode 100644 packages/verified-fetch/test/utils/response-headers.spec.ts diff --git a/packages/verified-fetch/src/utils/response-headers.ts b/packages/verified-fetch/src/utils/response-headers.ts new file mode 100644 index 00000000..53bfd5af --- /dev/null +++ b/packages/verified-fetch/src/utils/response-headers.ts @@ -0,0 +1,31 @@ +import type { SupportedBodyTypes } from '../types.js' + +/** + * Gets the body size of a given body if it's possible to calculate it synchronously. + */ +function syncBodySize (body: SupportedBodyTypes): number | null { + if (typeof body === 'string') { + return body.length + } + if (body instanceof ArrayBuffer) { + return body.byteLength + } + if (body instanceof Blob) { + return body.size + } + if (body instanceof ReadableStream) { + return null + } + return null +} + +/** + * This function returns the value of the `Content-Range` header for a given range. + * If you know the total size of the body, you should pass it in the `options` object. + * + * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range + */ +export function getContentRangeHeader (start: number, end: number, options: { total?: number, body: SupportedBodyTypes }): string { + const total = options.total ?? syncBodySize(options.body) ?? '*' // if we don't know the total size, we should use * + return `bytes ${start}-${end}/${total}` +} diff --git a/packages/verified-fetch/test/utils/response-headers.spec.ts b/packages/verified-fetch/test/utils/response-headers.spec.ts new file mode 100644 index 00000000..6906db31 --- /dev/null +++ b/packages/verified-fetch/test/utils/response-headers.spec.ts @@ -0,0 +1,55 @@ +import { expect } from 'aegir/chai' +import { getContentRangeHeader } from '../../src/utils/response-headers.js' + +describe('response-headers', () => { + describe('getContentRangeHeader', () => { + it('should return correct content range header when total is provided', () => { + const start = 0 + const end = 500 + const total = 1000 + const body = 'dummy body' + const result = getContentRangeHeader(start, end, { total, body }) + expect(result).to.equal(`bytes ${start}-${end}/${total}`) + }) + + it('should return correct content range header when total is not provided', () => { + const start = 0 + const end = 500 + const body = 'dummy body' + const result = getContentRangeHeader(start, end, { body }) + expect(result).to.equal(`bytes ${start}-${end}/${body.length}`) + }) + + it('should return content range header with * when total and body size are not known', () => { + const start = 0 + const end = 500 + const body = null + const result = getContentRangeHeader(start, end, { body }) + expect(result).to.equal(`bytes ${start}-${end}/*`) + }) + + it('should return content range header with * body is a ReadableStream', () => { + const start = 0 + const end = 500 + const body = new ReadableStream() + const result = getContentRangeHeader(start, end, { body }) + expect(result).to.equal(`bytes ${start}-${end}/*`) + }) + + it('should return content range header with the correct arrayBuffer size', () => { + const start = 0 + const end = 500 + const body = new ArrayBuffer(1000) + const result = getContentRangeHeader(start, end, { body }) + expect(result).to.equal(`bytes ${start}-${end}/${body.byteLength}`) + }) + + it('should return content range header with the correct Blob size', () => { + const start = 0 + const end = 500 + const body = new Blob(['dummy body']) + const result = getContentRangeHeader(start, end, { body }) + expect(result).to.equal(`bytes ${start}-${end}/${body.size}`) + }) + }) +}) From 5af92524d4babe18023da63f3b12052a08f8c32e Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Mon, 4 Mar 2024 13:00:32 -0800 Subject: [PATCH 03/39] feat: add range header parsing support --- .../src/utils/request-headers.ts | 91 +++++++++++++++++++ .../test/utils/request-headers.spec.ts | 68 ++++++++++++++ 2 files changed, 159 insertions(+) create mode 100644 packages/verified-fetch/src/utils/request-headers.ts create mode 100644 packages/verified-fetch/test/utils/request-headers.spec.ts diff --git a/packages/verified-fetch/src/utils/request-headers.ts b/packages/verified-fetch/src/utils/request-headers.ts new file mode 100644 index 00000000..919070d3 --- /dev/null +++ b/packages/verified-fetch/src/utils/request-headers.ts @@ -0,0 +1,91 @@ +import { type CatOptions } from '@helia/unixfs' +import { CodeError } from '@libp2p/interface' +import { type ExporterOptions } from 'ipfs-unixfs-exporter' + +export function getHeader (headers: HeadersInit | undefined, header: string): string | undefined { + if (headers == null) { + return undefined + } + if (headers instanceof Headers) { + return headers.get(header) ?? undefined + } + if (Array.isArray(headers)) { + const entry = headers.find(([key]) => key.toLowerCase() === header.toLowerCase()) + return entry?.[1] + } + if (typeof headers === 'object') { + const key = Object.keys(headers).find(k => k.toLowerCase() === header.toLowerCase()) + if (key == null) { + return undefined + } + return headers[key] + } + return headers[header] +} + +/** + * Type abstraction that will break the build if the supported range options change. + */ +export interface HeliaRangeOptions extends Pick { +} + +/** + * Converts a range request header into helia/unixfs supported range options + * Note that the gateway specification says we "MAY" support multiple ranges (https://specs.ipfs.tech/http-gateways/path-gateway/#range-request-header) but we don't + * + * Also note that @helia/unixfs and ipfs-unixfs-exporter expect length and offset to be numbers, the range header is a string, and the size of the resource is likely a bigint. + * + * SUPPORTED: + * Range: bytes=- + * Range: bytes=- + * Range: bytes=- // must pass size so we can calculate the offset + * + * NOT SUPPORTED: + * Range: bytes=-, - + * Range: bytes=-, -, - + * + * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Range#directives + */ +export function getRequestRange (headers: Headers | HeadersInit | undefined, size?: bigint): HeliaRangeOptions | undefined { + const range = getHeader(headers, 'Range') + if (range == null) { + return undefined + } + + // const match = range.match(/^bytes=(\d+)-(\d+)$/) + /** + * Range: bytes=- + * Range: bytes=- + * Range: bytes=- + */ + const match = range.match(/^bytes=((?\d+)-(?\d+)|(?\d+)-|-(?\d+))$/) + if (match?.groups == null) { + throw new CodeError('ERR_INVALID_RANGE_REQUEST', 'Invalid range request') + } + const { start, end, start2, end2 } = match.groups + if (start2 != null && end2 != null) { + // this should never happen + throw new CodeError('ERR_INVALID_RANGE_REQUEST', 'Invalid range request') + } + + let offset: number | undefined + let length: number | undefined + if (end2 != null) { + if (size == null) { + throw new CodeError('ERR_HANDLING_RANGE_REQUEST', 'Cannot use suffix length without knowing the size of the resource') + } + const stop = BigInt(end2) + offset = Number(size - stop) + length = Number(stop) + } else if (start2 != null) { + offset = parseInt(start2) + } else { + offset = parseInt(start) + length = parseInt(end) - offset + 1 + } + + return { + offset, + length + } +} diff --git a/packages/verified-fetch/test/utils/request-headers.spec.ts b/packages/verified-fetch/test/utils/request-headers.spec.ts new file mode 100644 index 00000000..fb90bae1 --- /dev/null +++ b/packages/verified-fetch/test/utils/request-headers.spec.ts @@ -0,0 +1,68 @@ +import { expect } from 'aegir/chai' +import { getHeader, getRequestRange } from '../../src/utils/request-headers.js' + +describe('request-headers', () => { + describe('getHeader', () => { + it('should return undefined when headers are undefined', () => { + const result = getHeader(undefined, 'dummy') + expect(result).to.be.undefined() + }) + + it('should return correct header value for Headers instance', () => { + const headers = new Headers({ Dummy: 'value' }) + expect(getHeader(headers, 'Dummy')).to.equal('value') + expect(getHeader(headers, 'dummy')).to.equal('value') + }) + + it('should return correct header value for array of tuples', () => { + const headers: Array<[string, string]> = [['Dummy', 'value']] + expect(getHeader(headers, 'Dummy')).to.equal('value') + expect(getHeader(headers, 'dummy')).to.equal('value') + }) + + it('should return correct header value for record', () => { + const headers: Record = { Dummy: 'value' } + expect(getHeader(headers, 'Dummy')).to.equal('value') + expect(getHeader(headers, 'dummy')).to.equal('value') + }) + }) + + describe('getRequestRange', () => { + it('should return undefined when no Range header is provided', () => { + const headers = new Headers() + const result = getRequestRange(headers) + expect(result).to.be.undefined() + }) + + it('should throw an error when an invalid Range header is provided', () => { + expect(() => getRequestRange(new Headers({ Range: 'invalid' }))).to.throw('ERR_INVALID_RANGE_REQUEST') + expect(() => getRequestRange(new Headers({ Range: 'bytes=foobar' }))).to.throw('ERR_INVALID_RANGE_REQUEST') + }) + + it('throws for multi-range requests', () => { + expect(() => getRequestRange(new Headers({ Range: 'bytes=0-500, 600-1000' }))).to.throw('ERR_INVALID_RANGE_REQUEST') + }) + + it('should return correct range options when a valid Range header has start only', () => { + const headers = new Headers({ Range: 'bytes=2-' }) + const result = getRequestRange(headers) + expect(result).to.deep.equal({ offset: 2, length: undefined }) + }) + + it('should return correct range options when a valid Range header has start and end', () => { + const headers = new Headers({ Range: 'bytes=0-500' }) + const result = getRequestRange(headers) + expect(result).to.deep.equal({ offset: 0, length: 501 }) + }) + + it('should throw when Range header has end only and size is not passed', () => { + expect(() => getRequestRange(new Headers({ Range: 'bytes=-20' }))).to.throw('ERR_HANDLING_RANGE_REQUEST') + }) + + it('should return correct range options when a valid Range header has end only', () => { + const headers = new Headers({ Range: 'bytes=-25' }) + const result = getRequestRange(headers, 100n) + expect(result).to.deep.equal({ offset: 75, length: 25 }) + }) + }) +}) From 3b2e3795947442660ee68f6022c35e3ed052072f Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Mon, 4 Mar 2024 13:43:07 -0800 Subject: [PATCH 04/39] feat: verified-fetch supports range-requests --- .../verified-fetch/src/utils/responses.ts | 51 ++++++++++++++ packages/verified-fetch/src/verified-fetch.ts | 58 +++++++++++----- .../test/range-requests.spec.ts | 69 +++++++++++++++++++ 3 files changed, 162 insertions(+), 16 deletions(-) create mode 100644 packages/verified-fetch/test/range-requests.spec.ts diff --git a/packages/verified-fetch/src/utils/responses.ts b/packages/verified-fetch/src/utils/responses.ts index 1a6d74c4..64ab37cc 100644 --- a/packages/verified-fetch/src/utils/responses.ts +++ b/packages/verified-fetch/src/utils/responses.ts @@ -1,3 +1,4 @@ +import { getContentRangeHeader } from './response-headers.js' import type { SupportedBodyTypes } from '../types.js' function setField (response: Response, name: string, value: string | boolean): void { @@ -98,3 +99,53 @@ export function movedPermanentlyResponse (url: string, location: string, init?: return response } + +/** + * Some caveats about range responses here: + * * We only support single range requests (multi-range is optional), see https://specs.ipfs.tech/http-gateways/path-gateway/#range-request-header + * * Range responses are only supported for unixfs and raw data, see https://specs.ipfs.tech/http-gateways/path-gateway/#range-request-header. + * + * If the user requests something other than unixfs or raw data, we should not call this method and ignore the range header (200 OK). See https://developer.mozilla.org/en-US/docs/Web/HTTP/Range_requests#partial_request_responses + * + * TODO: Supporting multiple range requests will require additional changes to the `handleDagPb` and `handleRaw` functions in `src/verified-fetch.js` + */ +export function okRangeResponse (url: string, range: { offset: number, length: number }, body: SupportedBodyTypes, init?: ResponseOptions): Response { + // if we know the full size of the body, we should use it in the content-range header. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range + const response = new Response(body, { + ...(init ?? {}), + status: 206, + statusText: 'Partial Content', + headers: { + ...(init?.headers ?? {}), + 'content-range': getContentRangeHeader(range.offset, range.length, { body }) + } + }) + + if (init?.redirected === true) { + setRedirected(response) + } + + setType(response, 'basic') + setUrl(response, url) + + return response +} + +/** + * We likely need to catch errors handled by upstream helia libraries if range-request throws an error. Some examples: + * * The range is out of bounds + * * The range is invalid + * * The range is not supported for the given type + */ +export function badRangeResponse (url: string, body?: SupportedBodyTypes, init?: ResponseInit): Response { + const response = new Response(body, { + ...(init ?? {}), + status: 416, + statusText: 'Requested Range Not Satisfiable' + }) + + setType(response, 'basic') + setUrl(response, url) + + return response +} diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index ca8c6dbc..40368bb3 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -22,13 +22,14 @@ import { getETag } from './utils/get-e-tag.js' import { getStreamFromAsyncIterable } from './utils/get-stream-from-async-iterable.js' import { tarStream } from './utils/get-tar-stream.js' import { parseResource } from './utils/parse-resource.js' -import { badRequestResponse, movedPermanentlyResponse, notAcceptableResponse, notSupportedResponse, okResponse } from './utils/responses.js' +import { getRequestRange } from './utils/request-headers.js' +import { badRequestResponse, movedPermanentlyResponse, notAcceptableResponse, notSupportedResponse, okResponse, badRangeResponse } from './utils/responses.js' import { selectOutputType, queryFormatToAcceptHeader } from './utils/select-output-type.js' import { walkPath } from './utils/walk-path.js' import type { CIDDetail, ContentTypeParser, Resource, VerifiedFetchInit as VerifiedFetchOptions } from './index.js' import type { RequestFormatShorthand } from './types.js' import type { Helia } from '@helia/interface' -import type { AbortOptions, Logger, PeerId } from '@libp2p/interface' +import type { AbortOptions, CodeError, Logger, PeerId } from '@libp2p/interface' import type { UnixFSEntry } from 'ipfs-unixfs-exporter' import type { CID } from 'multiformats/cid' @@ -280,13 +281,26 @@ export class VerifiedFetch { let terminalElement: UnixFSEntry | undefined let ipfsRoots: CID[] | undefined let redirected = false + const rangeOptions = getRequestRange(options?.headers) + const offset = rangeOptions?.offset + const length = rangeOptions?.length try { - const pathDetails = await walkPath(this.helia.blockstore, `${cid.toString()}/${path}`, options) + const pathDetails = await walkPath(this.helia.blockstore, `${cid.toString()}/${path}`, { ...options, offset, length }) ipfsRoots = pathDetails.ipfsRoots terminalElement = pathDetails.terminalElement } catch (err) { + this.log.error('RANGE TEST', err) this.log.error('Error walking path %s', path, err) + if ((err as CodeError)?.code === 'ERR_INVALID_PARAMS') { + /** + * the code is not explicit but seems to be the only error for invalid range requests + * + * @see https://github.com/search?q=repo%3Aipfs%2Fjs-ipfs-unixfs+ERR_INVALID_PARAMS+path%3Apackages%2Fipfs-unixfs-exporter&type=code + * @see https://github.com/ipfs/js-ipfs-unixfs/blob/4749d9a7c1eddd86b8fc42c3fa47f88c7b1b75ae/packages/ipfs-unixfs-exporter/src/utils/validate-offset-and-length.ts#L16-L30 + */ + return badRangeResponse(resource) + } } let resolvedCID = terminalElement?.cid ?? cid @@ -330,23 +344,33 @@ export class VerifiedFetch { const asyncIter = this.unixfs.cat(resolvedCID, { signal: options?.signal, - onProgress: options?.onProgress + onProgress: options?.onProgress, + offset, + length }) this.log('got async iterator for %c/%s', cid, path) - const { stream, firstChunk } = await getStreamFromAsyncIterable(asyncIter, path ?? '', this.helia.logger, { - onProgress: options?.onProgress - }) - const response = okResponse(resource, stream, { - redirected - }) - await this.setContentType(firstChunk, path, response) + try { + const { stream, firstChunk } = await getStreamFromAsyncIterable(asyncIter, path ?? '', this.helia.logger, { + onProgress: options?.onProgress + }) + const response = okResponse(resource, stream, { + redirected + }) + await this.setContentType(firstChunk, path, response) + + if (ipfsRoots != null) { + response.headers.set('X-Ipfs-Roots', ipfsRoots.map(cid => cid.toV1().toString()).join(',')) // https://specs.ipfs.tech/http-gateways/path-gateway/#x-ipfs-roots-response-header + } - if (ipfsRoots != null) { - response.headers.set('X-Ipfs-Roots', ipfsRoots.map(cid => cid.toV1().toString()).join(',')) // https://specs.ipfs.tech/http-gateways/path-gateway/#x-ipfs-roots-response-header + return response + } catch (err) { + this.log.error('Error streaming %c/%s', cid, path, err) + if ((err as CodeError)?.code === 'ERR_INVALID_PARAMS') { + return badRangeResponse(resource) + } + return notSupportedResponse('Unable to stream content') } - - return response } private async handleRaw ({ resource, cid, path, options }: FetchHandlerFunctionArg): Promise { @@ -388,7 +412,7 @@ export class VerifiedFetch { this.log.error('Error parsing content type', err) } } - + this.log.trace('setting content type to "%s"', contentType) response.headers.set('content-type', contentType) } @@ -466,12 +490,14 @@ export class VerifiedFetch { query.filename = query.filename ?? `${cid.toString()}.tar` response = await this.handleTar(handlerArgs) } else { + this.log.trace('Finding handler for cid code "%s" and output type "%s"', cid.code, accept) // derive the handler from the CID type const codecHandler = this.codecHandlers[cid.code] if (codecHandler == null) { return notSupportedResponse(`Support for codec with code ${cid.code} is not yet implemented. Please open an issue at https://github.com/ipfs/helia/issues/new`) } + this.log.trace('calling handler "%s"', codecHandler.name) response = await codecHandler.call(this, handlerArgs) } diff --git a/packages/verified-fetch/test/range-requests.spec.ts b/packages/verified-fetch/test/range-requests.spec.ts new file mode 100644 index 00000000..af4a5af0 --- /dev/null +++ b/packages/verified-fetch/test/range-requests.spec.ts @@ -0,0 +1,69 @@ +import { unixfs, type UnixFS } from '@helia/unixfs' +import { stop } from '@libp2p/interface' +import { expect } from 'aegir/chai' +import { type CID } from 'multiformats/cid' +import { VerifiedFetch } from '../src/verified-fetch.js' +import { createHelia } from './fixtures/create-offline-helia.js' +import type { Helia } from '@helia/interface' + +/** + * Range request headers for IPFS gateways only support raw and unixfs + */ +describe('range requests', () => { + let helia: Helia + let verifiedFetch: VerifiedFetch + let fs: UnixFS + let dagPbCid: CID + + beforeEach(async () => { + helia = await createHelia() + fs = unixfs(helia) + verifiedFetch = new VerifiedFetch({ + helia + }) + + dagPbCid = await fs.addFile({ + path: 'bigbuckbunny.mp4', + content: new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) + }, { + rawLeaves: false, + leafType: 'file' + }) + }) + + afterEach(async () => { + await stop(helia, verifiedFetch) + }) + + async function testRange (cid: CID, range: { start: number, end: number }): Promise { + const response = await verifiedFetch.fetch(cid, { + headers: { + Range: `bytes=${range.start}-${range.end}` + } + }) + + expect(response.status).to.equal(206) + expect(response.statusText).to.equal('Partial Content') + + expect(response).to.have.property('headers') + expect(response.headers).to.have.property('content-range') + expect(response.headers.get('content-range')).to.equal(`bytes ${range.start}-${range.end}/*`) // the response should include the range that was requested + + const content = await response.arrayBuffer() + expect(content.byteLength).to.equal(range.end - range.start + 1) // the length of the data should match the requested range + } + + it('should return 206 Partial Content with correct byte range for unixfs', async () => { + await expect(testRange(dagPbCid, { start: 0, end: 500 })).to.eventually.not.be.rejected() + }) + + it('should return 416 Range Not Satisfiable when the range is out of bounds', async () => { + const response = await verifiedFetch.fetch(dagPbCid, { + headers: { + Range: 'bytes=50-900' + } + }) + expect(response.status).to.equal(416) + expect(response.statusText).to.equal('Requested Range Not Satisfiable') + }) +}) From d805a5115cf9526895c6110666fc9a81ccdcacdb Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Mon, 4 Mar 2024 13:43:49 -0800 Subject: [PATCH 05/39] test: fix dns test asserting test failure since we are catching it now --- packages/verified-fetch/test/custom-dns-resolvers.spec.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/verified-fetch/test/custom-dns-resolvers.spec.ts b/packages/verified-fetch/test/custom-dns-resolvers.spec.ts index a37c5292..1b78d9ef 100644 --- a/packages/verified-fetch/test/custom-dns-resolvers.spec.ts +++ b/packages/verified-fetch/test/custom-dns-resolvers.spec.ts @@ -26,8 +26,7 @@ describe('custom dns-resolvers', () => { gateways: ['http://127.0.0.1:8080'], dnsResolvers: [customDnsResolver] }) - // error of walking the CID/dag because we haven't actually added the block to the blockstore - await expect(fetch('ipns://some-non-cached-domain.com')).to.eventually.be.rejected.with.property('errors') + await fetch('ipns://some-non-cached-domain.com') expect(customDnsResolver.callCount).to.equal(1) expect(customDnsResolver.getCall(0).args).to.deep.equal(['some-non-cached-domain.com', { onProgress: undefined }]) @@ -43,8 +42,8 @@ describe('custom dns-resolvers', () => { }, { dnsResolvers: [customDnsResolver] }) - // error of walking the CID/dag because we haven't actually added the block to the blockstore - await expect(verifiedFetch.fetch('ipns://some-non-cached-domain2.com')).to.eventually.be.rejected.with.property('errors').that.has.lengthOf(0) + + await verifiedFetch.fetch('ipns://some-non-cached-domain2.com') expect(customDnsResolver.callCount).to.equal(1) expect(customDnsResolver.getCall(0).args).to.deep.equal(['some-non-cached-domain2.com', { onProgress: undefined }]) From 4d8e57da1725fd80abb38ea9d66335259a5bf35c Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Mon, 4 Mar 2024 13:47:29 -0800 Subject: [PATCH 06/39] fix: return 500 error when streaming unixfs content throws --- packages/verified-fetch/src/utils/responses.ts | 13 +++++++++++++ packages/verified-fetch/src/verified-fetch.ts | 4 ++-- .../test/custom-dns-resolvers.spec.ts | 8 ++++++-- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/verified-fetch/src/utils/responses.ts b/packages/verified-fetch/src/utils/responses.ts index 64ab37cc..04a9df0a 100644 --- a/packages/verified-fetch/src/utils/responses.ts +++ b/packages/verified-fetch/src/utils/responses.ts @@ -43,6 +43,19 @@ export function okResponse (url: string, body?: SupportedBodyTypes, init?: Respo return response } +export function internalServerErrorResponse (url: string, body?: SupportedBodyTypes, init?: ResponseInit): Response { + const response = new Response(body, { + ...(init ?? {}), + status: 500, + statusText: 'Internal Server Error' + }) + + setType(response, 'basic') + setUrl(response, url) + + return response +} + export function notSupportedResponse (url: string, body?: SupportedBodyTypes, init?: ResponseInit): Response { const response = new Response(body, { ...(init ?? {}), diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index 40368bb3..47a46d92 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -23,7 +23,7 @@ import { getStreamFromAsyncIterable } from './utils/get-stream-from-async-iterab import { tarStream } from './utils/get-tar-stream.js' import { parseResource } from './utils/parse-resource.js' import { getRequestRange } from './utils/request-headers.js' -import { badRequestResponse, movedPermanentlyResponse, notAcceptableResponse, notSupportedResponse, okResponse, badRangeResponse } from './utils/responses.js' +import { badRequestResponse, movedPermanentlyResponse, notAcceptableResponse, notSupportedResponse, okResponse, badRangeResponse, internalServerErrorResponse } from './utils/responses.js' import { selectOutputType, queryFormatToAcceptHeader } from './utils/select-output-type.js' import { walkPath } from './utils/walk-path.js' import type { CIDDetail, ContentTypeParser, Resource, VerifiedFetchInit as VerifiedFetchOptions } from './index.js' @@ -369,7 +369,7 @@ export class VerifiedFetch { if ((err as CodeError)?.code === 'ERR_INVALID_PARAMS') { return badRangeResponse(resource) } - return notSupportedResponse('Unable to stream content') + return internalServerErrorResponse('Unable to stream content') } } diff --git a/packages/verified-fetch/test/custom-dns-resolvers.spec.ts b/packages/verified-fetch/test/custom-dns-resolvers.spec.ts index 1b78d9ef..49a17d2f 100644 --- a/packages/verified-fetch/test/custom-dns-resolvers.spec.ts +++ b/packages/verified-fetch/test/custom-dns-resolvers.spec.ts @@ -26,7 +26,9 @@ describe('custom dns-resolvers', () => { gateways: ['http://127.0.0.1:8080'], dnsResolvers: [customDnsResolver] }) - await fetch('ipns://some-non-cached-domain.com') + const response = await fetch('ipns://some-non-cached-domain.com') + expect(response.status).to.equal(500) + expect(response.statusText).to.equal('Internal Server Error') expect(customDnsResolver.callCount).to.equal(1) expect(customDnsResolver.getCall(0).args).to.deep.equal(['some-non-cached-domain.com', { onProgress: undefined }]) @@ -43,7 +45,9 @@ describe('custom dns-resolvers', () => { dnsResolvers: [customDnsResolver] }) - await verifiedFetch.fetch('ipns://some-non-cached-domain2.com') + const response = await verifiedFetch.fetch('ipns://some-non-cached-domain2.com') + expect(response.status).to.equal(500) + expect(response.statusText).to.equal('Internal Server Error') expect(customDnsResolver.callCount).to.equal(1) expect(customDnsResolver.getCall(0).args).to.deep.equal(['some-non-cached-domain2.com', { onProgress: undefined }]) From aa25f0c0ebf7f7a47af5469ae92cfd34e426dc2f Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Mon, 4 Mar 2024 17:12:59 -0800 Subject: [PATCH 07/39] fix: cleanup code and unexecuting tests hiding errors --- .../src/utils/request-headers.ts | 23 ++--- .../src/utils/response-headers.ts | 30 +++++- .../verified-fetch/src/utils/responses.ts | 14 ++- packages/verified-fetch/src/verified-fetch.ts | 93 +++++++++++++++--- .../test/range-requests.spec.ts | 95 ++++++++++++++++--- .../test/utils/request-headers.spec.ts | 17 ++-- .../test/utils/response-headers.spec.ts | 88 +++++++++++------ 7 files changed, 279 insertions(+), 81 deletions(-) diff --git a/packages/verified-fetch/src/utils/request-headers.ts b/packages/verified-fetch/src/utils/request-headers.ts index 919070d3..124f4209 100644 --- a/packages/verified-fetch/src/utils/request-headers.ts +++ b/packages/verified-fetch/src/utils/request-headers.ts @@ -27,6 +27,7 @@ export function getHeader (headers: HeadersInit | undefined, header: string): st * Type abstraction that will break the build if the supported range options change. */ export interface HeliaRangeOptions extends Pick { + suffixLength?: number } /** @@ -52,31 +53,26 @@ export function getRequestRange (headers: Headers | HeadersInit | undefined, siz return undefined } - // const match = range.match(/^bytes=(\d+)-(\d+)$/) /** - * Range: bytes=- - * Range: bytes=- - * Range: bytes=- + * Range: bytes=- | bytes=- | bytes=- */ const match = range.match(/^bytes=((?\d+)-(?\d+)|(?\d+)-|-(?\d+))$/) if (match?.groups == null) { throw new CodeError('ERR_INVALID_RANGE_REQUEST', 'Invalid range request') } const { start, end, start2, end2 } = match.groups - if (start2 != null && end2 != null) { - // this should never happen - throw new CodeError('ERR_INVALID_RANGE_REQUEST', 'Invalid range request') - } let offset: number | undefined let length: number | undefined + let suffixLength: number | undefined if (end2 != null) { if (size == null) { - throw new CodeError('ERR_HANDLING_RANGE_REQUEST', 'Cannot use suffix length without knowing the size of the resource') + suffixLength = Number(end2) + } else { + const stop = BigInt(end2) + offset = Number(size - stop) + length = Number(stop) } - const stop = BigInt(end2) - offset = Number(size - stop) - length = Number(stop) } else if (start2 != null) { offset = parseInt(start2) } else { @@ -86,6 +82,7 @@ export function getRequestRange (headers: Headers | HeadersInit | undefined, siz return { offset, - length + length, + suffixLength } } diff --git a/packages/verified-fetch/src/utils/response-headers.ts b/packages/verified-fetch/src/utils/response-headers.ts index 53bfd5af..84d175ea 100644 --- a/packages/verified-fetch/src/utils/response-headers.ts +++ b/packages/verified-fetch/src/utils/response-headers.ts @@ -1,3 +1,4 @@ +import { CodeError } from '@libp2p/interface' import type { SupportedBodyTypes } from '../types.js' /** @@ -7,15 +8,17 @@ function syncBodySize (body: SupportedBodyTypes): number | null { if (typeof body === 'string') { return body.length } - if (body instanceof ArrayBuffer) { + if (body instanceof ArrayBuffer || body instanceof Uint8Array) { return body.byteLength } if (body instanceof Blob) { return body.size } + if (body instanceof ReadableStream) { return null } + return null } @@ -23,9 +26,28 @@ function syncBodySize (body: SupportedBodyTypes): number | null { * This function returns the value of the `Content-Range` header for a given range. * If you know the total size of the body, you should pass it in the `options` object. * + * offset and length are 0-based, and the range is inclusive. + * * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range */ -export function getContentRangeHeader (start: number, end: number, options: { total?: number, body: SupportedBodyTypes }): string { - const total = options.total ?? syncBodySize(options.body) ?? '*' // if we don't know the total size, we should use * - return `bytes ${start}-${end}/${total}` +export function getContentRangeHeader ({ body, ...opts }: { offset?: number, length?: number, total?: number, body: SupportedBodyTypes }): string { + const totalSizeNum = opts.total ?? syncBodySize(body) + const rangeStart = opts.offset ?? 0 + let rangeEnd = opts.length + + if (rangeEnd == null) { + if (totalSizeNum == null) { + throw new CodeError('Cannot calculate content range without total size or length', 'ERR_INVALID_CONTENT_RANGE') + } + rangeEnd = totalSizeNum - rangeStart + 1 + } + let end: number + if (rangeStart != null && rangeEnd != null) { + end = rangeStart + rangeEnd - 1 + } else { + end = totalSizeNum ?? 0 + } + const total = totalSizeNum ?? '*' // if we don't know the total size, we should use * + + return `bytes ${rangeStart}-${end}/${total}` } diff --git a/packages/verified-fetch/src/utils/responses.ts b/packages/verified-fetch/src/utils/responses.ts index 04a9df0a..17577465 100644 --- a/packages/verified-fetch/src/utils/responses.ts +++ b/packages/verified-fetch/src/utils/responses.ts @@ -113,6 +113,8 @@ export function movedPermanentlyResponse (url: string, location: string, init?: return response } +type RangeOptions = { offset: number, length: number, totalSize?: number } | { offset: number, length?: number, totalSize: number } + /** * Some caveats about range responses here: * * We only support single range requests (multi-range is optional), see https://specs.ipfs.tech/http-gateways/path-gateway/#range-request-header @@ -122,15 +124,23 @@ export function movedPermanentlyResponse (url: string, location: string, init?: * * TODO: Supporting multiple range requests will require additional changes to the `handleDagPb` and `handleRaw` functions in `src/verified-fetch.js` */ -export function okRangeResponse (url: string, range: { offset: number, length: number }, body: SupportedBodyTypes, init?: ResponseOptions): Response { +export function okRangeResponse (url: string, body: SupportedBodyTypes, range: RangeOptions, init?: ResponseOptions): Response { // if we know the full size of the body, we should use it in the content-range header. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range + let contentRangeHeader: string | undefined + + try { + contentRangeHeader = getContentRangeHeader({ body, total: range.totalSize, offset: range.offset, length: range.length }) + } catch (e) { + return badRangeResponse(url, body, init) + } + const response = new Response(body, { ...(init ?? {}), status: 206, statusText: 'Partial Content', headers: { ...(init?.headers ?? {}), - 'content-range': getContentRangeHeader(range.offset, range.length, { body }) + 'content-range': contentRangeHeader } }) diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index 47a46d92..3da6f074 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -23,7 +23,7 @@ import { getStreamFromAsyncIterable } from './utils/get-stream-from-async-iterab import { tarStream } from './utils/get-tar-stream.js' import { parseResource } from './utils/parse-resource.js' import { getRequestRange } from './utils/request-headers.js' -import { badRequestResponse, movedPermanentlyResponse, notAcceptableResponse, notSupportedResponse, okResponse, badRangeResponse, internalServerErrorResponse } from './utils/responses.js' +import { badRequestResponse, movedPermanentlyResponse, notAcceptableResponse, notSupportedResponse, okResponse, badRangeResponse, internalServerErrorResponse, okRangeResponse } from './utils/responses.js' import { selectOutputType, queryFormatToAcceptHeader } from './utils/select-output-type.js' import { walkPath } from './utils/walk-path.js' import type { CIDDetail, ContentTypeParser, Resource, VerifiedFetchInit as VerifiedFetchOptions } from './index.js' @@ -277,22 +277,36 @@ export class VerifiedFetch { return response } + // eslint-disable-next-line complexity private async handleDagPb ({ cid, path, resource, options }: FetchHandlerFunctionArg): Promise { let terminalElement: UnixFSEntry | undefined let ipfsRoots: CID[] | undefined let redirected = false - const rangeOptions = getRequestRange(options?.headers) - const offset = rangeOptions?.offset - const length = rangeOptions?.length + let offset: number | undefined + let length: number | undefined + let suffixLength: number | undefined + let isRangeRequest = false + try { + const rangeOptions = getRequestRange(options?.headers) + offset = rangeOptions?.offset + length = rangeOptions?.length + suffixLength = rangeOptions?.suffixLength + this.log.trace('range options %o', rangeOptions) + if (offset != null || length != null || suffixLength != null) { + isRangeRequest = true + } + } catch (err) { + this.log.error('Error parsing range request', err) + return badRangeResponse('Invalid range request') + } try { const pathDetails = await walkPath(this.helia.blockstore, `${cid.toString()}/${path}`, { ...options, offset, length }) ipfsRoots = pathDetails.ipfsRoots terminalElement = pathDetails.terminalElement } catch (err) { - this.log.error('RANGE TEST', err) this.log.error('Error walking path %s', path, err) - if ((err as CodeError)?.code === 'ERR_INVALID_PARAMS') { + if (isRangeRequest && (err as CodeError)?.code === 'ERR_INVALID_PARAMS') { /** * the code is not explicit but seems to be the only error for invalid range requests * @@ -301,10 +315,12 @@ export class VerifiedFetch { */ return badRangeResponse(resource) } + + return internalServerErrorResponse('Error walking path') } let resolvedCID = terminalElement?.cid ?? cid - let stat: UnixFSStats + let stat: UnixFSStats | undefined if (terminalElement?.type === 'directory') { const dirCid = terminalElement.cid @@ -342,6 +358,51 @@ export class VerifiedFetch { } } + /** + * range-request validation. This shouldn't be required, but unixfs.cat isn't throwing in some cases. + */ + if (isRangeRequest) { + /** + * if requested CID was a directory, we already have a unixfs stat object + * if requested CID was a file or raw, we need to fetch the stat object + */ + stat = stat ?? await this.unixfs.stat(resolvedCID, { + signal: options?.signal, + onProgress: options?.onProgress + }) + + if (suffixLength != null) { + const rangeWithSize = getRequestRange(options?.headers, stat.fileSize) + if (rangeWithSize != null) { + offset = rangeWithSize.offset + length = rangeWithSize.length + } + } + + if (offset != null) { + if (offset < 0) { + // the requested range start is negative + this.log.error('range start %d is negative', offset) + return badRangeResponse(resource) + } else if (stat.fileSize < offset) { + // the requested range start is larger than the requested content size + this.log.error('range start %d is larger than the requested content size %d', offset, stat.fileSize) + return badRangeResponse(resource) + } else if (length != null && Number(stat.fileSize) < (offset + length)) { + // the requested range is larger than the requested content size + this.log.error('range end %d is larger than the requested content size %d', offset + length, stat.fileSize) + return badRangeResponse(resource) + } + } + if (length != null) { + if (Number(stat.fileSize) < length) { + // the requested range is larger than the requested content size + this.log.error('range end %d is larger than the requested content size %d', length, stat.fileSize) + return badRangeResponse(resource) + } + } + } + const asyncIter = this.unixfs.cat(resolvedCID, { signal: options?.signal, onProgress: options?.onProgress, @@ -354,9 +415,19 @@ export class VerifiedFetch { const { stream, firstChunk } = await getStreamFromAsyncIterable(asyncIter, path ?? '', this.helia.logger, { onProgress: options?.onProgress }) - const response = okResponse(resource, stream, { - redirected - }) + let response: Response + if (isRangeRequest) { + // type assertion says stat may still be undefined, but we've set it again in the above check for isRangeRequest + const totalSize = Number(stat?.fileSize) + offset = offset ?? 0 + response = okRangeResponse(resource, stream, { offset, length, totalSize }, { + redirected + }) + } else { + response = okResponse(resource, stream, { + redirected + }) + } await this.setContentType(firstChunk, path, response) if (ipfsRoots != null) { @@ -366,7 +437,7 @@ export class VerifiedFetch { return response } catch (err) { this.log.error('Error streaming %c/%s', cid, path, err) - if ((err as CodeError)?.code === 'ERR_INVALID_PARAMS') { + if (isRangeRequest && (err as CodeError)?.code === 'ERR_INVALID_PARAMS') { return badRangeResponse(resource) } return internalServerErrorResponse('Unable to stream content') diff --git a/packages/verified-fetch/test/range-requests.spec.ts b/packages/verified-fetch/test/range-requests.spec.ts index af4a5af0..8a83fa3c 100644 --- a/packages/verified-fetch/test/range-requests.spec.ts +++ b/packages/verified-fetch/test/range-requests.spec.ts @@ -35,10 +35,14 @@ describe('range requests', () => { await stop(helia, verifiedFetch) }) - async function testRange (cid: CID, range: { start: number, end: number }): Promise { + interface SuccessfulTestExpectation { + contentRange?: string + byteSize?: number + } + async function testRange (cid: CID, headerRange: string, expected: SuccessfulTestExpectation): Promise { const response = await verifiedFetch.fetch(cid, { headers: { - Range: `bytes=${range.start}-${range.end}` + Range: headerRange } }) @@ -46,24 +50,87 @@ describe('range requests', () => { expect(response.statusText).to.equal('Partial Content') expect(response).to.have.property('headers') - expect(response.headers).to.have.property('content-range') - expect(response.headers.get('content-range')).to.equal(`bytes ${range.start}-${range.end}/*`) // the response should include the range that was requested + const contentRange = response.headers.get('content-range') + expect(contentRange).to.be.ok() + expect(contentRange).to.equal(expected.contentRange) // the response should include the range that was requested const content = await response.arrayBuffer() - expect(content.byteLength).to.equal(range.end - range.start + 1) // the length of the data should match the requested range + expect(content.byteLength).to.equal(expected.byteSize) // the length of the data should match the requested range } - it('should return 206 Partial Content with correct byte range for unixfs', async () => { - await expect(testRange(dagPbCid, { start: 0, end: 500 })).to.eventually.not.be.rejected() - }) + async function assertFailingRange (response: Promise): Promise { + await expect(response).to.eventually.have.property('status', 416) + await expect(response).to.eventually.have.property('statusText', 'Requested Range Not Satisfiable') + } - it('should return 416 Range Not Satisfiable when the range is out of bounds', async () => { - const response = await verifiedFetch.fetch(dagPbCid, { - headers: { - Range: 'bytes=50-900' + describe('unixfs', () => { + it('should return correct 206 Partial Content response for byte=-', async () => { + const expected: SuccessfulTestExpectation = { + byteSize: 6, + contentRange: 'bytes 0-5/11' + } + await testRange(dagPbCid, 'bytes=0-5', expected) + }) + + it('should return correct 206 Partial Content response for byte=-', async () => { + const expected = { + byteSize: 7, + contentRange: 'bytes 4-11/11' + } + await testRange(dagPbCid, 'bytes=4-', expected) + }) + + it('should return correct 206 Partial Content response for byte=-', async () => { + const expected = { + byteSize: 9, + contentRange: 'bytes 2-10/11' } + await testRange(dagPbCid, 'bytes=-9', expected) + }) + + it('should return 416 Range Not Satisfiable when the range is invalid', async () => { + await assertFailingRange(verifiedFetch.fetch(dagPbCid, { + headers: { + Range: 'bytes=-0-' + } + })) + await assertFailingRange(verifiedFetch.fetch(dagPbCid, { + headers: { + Range: 'bytes=foobar' + } + })) + }) + + it('should return 416 Range Not Satisfiable when the range offset is larger than content', async () => { + await assertFailingRange(verifiedFetch.fetch(dagPbCid, { + headers: { + Range: 'bytes=50-' + } + })) + }) + + it('should return 416 Range Not Satisfiable when the suffix-length is larger than content', async () => { + await assertFailingRange(verifiedFetch.fetch(dagPbCid, { + headers: { + Range: 'bytes=-50' + } + })) + }) + + it('should return 416 Range Not Satisfiable when the range is out of bounds', async () => { + await assertFailingRange(verifiedFetch.fetch(dagPbCid, { + headers: { + Range: 'bytes=0-900' + } + })) + }) + + it('should return 416 Range Not Satisfiable when passed multiple ranges', async () => { + await assertFailingRange(verifiedFetch.fetch(dagPbCid, { + headers: { + Range: 'bytes=0-2,3-5' + } + })) }) - expect(response.status).to.equal(416) - expect(response.statusText).to.equal('Requested Range Not Satisfiable') }) }) diff --git a/packages/verified-fetch/test/utils/request-headers.spec.ts b/packages/verified-fetch/test/utils/request-headers.spec.ts index fb90bae1..12d7b730 100644 --- a/packages/verified-fetch/test/utils/request-headers.spec.ts +++ b/packages/verified-fetch/test/utils/request-headers.spec.ts @@ -44,25 +44,24 @@ describe('request-headers', () => { }) it('should return correct range options when a valid Range header has start only', () => { - const headers = new Headers({ Range: 'bytes=2-' }) - const result = getRequestRange(headers) - expect(result).to.deep.equal({ offset: 2, length: undefined }) + const result = getRequestRange(new Headers({ Range: 'bytes=2-' })) + expect(result).to.deep.equal({ offset: 2, length: undefined, suffixLength: undefined }) }) it('should return correct range options when a valid Range header has start and end', () => { - const headers = new Headers({ Range: 'bytes=0-500' }) - const result = getRequestRange(headers) - expect(result).to.deep.equal({ offset: 0, length: 501 }) + const result = getRequestRange(new Headers({ Range: 'bytes=0-500' })) + expect(result).to.deep.equal({ offset: 0, length: 501, suffixLength: undefined }) }) - it('should throw when Range header has end only and size is not passed', () => { - expect(() => getRequestRange(new Headers({ Range: 'bytes=-20' }))).to.throw('ERR_HANDLING_RANGE_REQUEST') + it('should return only suffixLength when not passed range-start nor size', () => { + const result = getRequestRange(new Headers({ Range: 'bytes=-20' })) + expect(result).to.deep.equal({ offset: undefined, length: undefined, suffixLength: 20 }) }) it('should return correct range options when a valid Range header has end only', () => { const headers = new Headers({ Range: 'bytes=-25' }) const result = getRequestRange(headers, 100n) - expect(result).to.deep.equal({ offset: 75, length: 25 }) + expect(result).to.deep.equal({ offset: 75, length: 25, suffixLength: undefined }) }) }) }) diff --git a/packages/verified-fetch/test/utils/response-headers.spec.ts b/packages/verified-fetch/test/utils/response-headers.spec.ts index 6906db31..dd45a198 100644 --- a/packages/verified-fetch/test/utils/response-headers.spec.ts +++ b/packages/verified-fetch/test/utils/response-headers.spec.ts @@ -3,53 +3,85 @@ import { getContentRangeHeader } from '../../src/utils/response-headers.js' describe('response-headers', () => { describe('getContentRangeHeader', () => { - it('should return correct content range header when total is provided', () => { - const start = 0 - const end = 500 - const total = 1000 - const body = 'dummy body' - const result = getContentRangeHeader(start, end, { total, body }) - expect(result).to.equal(`bytes ${start}-${end}/${total}`) + const body = 'dummy body' + const bodySize = body.length + it('should return correct content range header when only given offset', () => { + const offset = 2 + const result = getContentRangeHeader({ body, offset }) + expect(result).to.equal(`bytes 2-${bodySize}/${bodySize}`) + }) + + it('should return correct content range header when only given length', () => { + const length = 2 + const result = getContentRangeHeader({ body, length }) + expect(result).to.equal(`bytes 0-1/${bodySize}`) + }) + + it('should override byte-size when total is provided', () => { + const offset = 1 + const length = 2 + const total = 4 + const result = getContentRangeHeader({ total, body, offset, length }) + expect(result).to.equal('bytes 1-2/4') }) it('should return correct content range header when total is not provided', () => { - const start = 0 - const end = 500 - const body = 'dummy body' - const result = getContentRangeHeader(start, end, { body }) - expect(result).to.equal(`bytes ${start}-${end}/${body.length}`) + const offset = 0 + const length = 3 + const result = getContentRangeHeader({ body, offset, length }) + expect(result).to.equal(`bytes 0-2/${body.length}`) }) it('should return content range header with * when total and body size are not known', () => { - const start = 0 - const end = 500 + const offset = 1 + const length = 4 const body = null - const result = getContentRangeHeader(start, end, { body }) - expect(result).to.equal(`bytes ${start}-${end}/*`) + const result = getContentRangeHeader({ body, offset, length }) + expect(result).to.equal('bytes 1-4/*') }) it('should return content range header with * body is a ReadableStream', () => { - const start = 0 - const end = 500 + const offset = 5 + const length = 500 const body = new ReadableStream() - const result = getContentRangeHeader(start, end, { body }) - expect(result).to.equal(`bytes ${start}-${end}/*`) + const result = getContentRangeHeader({ body, offset, length }) + expect(result).to.equal('bytes 5-504/*') }) it('should return content range header with the correct arrayBuffer size', () => { - const start = 0 - const end = 500 + const offset = 6 + const length = 40 const body = new ArrayBuffer(1000) - const result = getContentRangeHeader(start, end, { body }) - expect(result).to.equal(`bytes ${start}-${end}/${body.byteLength}`) + const result = getContentRangeHeader({ body, offset, length }) + expect(result).to.equal(`bytes 6-45/${body.byteLength}`) }) it('should return content range header with the correct Blob size', () => { - const start = 0 - const end = 500 + const offset = 7 + const length = 2 const body = new Blob(['dummy body']) - const result = getContentRangeHeader(start, end, { body }) - expect(result).to.equal(`bytes ${start}-${end}/${body.size}`) + const result = getContentRangeHeader({ body, offset, length }) + expect(result).to.equal(`bytes 7-8/${body.size}`) + }) + + it('should return content range header with the correct Uint8Array size', () => { + const offset = 2 + const length = 9 + const body = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) + const result = getContentRangeHeader({ body, offset, length }) + expect(result).to.equal('bytes 2-10/11') + }) + + it('should return content range header with the correct Uint8Array size & explicit total', () => { + const offset = 4 + const body = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) + const result = getContentRangeHeader({ body, offset, total: 11 }) + expect(result).to.equal('bytes 4-11/11') + }) + + it('should return content range header with offset, length, & total', () => { + const result = getContentRangeHeader({ body: null, offset: 2, length: 9, total: 11 }) + expect(result).to.equal('bytes 2-10/11') }) }) }) From 60b56c9f66efa24e8fe943a13e4f2d76e0b80964 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Mon, 4 Mar 2024 17:35:45 -0800 Subject: [PATCH 08/39] chore: some cleanup and code coverage --- .../src/utils/request-headers.ts | 12 ++--- packages/verified-fetch/src/verified-fetch.ts | 50 +++++++++++-------- .../test/utils/request-headers.spec.ts | 6 ++- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/packages/verified-fetch/src/utils/request-headers.ts b/packages/verified-fetch/src/utils/request-headers.ts index 124f4209..109bae8f 100644 --- a/packages/verified-fetch/src/utils/request-headers.ts +++ b/packages/verified-fetch/src/utils/request-headers.ts @@ -13,14 +13,12 @@ export function getHeader (headers: HeadersInit | undefined, header: string): st const entry = headers.find(([key]) => key.toLowerCase() === header.toLowerCase()) return entry?.[1] } - if (typeof headers === 'object') { - const key = Object.keys(headers).find(k => k.toLowerCase() === header.toLowerCase()) - if (key == null) { - return undefined - } - return headers[key] + const key = Object.keys(headers).find(k => k.toLowerCase() === header.toLowerCase()) + if (key == null) { + return undefined } - return headers[header] + + return headers[key] } /** diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index 3da6f074..60cde747 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -378,28 +378,9 @@ export class VerifiedFetch { length = rangeWithSize.length } } - - if (offset != null) { - if (offset < 0) { - // the requested range start is negative - this.log.error('range start %d is negative', offset) - return badRangeResponse(resource) - } else if (stat.fileSize < offset) { - // the requested range start is larger than the requested content size - this.log.error('range start %d is larger than the requested content size %d', offset, stat.fileSize) - return badRangeResponse(resource) - } else if (length != null && Number(stat.fileSize) < (offset + length)) { - // the requested range is larger than the requested content size - this.log.error('range end %d is larger than the requested content size %d', offset + length, stat.fileSize) - return badRangeResponse(resource) - } - } - if (length != null) { - if (Number(stat.fileSize) < length) { - // the requested range is larger than the requested content size - this.log.error('range end %d is larger than the requested content size %d', length, stat.fileSize) - return badRangeResponse(resource) - } + const resp = this.checkForInvalidRangeRequest({ stat, offset, length, resource }) + if (resp != null) { + return resp } } @@ -487,6 +468,31 @@ export class VerifiedFetch { response.headers.set('content-type', contentType) } + private checkForInvalidRangeRequest ({ offset, length, resource, stat }: { offset?: number, length?: number, resource: string, stat: UnixFSStats }): Response | undefined { + if (offset != null) { + if (offset < 0) { + // the requested range start is negative + this.log.error('range start %d is negative', offset) + return badRangeResponse(resource) + } else if (stat.fileSize < offset) { + // the requested range start is larger than the requested content size + this.log.error('range start %d is larger than the requested content size %d', offset, stat.fileSize) + return badRangeResponse(resource) + } else if (length != null && Number(stat.fileSize) < (offset + length)) { + // the requested range is larger than the requested content size + this.log.error('range end %d is larger than the requested content size %d', offset + length, stat.fileSize) + return badRangeResponse(resource) + } + } + if (length != null) { + if (Number(stat.fileSize) < length) { + // the requested range is larger than the requested content size + this.log.error('range end %d is larger than the requested content size %d', length, stat.fileSize) + return badRangeResponse(resource) + } + } + } + /** * If the user has not specified an Accept header or format query string arg, * use the CID codec to choose an appropriate handler for the block data. diff --git a/packages/verified-fetch/test/utils/request-headers.spec.ts b/packages/verified-fetch/test/utils/request-headers.spec.ts index 12d7b730..9bfbbae3 100644 --- a/packages/verified-fetch/test/utils/request-headers.spec.ts +++ b/packages/verified-fetch/test/utils/request-headers.spec.ts @@ -4,8 +4,10 @@ import { getHeader, getRequestRange } from '../../src/utils/request-headers.js' describe('request-headers', () => { describe('getHeader', () => { it('should return undefined when headers are undefined', () => { - const result = getHeader(undefined, 'dummy') - expect(result).to.be.undefined() + expect(getHeader(undefined, 'dummy')).to.be.undefined() + expect(getHeader(new Headers(), 'dummy')).to.be.undefined() + expect(getHeader({}, 'dummy')).to.be.undefined() + expect(getHeader([], 'dummy')).to.be.undefined() }) it('should return correct header value for Headers instance', () => { From 6da36fd3b00cbd3a4aaec727b691203377c3c690 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Tue, 5 Mar 2024 12:37:05 -0800 Subject: [PATCH 09/39] tmp: most things working --- .../src/utils/byte-range-context.ts | 306 ++++++++++++++++++ .../src/utils/request-headers.ts | 68 ---- .../src/utils/response-headers.ts | 53 --- .../verified-fetch/src/utils/responses.ts | 56 ++-- packages/verified-fetch/src/verified-fetch.ts | 127 ++------ .../test/range-requests.spec.ts | 169 +++++----- .../test/utils/byte-range-context.spec.ts | 81 +++++ .../test/utils/request-headers.spec.ts | 40 +-- .../test/utils/response-headers.spec.ts | 87 ----- 9 files changed, 543 insertions(+), 444 deletions(-) create mode 100644 packages/verified-fetch/src/utils/byte-range-context.ts delete mode 100644 packages/verified-fetch/src/utils/response-headers.ts create mode 100644 packages/verified-fetch/test/utils/byte-range-context.spec.ts delete mode 100644 packages/verified-fetch/test/utils/response-headers.spec.ts diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts new file mode 100644 index 00000000..c056bc1d --- /dev/null +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -0,0 +1,306 @@ +import { getHeader } from './request-headers.js' +import type { SupportedBodyTypes } from '../types.js' +import type { ComponentLogger, Logger } from '@libp2p/logger' + +/** + * Gets the body size of a given body if it's possible to calculate it synchronously. + */ +function getBodySizeSync (body: SupportedBodyTypes): number | null { + if (typeof body === 'string') { + return body.length + } + if (body instanceof ArrayBuffer || body instanceof Uint8Array) { + return body.byteLength + } + if (body instanceof Blob) { + return body.size + } + + if (body instanceof ReadableStream) { + return null + } + + return null +} + +function getByteRangeFromHeader (rangeHeader: string): { start: string, end: string } { + /** + * Range: bytes=- | bytes=- | bytes=- + */ + const match = rangeHeader.match(/^bytes=(?\d+)?-(?\d+)?$/) + if (match?.groups == null) { + throw new Error('Invalid range request') + } + + const { start, end } = match.groups + + return { start, end } +} + +export class ByteRangeContext { + private readonly _isRangeRequest: boolean + private _fileSize: number | null | undefined + private _contentRangeHeaderValue: string | undefined + private _body: SupportedBodyTypes | null = null + private readonly _rangeRequestHeader: string | undefined + private readonly log: Logger + private _isValidRangeRequest: boolean | null = null + private _offset: number | undefined = undefined + private _length: number | undefined = undefined + private readonly requestRangeStart!: string | undefined + private readonly requestRangeEnd!: string | undefined + private responseRangeStart!: number + private responseRangeEnd!: number + + constructor (logger: ComponentLogger, private readonly headers?: HeadersInit) { + this.log = logger.forComponent('helia:verified-fetch:byte-range-context') + this._rangeRequestHeader = getHeader(this.headers, 'Range') + if (this._rangeRequestHeader != null) { + this.log.trace('Range request detected') + this._isRangeRequest = true + try { + const { start, end } = getByteRangeFromHeader(this._rangeRequestHeader) + this.requestRangeStart = start + this.requestRangeEnd = end + } catch (e) { + this.log.error('error parsing range request header: %o', e) + this.isValidRangeRequest = false + } + + this.setOffsetDetails() + } else { + this.log.trace('No range request detected') + this._isRangeRequest = false + this.requestRangeStart = undefined + this.requestRangeEnd = undefined + } + + this.log.trace('created ByteRangeContext with headers %o. is range request? %s, is valid range? %s', this.headers, this._isRangeRequest, this.isValidRangeRequest) + } + + public set body (body: SupportedBodyTypes) { + this._body = body + // if fileSize was set explicitly or already set, don't recalculate it + this.fileSize = this.fileSize ?? getBodySizeSync(body) + this.setOffsetDetails() + + this.log.trace('set request body with fileSize %o', this._fileSize) + } + + public get body (): SupportedBodyTypes { + this.log.trace('getting body') + const body = this._body + if (body == null) { + this.log.trace('body is null') + return body + } + if (!this.isRangeRequest || !this.isValidRangeRequest) { + this.log.trace('returning body without offset or length') + return body + } + const offset = this.offset + const length = this.length + if (offset != null || length != null) { + this.log.trace('returning body with offset %o and length %o', offset, length) + if (body instanceof Uint8Array) { + this.log.trace('body is Uint8Array') + return this.getSlicedBody(body, offset, length) + } else if (body instanceof ArrayBuffer) { + return this.getSlicedBody(body, offset, length) + } else if (body instanceof Blob) { + return this.getSlicedBody(body, offset, length) + } else if (body instanceof ReadableStream) { + // TODO: if offset and length is not null, will the browser handle this? + return this.getSlicedStream(body, offset, length) + } + } + // offset and length are not set, so not a range request, return body untouched. + return body + } + + private getSlicedBody (body: T, offset: number | undefined, length: number | undefined): T { + // const bodySize = body instanceof Blob ? body.size : body.byteLength + this.log.trace('slicing body with offset %o and length %o', offset, length) + try { + if (offset != null && length != null) { + return body.slice(offset, offset + length) as T + } else if (offset != null) { + return body.slice(offset) as T + } else if (length != null) { + return body.slice(0, length + 1) as T + } else { + return body + } + } catch (e) { + this.log.error('error slicing Uint8Array: %o', e) + throw e + } + } + + /** + * If a user requests a range of bytes from a ReadableStream, we need to read the stream and enqueue only the requested bytes. + */ + private getSlicedStream (body: ReadableStream, offset: number | undefined, length: number | undefined): ReadableStream { + const reader = body.getReader() + return new ReadableStream({ + // async start (controller) { + // if (offset != null) { + // // skip bytes until we reach the offset + // let bytesRead = 0 + // while (bytesRead < offset) { + // const { value, done } = await reader.read() + // if (done) { + // break + // } + // bytesRead += value.byteLength + // } + // } + // }, + async pull (controller) { + if (length == null) { + const { value, done } = await reader.read() + if (done) { + controller.close() + return + } + controller.enqueue(value) + return + } + let bytesRead = 0 + let bytesRemaining = length + while (bytesRead < length) { + const { value, done } = await reader.read() + if (done) { + controller.close() + return + } + if (value.byteLength > bytesRemaining) { + controller.enqueue(value.slice(0, bytesRemaining)) + bytesRead += bytesRemaining + return + } + bytesRead += value.byteLength + bytesRemaining -= value.byteLength + controller.enqueue(value) + } + } + }) + } + + public set fileSize (size: number | bigint | null) { + this._fileSize = size != null ? Number(size) : null + } + + public get fileSize (): number | null | undefined { + return this._fileSize + } + + public get isRangeRequest (): boolean { + return this._isRangeRequest + } + + public set isValidRangeRequest (val: boolean) { + this._isValidRangeRequest = val + } + + public get isValidRangeRequest (): boolean { + if (this.length != null && this.length < 0) { + this._isValidRangeRequest = false + } else if (this.offset != null && this.offset < 0) { + this._isValidRangeRequest = false + } else if (this.length != null && this._fileSize != null && this.length > this._fileSize) { + this._isValidRangeRequest = false + } else if (this.requestRangeStart != null && this.requestRangeEnd != null) { + if (parseInt(this.requestRangeStart) > parseInt(this.requestRangeEnd)) { + this._isValidRangeRequest = false + } + } + this._isValidRangeRequest = this._isValidRangeRequest ?? true + + return this._isValidRangeRequest + } + + private set offset (val: number | undefined) { + this._offset = this._offset ?? val + this.log.trace('set _offset to %o', this._offset) + } + + public get offset (): number | undefined { + return this._offset + } + + private set length (val: number | undefined) { + this._length = val + this.log.trace('set _length to %o', this._length) + } + + public get length (): number | undefined { + return this._length + } + + /** + * Converts a range request header into helia/unixfs supported range options + * Note that the gateway specification says we "MAY" support multiple ranges (https://specs.ipfs.tech/http-gateways/path-gateway/#range-request-header) but we don't + * + * Also note that @helia/unixfs and ipfs-unixfs-exporter expect length and offset to be numbers, the range header is a string, and the size of the resource is likely a bigint. + * + * SUPPORTED: + * Range: bytes=- + * Range: bytes=- + * Range: bytes=- // must pass size so we can calculate the offset. suffix-length is the number of bytes from the end of the file. + * + * NOT SUPPORTED: + * Range: bytes=-, - + * Range: bytes=-, -, - + * + * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Range#directives + */ + private setOffsetDetails (): void { + if (this.requestRangeStart == null && this.requestRangeEnd == null) { + this.log.trace('requestRangeStart and requestRangeEnd are null') + return + } + this.log.trace('requestRangeStart %o, requestRangeEnd %o', this.requestRangeStart, this.requestRangeEnd) + if (this.requestRangeStart != null && this.requestRangeEnd != null) { + // we have a specific rangeRequest start & end + this.offset = parseInt(this.requestRangeStart) + this.length = parseInt(this.requestRangeEnd) - this.offset + 1 + this.responseRangeStart = this.offset + this.responseRangeEnd = this.offset + this.length - 1 + } else if (this.requestRangeStart != null) { + // we have a specific rangeRequest start + this.offset = parseInt(this.requestRangeStart) + this.responseRangeStart = this.offset + if (this._fileSize != null) { + this.length = this._fileSize - this.offset + 1 + this.responseRangeEnd = this.offset + this.length - 1 + } else { + this.length = undefined + } + } else if (this.requestRangeEnd != null && this._fileSize != null) { + this.log.trace('requestRangeEnd %o, fileSize %o', this.requestRangeEnd, this._fileSize) + // we have a specific rangeRequest end (suffix-length) & filesize + const lengthRequested = parseInt(this.requestRangeEnd) + // if the user requested length of N, the offset is N bytes from the end of the file + this.offset = this._fileSize - lengthRequested + this.length = lengthRequested + this.responseRangeStart = this.offset // inclusive + this.responseRangeEnd = this._fileSize // inclusive + } else { + this.log.trace('Not enough information to set offset and length') + } + } + + public get contentRangeHeaderValue (): string { + if (this._contentRangeHeaderValue != null) { + return this._contentRangeHeaderValue + } + if (!this.isValidRangeRequest) { + this.log.error('cannot get contentRangeHeaderValue for invalid range request') + throw new Error('Invalid range request') + } + const fileSize = this._fileSize ?? '*' + this._contentRangeHeaderValue = `bytes ${this.responseRangeStart}-${this.responseRangeEnd}/${fileSize}` + return this._contentRangeHeaderValue + } +} diff --git a/packages/verified-fetch/src/utils/request-headers.ts b/packages/verified-fetch/src/utils/request-headers.ts index 109bae8f..1686f54d 100644 --- a/packages/verified-fetch/src/utils/request-headers.ts +++ b/packages/verified-fetch/src/utils/request-headers.ts @@ -1,7 +1,3 @@ -import { type CatOptions } from '@helia/unixfs' -import { CodeError } from '@libp2p/interface' -import { type ExporterOptions } from 'ipfs-unixfs-exporter' - export function getHeader (headers: HeadersInit | undefined, header: string): string | undefined { if (headers == null) { return undefined @@ -20,67 +16,3 @@ export function getHeader (headers: HeadersInit | undefined, header: string): st return headers[key] } - -/** - * Type abstraction that will break the build if the supported range options change. - */ -export interface HeliaRangeOptions extends Pick { - suffixLength?: number -} - -/** - * Converts a range request header into helia/unixfs supported range options - * Note that the gateway specification says we "MAY" support multiple ranges (https://specs.ipfs.tech/http-gateways/path-gateway/#range-request-header) but we don't - * - * Also note that @helia/unixfs and ipfs-unixfs-exporter expect length and offset to be numbers, the range header is a string, and the size of the resource is likely a bigint. - * - * SUPPORTED: - * Range: bytes=- - * Range: bytes=- - * Range: bytes=- // must pass size so we can calculate the offset - * - * NOT SUPPORTED: - * Range: bytes=-, - - * Range: bytes=-, -, - - * - * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Range#directives - */ -export function getRequestRange (headers: Headers | HeadersInit | undefined, size?: bigint): HeliaRangeOptions | undefined { - const range = getHeader(headers, 'Range') - if (range == null) { - return undefined - } - - /** - * Range: bytes=- | bytes=- | bytes=- - */ - const match = range.match(/^bytes=((?\d+)-(?\d+)|(?\d+)-|-(?\d+))$/) - if (match?.groups == null) { - throw new CodeError('ERR_INVALID_RANGE_REQUEST', 'Invalid range request') - } - const { start, end, start2, end2 } = match.groups - - let offset: number | undefined - let length: number | undefined - let suffixLength: number | undefined - if (end2 != null) { - if (size == null) { - suffixLength = Number(end2) - } else { - const stop = BigInt(end2) - offset = Number(size - stop) - length = Number(stop) - } - } else if (start2 != null) { - offset = parseInt(start2) - } else { - offset = parseInt(start) - length = parseInt(end) - offset + 1 - } - - return { - offset, - length, - suffixLength - } -} diff --git a/packages/verified-fetch/src/utils/response-headers.ts b/packages/verified-fetch/src/utils/response-headers.ts deleted file mode 100644 index 84d175ea..00000000 --- a/packages/verified-fetch/src/utils/response-headers.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { CodeError } from '@libp2p/interface' -import type { SupportedBodyTypes } from '../types.js' - -/** - * Gets the body size of a given body if it's possible to calculate it synchronously. - */ -function syncBodySize (body: SupportedBodyTypes): number | null { - if (typeof body === 'string') { - return body.length - } - if (body instanceof ArrayBuffer || body instanceof Uint8Array) { - return body.byteLength - } - if (body instanceof Blob) { - return body.size - } - - if (body instanceof ReadableStream) { - return null - } - - return null -} - -/** - * This function returns the value of the `Content-Range` header for a given range. - * If you know the total size of the body, you should pass it in the `options` object. - * - * offset and length are 0-based, and the range is inclusive. - * - * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range - */ -export function getContentRangeHeader ({ body, ...opts }: { offset?: number, length?: number, total?: number, body: SupportedBodyTypes }): string { - const totalSizeNum = opts.total ?? syncBodySize(body) - const rangeStart = opts.offset ?? 0 - let rangeEnd = opts.length - - if (rangeEnd == null) { - if (totalSizeNum == null) { - throw new CodeError('Cannot calculate content range without total size or length', 'ERR_INVALID_CONTENT_RANGE') - } - rangeEnd = totalSizeNum - rangeStart + 1 - } - let end: number - if (rangeStart != null && rangeEnd != null) { - end = rangeStart + rangeEnd - 1 - } else { - end = totalSizeNum ?? 0 - } - const total = totalSizeNum ?? '*' // if we don't know the total size, we should use * - - return `bytes ${rangeStart}-${end}/${total}` -} diff --git a/packages/verified-fetch/src/utils/responses.ts b/packages/verified-fetch/src/utils/responses.ts index 17577465..fe3065bf 100644 --- a/packages/verified-fetch/src/utils/responses.ts +++ b/packages/verified-fetch/src/utils/responses.ts @@ -1,4 +1,4 @@ -import { getContentRangeHeader } from './response-headers.js' +import type { ByteRangeContext } from './byte-range-context' import type { SupportedBodyTypes } from '../types.js' function setField (response: Response, name: string, value: string | boolean): void { @@ -113,36 +113,40 @@ export function movedPermanentlyResponse (url: string, location: string, init?: return response } -type RangeOptions = { offset: number, length: number, totalSize?: number } | { offset: number, length?: number, totalSize: number } +interface RangeOptions { + byteRangeContext: ByteRangeContext +} -/** - * Some caveats about range responses here: - * * We only support single range requests (multi-range is optional), see https://specs.ipfs.tech/http-gateways/path-gateway/#range-request-header - * * Range responses are only supported for unixfs and raw data, see https://specs.ipfs.tech/http-gateways/path-gateway/#range-request-header. - * - * If the user requests something other than unixfs or raw data, we should not call this method and ignore the range header (200 OK). See https://developer.mozilla.org/en-US/docs/Web/HTTP/Range_requests#partial_request_responses - * - * TODO: Supporting multiple range requests will require additional changes to the `handleDagPb` and `handleRaw` functions in `src/verified-fetch.js` - */ -export function okRangeResponse (url: string, body: SupportedBodyTypes, range: RangeOptions, init?: ResponseOptions): Response { - // if we know the full size of the body, we should use it in the content-range header. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range - let contentRangeHeader: string | undefined +export function okRangeResponse (url: string, body: SupportedBodyTypes, { byteRangeContext }: RangeOptions, init?: ResponseOptions): Response { + if (!byteRangeContext.isRangeRequest) { + return okResponse(url, body, init) + } + byteRangeContext.body = body + if (!byteRangeContext.isValidRangeRequest) { + return badRangeResponse(url, body, init) + } + let response: Response + // let body: SupportedBodyTypes try { - contentRangeHeader = getContentRangeHeader({ body, total: range.totalSize, offset: range.offset, length: range.length }) - } catch (e) { + body = byteRangeContext.body + } catch { + return badRangeResponse(url, body, init) + } + try { + response = new Response(byteRangeContext.body, { + ...(init ?? {}), + status: 206, + statusText: 'Partial Content', + headers: { + ...(init?.headers ?? {}), + 'content-range': byteRangeContext.contentRangeHeaderValue + } + }) + } catch { + // TODO: should we return a different status code here? return badRangeResponse(url, body, init) } - - const response = new Response(body, { - ...(init ?? {}), - status: 206, - statusText: 'Partial Content', - headers: { - ...(init?.headers ?? {}), - 'content-range': contentRangeHeader - } - }) if (init?.redirected === true) { setRedirected(response) diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index 60cde747..e7363166 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -16,20 +16,20 @@ import { CustomProgressEvent } from 'progress-events' import { concat as uint8ArrayConcat } from 'uint8arrays/concat' import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string' import { toString as uint8ArrayToString } from 'uint8arrays/to-string' +import { ByteRangeContext } from './utils/byte-range-context.js' import { dagCborToSafeJSON } from './utils/dag-cbor-to-safe-json.js' import { getContentDispositionFilename } from './utils/get-content-disposition-filename.js' import { getETag } from './utils/get-e-tag.js' import { getStreamFromAsyncIterable } from './utils/get-stream-from-async-iterable.js' import { tarStream } from './utils/get-tar-stream.js' import { parseResource } from './utils/parse-resource.js' -import { getRequestRange } from './utils/request-headers.js' import { badRequestResponse, movedPermanentlyResponse, notAcceptableResponse, notSupportedResponse, okResponse, badRangeResponse, internalServerErrorResponse, okRangeResponse } from './utils/responses.js' import { selectOutputType, queryFormatToAcceptHeader } from './utils/select-output-type.js' import { walkPath } from './utils/walk-path.js' import type { CIDDetail, ContentTypeParser, Resource, VerifiedFetchInit as VerifiedFetchOptions } from './index.js' import type { RequestFormatShorthand } from './types.js' import type { Helia } from '@helia/interface' -import type { AbortOptions, CodeError, Logger, PeerId } from '@libp2p/interface' +import type { AbortOptions, Logger, PeerId } from '@libp2p/interface' import type { UnixFSEntry } from 'ipfs-unixfs-exporter' import type { CID } from 'multiformats/cid' @@ -277,44 +277,21 @@ export class VerifiedFetch { return response } - // eslint-disable-next-line complexity private async handleDagPb ({ cid, path, resource, options }: FetchHandlerFunctionArg): Promise { let terminalElement: UnixFSEntry | undefined let ipfsRoots: CID[] | undefined let redirected = false - let offset: number | undefined - let length: number | undefined - let suffixLength: number | undefined - let isRangeRequest = false - try { - const rangeOptions = getRequestRange(options?.headers) - offset = rangeOptions?.offset - length = rangeOptions?.length - suffixLength = rangeOptions?.suffixLength - this.log.trace('range options %o', rangeOptions) - if (offset != null || length != null || suffixLength != null) { - isRangeRequest = true - } - } catch (err) { - this.log.error('Error parsing range request', err) - return badRangeResponse('Invalid range request') + const byteRangeContext = new ByteRangeContext(this.helia.logger, options?.headers) + if (byteRangeContext.isRangeRequest && !byteRangeContext.isValidRangeRequest) { + return badRangeResponse(resource) } try { - const pathDetails = await walkPath(this.helia.blockstore, `${cid.toString()}/${path}`, { ...options, offset, length }) + const pathDetails = await walkPath(this.helia.blockstore, `${cid.toString()}/${path}`, options) ipfsRoots = pathDetails.ipfsRoots terminalElement = pathDetails.terminalElement } catch (err) { this.log.error('Error walking path %s', path, err) - if (isRangeRequest && (err as CodeError)?.code === 'ERR_INVALID_PARAMS') { - /** - * the code is not explicit but seems to be the only error for invalid range requests - * - * @see https://github.com/search?q=repo%3Aipfs%2Fjs-ipfs-unixfs+ERR_INVALID_PARAMS+path%3Apackages%2Fipfs-unixfs-exporter&type=code - * @see https://github.com/ipfs/js-ipfs-unixfs/blob/4749d9a7c1eddd86b8fc42c3fa47f88c7b1b75ae/packages/ipfs-unixfs-exporter/src/utils/validate-offset-and-length.ts#L16-L30 - */ - return badRangeResponse(resource) - } return internalServerErrorResponse('Error walking path') } @@ -358,37 +335,11 @@ export class VerifiedFetch { } } - /** - * range-request validation. This shouldn't be required, but unixfs.cat isn't throwing in some cases. - */ - if (isRangeRequest) { - /** - * if requested CID was a directory, we already have a unixfs stat object - * if requested CID was a file or raw, we need to fetch the stat object - */ - stat = stat ?? await this.unixfs.stat(resolvedCID, { - signal: options?.signal, - onProgress: options?.onProgress - }) - - if (suffixLength != null) { - const rangeWithSize = getRequestRange(options?.headers, stat.fileSize) - if (rangeWithSize != null) { - offset = rangeWithSize.offset - length = rangeWithSize.length - } - } - const resp = this.checkForInvalidRangeRequest({ stat, offset, length, resource }) - if (resp != null) { - return resp - } - } - const asyncIter = this.unixfs.cat(resolvedCID, { signal: options?.signal, onProgress: options?.onProgress, - offset, - length + offset: byteRangeContext.offset, + length: byteRangeContext.length }) this.log('got async iterator for %c/%s', cid, path) @@ -396,19 +347,19 @@ export class VerifiedFetch { const { stream, firstChunk } = await getStreamFromAsyncIterable(asyncIter, path ?? '', this.helia.logger, { onProgress: options?.onProgress }) - let response: Response - if (isRangeRequest) { - // type assertion says stat may still be undefined, but we've set it again in the above check for isRangeRequest - const totalSize = Number(stat?.fileSize) - offset = offset ?? 0 - response = okRangeResponse(resource, stream, { offset, length, totalSize }, { - redirected - }) - } else { - response = okResponse(resource, stream, { - redirected + if (byteRangeContext.isRangeRequest && byteRangeContext.isValidRangeRequest) { + stat = stat ?? await this.unixfs.stat(resolvedCID, { + signal: options?.signal, + onProgress: options?.onProgress }) + byteRangeContext.fileSize = stat.fileSize + byteRangeContext.body = stream } + // if not a valid range request, okRangeRequest will call okResponse + const response = okRangeResponse(resource, byteRangeContext.body, { byteRangeContext }, { + redirected + }) + await this.setContentType(firstChunk, path, response) if (ipfsRoots != null) { @@ -416,9 +367,9 @@ export class VerifiedFetch { } return response - } catch (err) { + } catch (err: any) { this.log.error('Error streaming %c/%s', cid, path, err) - if (isRangeRequest && (err as CodeError)?.code === 'ERR_INVALID_PARAMS') { + if (byteRangeContext.isRangeRequest && err.code === 'ERR_INVALID_PARAMS') { return badRangeResponse(resource) } return internalServerErrorResponse('Unable to stream content') @@ -426,8 +377,17 @@ export class VerifiedFetch { } private async handleRaw ({ resource, cid, path, options }: FetchHandlerFunctionArg): Promise { + const byteRangeContext = new ByteRangeContext(this.helia.logger, options?.headers) const result = await this.helia.blockstore.get(cid, options) - const response = okResponse(resource, result) + byteRangeContext.body = result + let response: Response + if (byteRangeContext.isRangeRequest) { + response = okRangeResponse(resource, result, { byteRangeContext }, { + redirected: false + }) + } else { + response = okResponse(resource, result) + } // if the user has specified an `Accept` header that corresponds to a raw // type, honour that header, so for example they don't request @@ -468,31 +428,6 @@ export class VerifiedFetch { response.headers.set('content-type', contentType) } - private checkForInvalidRangeRequest ({ offset, length, resource, stat }: { offset?: number, length?: number, resource: string, stat: UnixFSStats }): Response | undefined { - if (offset != null) { - if (offset < 0) { - // the requested range start is negative - this.log.error('range start %d is negative', offset) - return badRangeResponse(resource) - } else if (stat.fileSize < offset) { - // the requested range start is larger than the requested content size - this.log.error('range start %d is larger than the requested content size %d', offset, stat.fileSize) - return badRangeResponse(resource) - } else if (length != null && Number(stat.fileSize) < (offset + length)) { - // the requested range is larger than the requested content size - this.log.error('range end %d is larger than the requested content size %d', offset + length, stat.fileSize) - return badRangeResponse(resource) - } - } - if (length != null) { - if (Number(stat.fileSize) < length) { - // the requested range is larger than the requested content size - this.log.error('range end %d is larger than the requested content size %d', length, stat.fileSize) - return badRangeResponse(resource) - } - } - } - /** * If the user has not specified an Accept header or format query string arg, * use the CID codec to choose an appropriate handler for the block data. diff --git a/packages/verified-fetch/test/range-requests.spec.ts b/packages/verified-fetch/test/range-requests.spec.ts index 8a83fa3c..c0978ed1 100644 --- a/packages/verified-fetch/test/range-requests.spec.ts +++ b/packages/verified-fetch/test/range-requests.spec.ts @@ -1,7 +1,9 @@ -import { unixfs, type UnixFS } from '@helia/unixfs' +import { unixfs } from '@helia/unixfs' import { stop } from '@libp2p/interface' import { expect } from 'aegir/chai' -import { type CID } from 'multiformats/cid' +import { CID } from 'multiformats/cid' +import * as raw from 'multiformats/codecs/raw' +import { sha256 } from 'multiformats/hashes/sha2' import { VerifiedFetch } from '../src/verified-fetch.js' import { createHelia } from './fixtures/create-offline-helia.js' import type { Helia } from '@helia/interface' @@ -12,23 +14,12 @@ import type { Helia } from '@helia/interface' describe('range requests', () => { let helia: Helia let verifiedFetch: VerifiedFetch - let fs: UnixFS - let dagPbCid: CID beforeEach(async () => { helia = await createHelia() - fs = unixfs(helia) verifiedFetch = new VerifiedFetch({ helia }) - - dagPbCid = await fs.addFile({ - path: 'bigbuckbunny.mp4', - content: new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) - }, { - rawLeaves: false, - leafType: 'file' - }) }) afterEach(async () => { @@ -63,74 +54,102 @@ describe('range requests', () => { await expect(response).to.eventually.have.property('statusText', 'Requested Range Not Satisfiable') } - describe('unixfs', () => { - it('should return correct 206 Partial Content response for byte=-', async () => { - const expected: SuccessfulTestExpectation = { - byteSize: 6, - contentRange: 'bytes 0-5/11' - } - await testRange(dagPbCid, 'bytes=0-5', expected) - }) - - it('should return correct 206 Partial Content response for byte=-', async () => { - const expected = { - byteSize: 7, - contentRange: 'bytes 4-11/11' - } - await testRange(dagPbCid, 'bytes=4-', expected) - }) - - it('should return correct 206 Partial Content response for byte=-', async () => { - const expected = { - byteSize: 9, - contentRange: 'bytes 2-10/11' - } - await testRange(dagPbCid, 'bytes=-9', expected) - }) - - it('should return 416 Range Not Satisfiable when the range is invalid', async () => { - await assertFailingRange(verifiedFetch.fetch(dagPbCid, { - headers: { - Range: 'bytes=-0-' - } - })) - await assertFailingRange(verifiedFetch.fetch(dagPbCid, { - headers: { - Range: 'bytes=foobar' - } - })) - }) - - it('should return 416 Range Not Satisfiable when the range offset is larger than content', async () => { - await assertFailingRange(verifiedFetch.fetch(dagPbCid, { - headers: { - Range: 'bytes=50-' + function runTests (description: string, getCid: () => Promise): void { + describe(description, () => { + let cid: CID + beforeEach(async () => { + cid = await getCid() + }) + it('should return correct 206 Partial Content response for byte=-', async () => { + const expected: SuccessfulTestExpectation = { + byteSize: 6, + contentRange: 'bytes 0-5/11' } - })) - }) + await testRange(cid, 'bytes=0-5', expected) + }) - it('should return 416 Range Not Satisfiable when the suffix-length is larger than content', async () => { - await assertFailingRange(verifiedFetch.fetch(dagPbCid, { - headers: { - Range: 'bytes=-50' + it('should return correct 206 Partial Content response for byte=-', async () => { + const expected = { + byteSize: 7, + contentRange: 'bytes 4-11/11' } - })) - }) + await testRange(cid, 'bytes=4-', expected) + }) - it('should return 416 Range Not Satisfiable when the range is out of bounds', async () => { - await assertFailingRange(verifiedFetch.fetch(dagPbCid, { - headers: { - Range: 'bytes=0-900' + it('should return correct 206 Partial Content response for byte=-', async () => { + const expected = { + byteSize: 9, + contentRange: 'bytes 3-11/11' } - })) + await testRange(cid, 'bytes=-9', expected) + }) + + it('should return 416 Range Not Satisfiable when the range is invalid', async () => { + await assertFailingRange(verifiedFetch.fetch(cid, { + headers: { + Range: 'bytes=-0-' + } + })) + await assertFailingRange(verifiedFetch.fetch(cid, { + headers: { + Range: 'bytes=foobar' + } + })) + }) + + it('should return 416 Range Not Satisfiable when the range offset is larger than content', async () => { + await assertFailingRange(verifiedFetch.fetch(cid, { + headers: { + Range: 'bytes=50-' + } + })) + }) + + it('should return 416 Range Not Satisfiable when the suffix-length is larger than content', async () => { + await assertFailingRange(verifiedFetch.fetch(cid, { + headers: { + Range: 'bytes=-50' + } + })) + }) + + it('should return 416 Range Not Satisfiable when the range is out of bounds', async () => { + await assertFailingRange(verifiedFetch.fetch(cid, { + headers: { + Range: 'bytes=0-900' + } + })) + }) + + it('should return 416 Range Not Satisfiable when passed multiple ranges', async () => { + await assertFailingRange(verifiedFetch.fetch(cid, { + headers: { + Range: 'bytes=0-2,3-5' + } + })) + }) }) + } - it('should return 416 Range Not Satisfiable when passed multiple ranges', async () => { - await assertFailingRange(verifiedFetch.fetch(dagPbCid, { - headers: { - Range: 'bytes=0-2,3-5' - } - })) - }) + const content = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) + const testTuples = [ + ['unixfs', async () => { + return unixfs(helia).addFile({ + content + }, { + rawLeaves: false, + leafType: 'file' + }) + }], + ['raw', async () => { + const buf = raw.encode(content) + const cid = CID.createV1(raw.code, await sha256.digest(buf)) + await helia.blockstore.put(cid, buf) + return cid + }] + ] as const + + testTuples.forEach(([name, fn]) => { + runTests(name, fn) }) }) diff --git a/packages/verified-fetch/test/utils/byte-range-context.spec.ts b/packages/verified-fetch/test/utils/byte-range-context.spec.ts new file mode 100644 index 00000000..74dc3fd6 --- /dev/null +++ b/packages/verified-fetch/test/utils/byte-range-context.spec.ts @@ -0,0 +1,81 @@ +import { prefixLogger } from '@libp2p/logger' +import { expect } from 'aegir/chai' +import { ByteRangeContext } from '../../src/utils/byte-range-context.js' + +describe('ByteRangeContext', () => { + const logger = prefixLogger('test') + + it('should correctly detect range request', () => { + const context = new ByteRangeContext(logger, { Range: 'bytes=0-2' }) + expect(context.isRangeRequest).to.be.true() + }) + + it('should correctly detect non-range request', () => { + const context = new ByteRangeContext(logger, {}) + expect(context.isRangeRequest).to.be.false() + }) + + it('should correctly set body and calculate fileSize', () => { + const context = new ByteRangeContext(logger, {}) + const body = new Uint8Array([1, 2, 3, 4, 5]) + context.body = body + expect(context.body).to.equal(body) + expect(context.fileSize).to.equal(body.length) + }) + + it('should correctly handle invalid range request', () => { + const invalidRanges = [ + 'bytes=f', + 'bytes=0-foobar', + 'bytes=f-0', + 'byte=0-2' + ] + invalidRanges.forEach(range => { + const context = new ByteRangeContext(logger, { Range: range }) + expect(context.isValidRangeRequest).to.be.false() + }) + }) + + // it('should correctly slice body with valid range headers', () => { + const validRanges = [ + // Uint8Arrays + { type: 'Uint8Array', range: 'bytes=0-2', contentRange: 'bytes 0-2/5', body: new Uint8Array([1, 2, 3, 4, 5]), expected: new Uint8Array([1, 2, 3]) }, + { type: 'Uint8Array', range: 'bytes=2-', contentRange: 'bytes 2-5/5', body: new Uint8Array([1, 2, 3, 4, 5]), expected: new Uint8Array([3, 4, 5]) }, + { type: 'Uint8Array', range: 'bytes=-2', contentRange: 'bytes 3-5/5', body: new Uint8Array([1, 2, 3, 4, 5]), expected: new Uint8Array([4, 5]) }, + // ArrayBuffers + { type: 'ArrayBuffer', range: 'bytes=0-2', contentRange: 'bytes 0-2/5', body: new Uint8Array([1, 2, 3, 4, 5]).buffer, expected: new Uint8Array([1, 2, 3]).buffer }, + { type: 'ArrayBuffer', range: 'bytes=2-', contentRange: 'bytes 2-5/5', body: new Uint8Array([1, 2, 3, 4, 5]).buffer, expected: new Uint8Array([3, 4, 5]).buffer }, + { type: 'ArrayBuffer', range: 'bytes=-2', contentRange: 'bytes 3-5/5', body: new Uint8Array([1, 2, 3, 4, 5]).buffer, expected: new Uint8Array([4, 5]).buffer }, + // Blobs + { type: 'Blob', range: 'bytes=0-2', contentRange: 'bytes 0-2/5', body: new Blob([new Uint8Array([1, 2, 3, 4, 5])]), expected: new Blob([new Uint8Array([1, 2, 3])]) }, + { type: 'Blob', range: 'bytes=2-', contentRange: 'bytes 2-5/5', body: new Blob([new Uint8Array([1, 2, 3, 4, 5])]), expected: new Blob([new Uint8Array([3, 4, 5])]) }, + { type: 'Blob', range: 'bytes=-2', contentRange: 'bytes 3-5/5', body: new Blob([new Uint8Array([1, 2, 3, 4, 5])]), expected: new Blob([new Uint8Array([4, 5])]) } + ] + validRanges.forEach(({ type, range, expected, body, contentRange }) => { + it(`should correctly slice ${type} body with range ${range}`, () => { + const context = new ByteRangeContext(logger, { Range: range }) + context.body = body + expect(context.body).to.deep.equal(expected) + expect(context.contentRangeHeaderValue).to.equal(contentRange) + }) + }) + // }) + + it('should correctly handle ReadableStreams', () => { + const context = new ByteRangeContext(logger, { Range: 'bytes=0-2' }) + let i = 0 + const body = new ReadableStream({ + pull (controller) { + if (i >= 3) { + controller.close() + return + } + controller.enqueue(new Uint8Array([i++])) + } + }) + context.body = body + expect(context.body).to.equal(body) + expect(context.fileSize).to.be.null() + expect(context.contentRangeHeaderValue).to.equal('bytes 0-2/*') + }) +}) diff --git a/packages/verified-fetch/test/utils/request-headers.spec.ts b/packages/verified-fetch/test/utils/request-headers.spec.ts index 9bfbbae3..53697c09 100644 --- a/packages/verified-fetch/test/utils/request-headers.spec.ts +++ b/packages/verified-fetch/test/utils/request-headers.spec.ts @@ -1,5 +1,5 @@ import { expect } from 'aegir/chai' -import { getHeader, getRequestRange } from '../../src/utils/request-headers.js' +import { getHeader } from '../../src/utils/request-headers.js' describe('request-headers', () => { describe('getHeader', () => { @@ -28,42 +28,4 @@ describe('request-headers', () => { expect(getHeader(headers, 'dummy')).to.equal('value') }) }) - - describe('getRequestRange', () => { - it('should return undefined when no Range header is provided', () => { - const headers = new Headers() - const result = getRequestRange(headers) - expect(result).to.be.undefined() - }) - - it('should throw an error when an invalid Range header is provided', () => { - expect(() => getRequestRange(new Headers({ Range: 'invalid' }))).to.throw('ERR_INVALID_RANGE_REQUEST') - expect(() => getRequestRange(new Headers({ Range: 'bytes=foobar' }))).to.throw('ERR_INVALID_RANGE_REQUEST') - }) - - it('throws for multi-range requests', () => { - expect(() => getRequestRange(new Headers({ Range: 'bytes=0-500, 600-1000' }))).to.throw('ERR_INVALID_RANGE_REQUEST') - }) - - it('should return correct range options when a valid Range header has start only', () => { - const result = getRequestRange(new Headers({ Range: 'bytes=2-' })) - expect(result).to.deep.equal({ offset: 2, length: undefined, suffixLength: undefined }) - }) - - it('should return correct range options when a valid Range header has start and end', () => { - const result = getRequestRange(new Headers({ Range: 'bytes=0-500' })) - expect(result).to.deep.equal({ offset: 0, length: 501, suffixLength: undefined }) - }) - - it('should return only suffixLength when not passed range-start nor size', () => { - const result = getRequestRange(new Headers({ Range: 'bytes=-20' })) - expect(result).to.deep.equal({ offset: undefined, length: undefined, suffixLength: 20 }) - }) - - it('should return correct range options when a valid Range header has end only', () => { - const headers = new Headers({ Range: 'bytes=-25' }) - const result = getRequestRange(headers, 100n) - expect(result).to.deep.equal({ offset: 75, length: 25, suffixLength: undefined }) - }) - }) }) diff --git a/packages/verified-fetch/test/utils/response-headers.spec.ts b/packages/verified-fetch/test/utils/response-headers.spec.ts deleted file mode 100644 index dd45a198..00000000 --- a/packages/verified-fetch/test/utils/response-headers.spec.ts +++ /dev/null @@ -1,87 +0,0 @@ -import { expect } from 'aegir/chai' -import { getContentRangeHeader } from '../../src/utils/response-headers.js' - -describe('response-headers', () => { - describe('getContentRangeHeader', () => { - const body = 'dummy body' - const bodySize = body.length - it('should return correct content range header when only given offset', () => { - const offset = 2 - const result = getContentRangeHeader({ body, offset }) - expect(result).to.equal(`bytes 2-${bodySize}/${bodySize}`) - }) - - it('should return correct content range header when only given length', () => { - const length = 2 - const result = getContentRangeHeader({ body, length }) - expect(result).to.equal(`bytes 0-1/${bodySize}`) - }) - - it('should override byte-size when total is provided', () => { - const offset = 1 - const length = 2 - const total = 4 - const result = getContentRangeHeader({ total, body, offset, length }) - expect(result).to.equal('bytes 1-2/4') - }) - - it('should return correct content range header when total is not provided', () => { - const offset = 0 - const length = 3 - const result = getContentRangeHeader({ body, offset, length }) - expect(result).to.equal(`bytes 0-2/${body.length}`) - }) - - it('should return content range header with * when total and body size are not known', () => { - const offset = 1 - const length = 4 - const body = null - const result = getContentRangeHeader({ body, offset, length }) - expect(result).to.equal('bytes 1-4/*') - }) - - it('should return content range header with * body is a ReadableStream', () => { - const offset = 5 - const length = 500 - const body = new ReadableStream() - const result = getContentRangeHeader({ body, offset, length }) - expect(result).to.equal('bytes 5-504/*') - }) - - it('should return content range header with the correct arrayBuffer size', () => { - const offset = 6 - const length = 40 - const body = new ArrayBuffer(1000) - const result = getContentRangeHeader({ body, offset, length }) - expect(result).to.equal(`bytes 6-45/${body.byteLength}`) - }) - - it('should return content range header with the correct Blob size', () => { - const offset = 7 - const length = 2 - const body = new Blob(['dummy body']) - const result = getContentRangeHeader({ body, offset, length }) - expect(result).to.equal(`bytes 7-8/${body.size}`) - }) - - it('should return content range header with the correct Uint8Array size', () => { - const offset = 2 - const length = 9 - const body = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) - const result = getContentRangeHeader({ body, offset, length }) - expect(result).to.equal('bytes 2-10/11') - }) - - it('should return content range header with the correct Uint8Array size & explicit total', () => { - const offset = 4 - const body = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) - const result = getContentRangeHeader({ body, offset, total: 11 }) - expect(result).to.equal('bytes 4-11/11') - }) - - it('should return content range header with offset, length, & total', () => { - const result = getContentRangeHeader({ body: null, offset: 2, length: 9, total: 11 }) - expect(result).to.equal('bytes 2-10/11') - }) - }) -}) From cac2b7997acb336f0fd35dc932a6ff51e9fa2e13 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Tue, 5 Mar 2024 13:56:45 -0800 Subject: [PATCH 10/39] fix: stream slicing and test correctness --- .../src/utils/byte-range-context.ts | 146 +++++------- .../utils/get-stream-from-async-iterable.ts | 2 +- .../src/utils/parse-resource.ts | 2 +- .../src/utils/parse-url-string.ts | 2 +- .../verified-fetch/src/utils/responses.ts | 18 +- .../src/utils/splicing-transform-stream.ts | 109 +++++++++ packages/verified-fetch/src/verified-fetch.ts | 12 +- .../test/range-requests.spec.ts | 6 +- .../test/utils/byte-range-context.spec.ts | 215 +++++++++++++++--- .../utils/splicing-transform-stream.spec.ts | 125 ++++++++++ 10 files changed, 493 insertions(+), 144 deletions(-) create mode 100644 packages/verified-fetch/src/utils/splicing-transform-stream.ts create mode 100644 packages/verified-fetch/test/utils/splicing-transform-stream.spec.ts diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index c056bc1d..370c83a1 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -1,4 +1,5 @@ import { getHeader } from './request-headers.js' +import { splicingTransformStream } from './splicing-transform-stream.js' import type { SupportedBodyTypes } from '../types.js' import type { ComponentLogger, Logger } from '@libp2p/logger' @@ -47,12 +48,12 @@ export class ByteRangeContext { private _isValidRangeRequest: boolean | null = null private _offset: number | undefined = undefined private _length: number | undefined = undefined - private readonly requestRangeStart!: string | undefined - private readonly requestRangeEnd!: string | undefined + private readonly requestRangeStart!: number | undefined + private readonly requestRangeEnd!: number | undefined private responseRangeStart!: number private responseRangeEnd!: number - constructor (logger: ComponentLogger, private readonly headers?: HeadersInit) { + constructor (private readonly logger: ComponentLogger, private readonly headers?: HeadersInit) { this.log = logger.forComponent('helia:verified-fetch:byte-range-context') this._rangeRequestHeader = getHeader(this.headers, 'Range') if (this._rangeRequestHeader != null) { @@ -60,8 +61,8 @@ export class ByteRangeContext { this._isRangeRequest = true try { const { start, end } = getByteRangeFromHeader(this._rangeRequestHeader) - this.requestRangeStart = start - this.requestRangeEnd = end + this.requestRangeStart = start == null ? undefined : parseInt(start) + this.requestRangeEnd = end == null ? undefined : parseInt(end) } catch (e) { this.log.error('error parsing range request header: %o', e) this.isValidRangeRequest = false @@ -74,11 +75,9 @@ export class ByteRangeContext { this.requestRangeStart = undefined this.requestRangeEnd = undefined } - - this.log.trace('created ByteRangeContext with headers %o. is range request? %s, is valid range? %s', this.headers, this._isRangeRequest, this.isValidRangeRequest) } - public set body (body: SupportedBodyTypes) { + public setBody (body: SupportedBodyTypes): void { this._body = body // if fileSize was set explicitly or already set, don't recalculate it this.fileSize = this.fileSize ?? getBodySizeSync(body) @@ -87,15 +86,15 @@ export class ByteRangeContext { this.log.trace('set request body with fileSize %o', this._fileSize) } - public get body (): SupportedBodyTypes { - this.log.trace('getting body') + public getBody (): SupportedBodyTypes { + this.log.trace('getting body, getBody') const body = this._body if (body == null) { this.log.trace('body is null') return body } if (!this.isRangeRequest || !this.isValidRangeRequest) { - this.log.trace('returning body without offset or length') + this.log.trace('returning body unmodified') return body } const offset = this.offset @@ -110,8 +109,7 @@ export class ByteRangeContext { } else if (body instanceof Blob) { return this.getSlicedBody(body, offset, length) } else if (body instanceof ReadableStream) { - // TODO: if offset and length is not null, will the browser handle this? - return this.getSlicedStream(body, offset, length) + return splicingTransformStream(body, offset, length, this.logger) } } // offset and length are not set, so not a range request, return body untouched. @@ -119,74 +117,17 @@ export class ByteRangeContext { } private getSlicedBody (body: T, offset: number | undefined, length: number | undefined): T { - // const bodySize = body instanceof Blob ? body.size : body.byteLength - this.log.trace('slicing body with offset %o and length %o', offset, length) - try { - if (offset != null && length != null) { - return body.slice(offset, offset + length) as T - } else if (offset != null) { - return body.slice(offset) as T - } else if (length != null) { - return body.slice(0, length + 1) as T - } else { - return body - } - } catch (e) { - this.log.error('error slicing Uint8Array: %o', e) - throw e + if (offset != null && length != null) { + return body.slice(offset, offset + length + 1) as T + } else if (offset != null) { + return body.slice(offset) as T + } else if (length != null) { + return body.slice(0, length + 1) as T + } else { + return body } } - /** - * If a user requests a range of bytes from a ReadableStream, we need to read the stream and enqueue only the requested bytes. - */ - private getSlicedStream (body: ReadableStream, offset: number | undefined, length: number | undefined): ReadableStream { - const reader = body.getReader() - return new ReadableStream({ - // async start (controller) { - // if (offset != null) { - // // skip bytes until we reach the offset - // let bytesRead = 0 - // while (bytesRead < offset) { - // const { value, done } = await reader.read() - // if (done) { - // break - // } - // bytesRead += value.byteLength - // } - // } - // }, - async pull (controller) { - if (length == null) { - const { value, done } = await reader.read() - if (done) { - controller.close() - return - } - controller.enqueue(value) - return - } - let bytesRead = 0 - let bytesRemaining = length - while (bytesRead < length) { - const { value, done } = await reader.read() - if (done) { - controller.close() - return - } - if (value.byteLength > bytesRemaining) { - controller.enqueue(value.slice(0, bytesRemaining)) - bytesRead += bytesRemaining - return - } - bytesRead += value.byteLength - bytesRemaining -= value.byteLength - controller.enqueue(value) - } - } - }) - } - public set fileSize (size: number | bigint | null) { this._fileSize = size != null ? Number(size) : null } @@ -211,7 +152,7 @@ export class ByteRangeContext { } else if (this.length != null && this._fileSize != null && this.length > this._fileSize) { this._isValidRangeRequest = false } else if (this.requestRangeStart != null && this.requestRangeEnd != null) { - if (parseInt(this.requestRangeStart) > parseInt(this.requestRangeEnd)) { + if (this.requestRangeStart > this.requestRangeEnd) { this._isValidRangeRequest = false } } @@ -260,37 +201,46 @@ export class ByteRangeContext { this.log.trace('requestRangeStart and requestRangeEnd are null') return } - this.log.trace('requestRangeStart %o, requestRangeEnd %o', this.requestRangeStart, this.requestRangeEnd) if (this.requestRangeStart != null && this.requestRangeEnd != null) { // we have a specific rangeRequest start & end - this.offset = parseInt(this.requestRangeStart) - this.length = parseInt(this.requestRangeEnd) - this.offset + 1 - this.responseRangeStart = this.offset - this.responseRangeEnd = this.offset + this.length - 1 + this.offset = this.requestRangeStart + this.length = this.requestRangeEnd - this.requestRangeStart + this.responseRangeStart = this.requestRangeStart + this.responseRangeEnd = this.requestRangeEnd } else if (this.requestRangeStart != null) { // we have a specific rangeRequest start - this.offset = parseInt(this.requestRangeStart) + this.offset = this.requestRangeStart this.responseRangeStart = this.offset if (this._fileSize != null) { - this.length = this._fileSize - this.offset + 1 - this.responseRangeEnd = this.offset + this.length - 1 + this.length = this._fileSize - this.offset + this.responseRangeEnd = this.offset + this.length } else { this.length = undefined } } else if (this.requestRangeEnd != null && this._fileSize != null) { - this.log.trace('requestRangeEnd %o, fileSize %o', this.requestRangeEnd, this._fileSize) // we have a specific rangeRequest end (suffix-length) & filesize - const lengthRequested = parseInt(this.requestRangeEnd) + const lengthRequested = this.requestRangeEnd // if the user requested length of N, the offset is N bytes from the end of the file this.offset = this._fileSize - lengthRequested this.length = lengthRequested - this.responseRangeStart = this.offset // inclusive - this.responseRangeEnd = this._fileSize // inclusive + this.responseRangeStart = this.offset === 0 ? this.offset : this.offset + 1 + this.responseRangeEnd = this._fileSize + } else if (this.requestRangeEnd != null) { + this.log.trace('requestRangeEnd %o, but no fileSize', this.requestRangeEnd) + // we have a specific rangeRequest end (suffix-length) but no filesize + this.offset = undefined + this.length = this.requestRangeEnd + this.responseRangeEnd = this.requestRangeEnd } else { this.log.trace('Not enough information to set offset and length') } } + // This function returns the value of the "content-range" header. + // Content-Range: -/ + // Content-Range: -/* + // Content-Range: */ + // @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range public get contentRangeHeaderValue (): string { if (this._contentRangeHeaderValue != null) { return this._contentRangeHeaderValue @@ -300,7 +250,19 @@ export class ByteRangeContext { throw new Error('Invalid range request') } const fileSize = this._fileSize ?? '*' - this._contentRangeHeaderValue = `bytes ${this.responseRangeStart}-${this.responseRangeEnd}/${fileSize}` + // TODO: if the body is a Stream and we don't have the offset or length... + // do we set "content-range" to: + // "bytes -*/*" + // "bytes *-/*" + // or "bytes */*" ? Are these valid? + // or do we want to consume the stream and calculate the size? + if (this._body instanceof ReadableStream) { + const rangeStart = this.responseRangeStart ?? this.requestRangeStart ?? '*' + const rangeEnd = this.responseRangeEnd ?? this.requestRangeEnd ?? '*' + this._contentRangeHeaderValue = `bytes ${rangeStart}-${rangeEnd}/${fileSize}` + } else { + this._contentRangeHeaderValue = `bytes ${this.responseRangeStart}-${this.responseRangeEnd}/${fileSize}` + } return this._contentRangeHeaderValue } } diff --git a/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts b/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts index c9266e2e..f57e97f0 100644 --- a/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts +++ b/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts @@ -1,6 +1,6 @@ import { CustomProgressEvent } from 'progress-events' import type { VerifiedFetchInit } from '../index.js' -import type { ComponentLogger } from '@libp2p/interface' +import type { ComponentLogger } from '@libp2p/logger' /** * Converts an async iterator of Uint8Array bytes to a stream and returns the first chunk of bytes diff --git a/packages/verified-fetch/src/utils/parse-resource.ts b/packages/verified-fetch/src/utils/parse-resource.ts index a20e82d4..23b3061a 100644 --- a/packages/verified-fetch/src/utils/parse-resource.ts +++ b/packages/verified-fetch/src/utils/parse-resource.ts @@ -3,7 +3,7 @@ import { parseUrlString } from './parse-url-string.js' import type { ParsedUrlStringResults } from './parse-url-string.js' import type { Resource } from '../index.js' import type { IPNS, IPNSRoutingEvents, ResolveDnsLinkProgressEvents, ResolveProgressEvents } from '@helia/ipns' -import type { ComponentLogger } from '@libp2p/interface' +import type { ComponentLogger } from '@libp2p/logger' import type { ProgressOptions } from 'progress-events' export interface ParseResourceComponents { diff --git a/packages/verified-fetch/src/utils/parse-url-string.ts b/packages/verified-fetch/src/utils/parse-url-string.ts index 115b250d..e6b941f3 100644 --- a/packages/verified-fetch/src/utils/parse-url-string.ts +++ b/packages/verified-fetch/src/utils/parse-url-string.ts @@ -3,7 +3,7 @@ import { CID } from 'multiformats/cid' import { TLRU } from './tlru.js' import type { RequestFormatShorthand } from '../types.js' import type { IPNS, IPNSRoutingEvents, ResolveDnsLinkProgressEvents, ResolveProgressEvents, ResolveResult } from '@helia/ipns' -import type { ComponentLogger } from '@libp2p/interface' +import type { ComponentLogger } from '@libp2p/logger' import type { ProgressOptions } from 'progress-events' const ipnsCache = new TLRU(1000) diff --git a/packages/verified-fetch/src/utils/responses.ts b/packages/verified-fetch/src/utils/responses.ts index fe3065bf..ee574dbf 100644 --- a/packages/verified-fetch/src/utils/responses.ts +++ b/packages/verified-fetch/src/utils/responses.ts @@ -39,6 +39,7 @@ export function okResponse (url: string, body?: SupportedBodyTypes, init?: Respo setType(response, 'basic') setUrl(response, url) + response.headers.set('Accept-Ranges', 'bytes') return response } @@ -121,20 +122,16 @@ export function okRangeResponse (url: string, body: SupportedBodyTypes, { byteRa if (!byteRangeContext.isRangeRequest) { return okResponse(url, body, init) } - byteRangeContext.body = body + if (!byteRangeContext.isValidRangeRequest) { + // eslint-disable-next-line no-console + console.error('Invalid range request', byteRangeContext) return badRangeResponse(url, body, init) } let response: Response - // let body: SupportedBodyTypes - try { - body = byteRangeContext.body - } catch { - return badRangeResponse(url, body, init) - } try { - response = new Response(byteRangeContext.body, { + response = new Response(body, { ...(init ?? {}), status: 206, statusText: 'Partial Content', @@ -143,8 +140,10 @@ export function okRangeResponse (url: string, body: SupportedBodyTypes, { byteRa 'content-range': byteRangeContext.contentRangeHeaderValue } }) - } catch { + } catch (e) { // TODO: should we return a different status code here? + // eslint-disable-next-line no-console + console.error('Invalid range request', e) return badRangeResponse(url, body, init) } @@ -154,6 +153,7 @@ export function okRangeResponse (url: string, body: SupportedBodyTypes, { byteRa setType(response, 'basic') setUrl(response, url) + response.headers.set('Accept-Ranges', 'bytes') return response } diff --git a/packages/verified-fetch/src/utils/splicing-transform-stream.ts b/packages/verified-fetch/src/utils/splicing-transform-stream.ts new file mode 100644 index 00000000..0835a57e --- /dev/null +++ b/packages/verified-fetch/src/utils/splicing-transform-stream.ts @@ -0,0 +1,109 @@ +import type { ComponentLogger } from '@libp2p/logger' + +/** + * Given a stream and a byte range, returns a new stream that only contains the requested range + */ +export function splicingTransformStream (originalStream: ReadableStream, offset: number | undefined, length: number | undefined, logger: ComponentLogger): ReadableStream { + const log = logger.forComponent('helia:splicing-transform-stream') + // default is noop + let transform: TransformerTransformCallback = async () => {} + let flush: TransformerFlushCallback | undefined + const offsetOnlyUseCase = offset != null && length == null // read from the offset to the end of the stream + const lengthOnlyUseCase = offset == null && length != null // only enqueue the last bytes of the stream + const offsetAndLengthUseCase = offset != null && length != null // read bytes from the offset + + if (lengthOnlyUseCase) { + const bufferSize = length // The size of the buffer to keep the last 'length' bytes + const buffer = new Uint8Array(bufferSize) // Initialize the buffer + let bufferFillSize = 0 // Track how much of the buffer is currently used + + transform = async (chunk, controller) => { + if (chunk.byteLength >= bufferSize) { + // If the chunk is larger than the entire buffer, just take the last 'bufferSize' bytes + buffer.set(chunk.slice(-bufferSize), 0) + bufferFillSize = bufferSize // The buffer is now fully filled + } else { + if (chunk.byteLength + bufferFillSize <= bufferSize) { + // If the chunk fits in the remaining space in the buffer, just add it at the end + buffer.set(chunk, bufferFillSize) + bufferFillSize += chunk.byteLength + } else { + // We need to shift existing data and add the new chunk at the end + const bytesToShift = bufferFillSize + chunk.byteLength - bufferSize + + // Shift existing data to the left by bytesToShift + buffer.copyWithin(0, bytesToShift) + + // Set the new chunk in the freed space, ensuring we don't exceed the buffer bounds + const newChunkStartIndex = Math.max(bufferFillSize - bytesToShift, 0) + buffer.set(chunk, newChunkStartIndex) + + bufferFillSize = Math.min(bufferFillSize + chunk.byteLength, bufferSize) // Ensure bufferFillSize doesn't exceed bufferSize + } + } + } + flush = async (controller) => { + if (bufferFillSize > 0) { + // Enqueue only the filled part of the buffer + controller.enqueue(buffer.slice(0, bufferFillSize)) + } + } + } else if (offsetAndLengthUseCase) { + let currentOffset = offset // Initialize with the given offset + let remainingLength = length // Track the remaining length to process + + transform = async (chunk, controller) => { + if (currentOffset >= chunk.byteLength) { + // Entire chunk is before the offset, skip it + currentOffset -= chunk.byteLength + } else if (remainingLength > 0) { + // Process chunk starting from the offset, considering the remaining length + const start = currentOffset + const end = Math.min(chunk.byteLength, start + remainingLength) + const processedChunk = chunk.slice(start, end) + controller.enqueue(processedChunk) + + // Update remaining length and reset currentOffset after first chunk + remainingLength -= processedChunk.byteLength + currentOffset = 0 + + if (remainingLength <= 0) { + // All required bytes processed, terminate the stream + controller.terminate() + } + } + } + } else if (offsetOnlyUseCase) { + // only offset provided + let currentOffset = offset + transform = async (chunk, controller) => { + if (currentOffset >= chunk.byteLength) { + // Entire chunk is before the offset, skip it + currentOffset -= chunk.byteLength + } else { + // Process chunk starting from the offset + const start = currentOffset + const processedChunk = chunk.slice(start) + controller.enqueue(processedChunk) + + // Reset currentOffset after first chunk + currentOffset = 0 + } + } + } else { + // noop + } + const { readable, writable } = new TransformStream({ + transform, + flush + }) + + void originalStream.pipeTo(writable).catch((err): void => { + if (err.message.includes('TransformStream') === true) { + // calling .terminate() on the controller will cause the transform stream to throw an error + return + } + log.error('Error piping original stream to splicing transform stream', err) + }) + return readable +} diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index e7363166..4db27531 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -282,9 +282,6 @@ export class VerifiedFetch { let ipfsRoots: CID[] | undefined let redirected = false const byteRangeContext = new ByteRangeContext(this.helia.logger, options?.headers) - if (byteRangeContext.isRangeRequest && !byteRangeContext.isValidRangeRequest) { - return badRangeResponse(resource) - } try { const pathDetails = await walkPath(this.helia.blockstore, `${cid.toString()}/${path}`, options) @@ -353,10 +350,10 @@ export class VerifiedFetch { onProgress: options?.onProgress }) byteRangeContext.fileSize = stat.fileSize - byteRangeContext.body = stream + byteRangeContext.setBody(stream) } // if not a valid range request, okRangeRequest will call okResponse - const response = okRangeResponse(resource, byteRangeContext.body, { byteRangeContext }, { + const response = okRangeResponse(resource, byteRangeContext.getBody(), { byteRangeContext }, { redirected }) @@ -370,6 +367,7 @@ export class VerifiedFetch { } catch (err: any) { this.log.error('Error streaming %c/%s', cid, path, err) if (byteRangeContext.isRangeRequest && err.code === 'ERR_INVALID_PARAMS') { + // this.log.error('Invalid range request for %c/%s', cid, path) return badRangeResponse(resource) } return internalServerErrorResponse('Unable to stream content') @@ -379,10 +377,10 @@ export class VerifiedFetch { private async handleRaw ({ resource, cid, path, options }: FetchHandlerFunctionArg): Promise { const byteRangeContext = new ByteRangeContext(this.helia.logger, options?.headers) const result = await this.helia.blockstore.get(cid, options) - byteRangeContext.body = result + byteRangeContext.setBody(result) let response: Response if (byteRangeContext.isRangeRequest) { - response = okRangeResponse(resource, result, { byteRangeContext }, { + response = okRangeResponse(resource, byteRangeContext.getBody(), { byteRangeContext }, { redirected: false }) } else { diff --git a/packages/verified-fetch/test/range-requests.spec.ts b/packages/verified-fetch/test/range-requests.spec.ts index c0978ed1..d860035d 100644 --- a/packages/verified-fetch/test/range-requests.spec.ts +++ b/packages/verified-fetch/test/range-requests.spec.ts @@ -60,7 +60,7 @@ describe('range requests', () => { beforeEach(async () => { cid = await getCid() }) - it('should return correct 206 Partial Content response for byte=-', async () => { + it('should return correct 206 Partial Content response for byte=0-5', async () => { const expected: SuccessfulTestExpectation = { byteSize: 6, contentRange: 'bytes 0-5/11' @@ -68,7 +68,7 @@ describe('range requests', () => { await testRange(cid, 'bytes=0-5', expected) }) - it('should return correct 206 Partial Content response for byte=-', async () => { + it('should return correct 206 Partial Content response for byte=4-', async () => { const expected = { byteSize: 7, contentRange: 'bytes 4-11/11' @@ -76,7 +76,7 @@ describe('range requests', () => { await testRange(cid, 'bytes=4-', expected) }) - it('should return correct 206 Partial Content response for byte=-', async () => { + it('should return correct 206 Partial Content response for byte=-9', async () => { const expected = { byteSize: 9, contentRange: 'bytes 3-11/11' diff --git a/packages/verified-fetch/test/utils/byte-range-context.spec.ts b/packages/verified-fetch/test/utils/byte-range-context.spec.ts index 74dc3fd6..bbcc5c4f 100644 --- a/packages/verified-fetch/test/utils/byte-range-context.spec.ts +++ b/packages/verified-fetch/test/utils/byte-range-context.spec.ts @@ -2,6 +2,38 @@ import { prefixLogger } from '@libp2p/logger' import { expect } from 'aegir/chai' import { ByteRangeContext } from '../../src/utils/byte-range-context.js' +/** + * You can construct a readable stream that contains a certain number of bytes, + * of a given size (chunkSize) or 1 byte by default. The Uint8Arrays in each chunk + * will be filled with the current index of the chunk. + * + * @example + * const stream = readableStreamOfSize(5) // Uint8Array(5) [0, 1, 2, 3, 4] + * const stream = readableStreamOfSize(5, 2) // Uint8Array(5) [0, 0, 1, 1, 2] + * const stream = readableStreamOfSize(5, 3) // Uint8Array(5) [0, 0, 0, 1, 1] + * const stream = readableStreamOfSize(5, 5) // Uint8Array(5) [0, 0, 0, 0, 0] + */ +function readableStreamOfSize (totalBytes: number, chunkSize: number = 1): ReadableStream { + let i = 0 + let bytesEnqueued = 0 + return new ReadableStream({ + pull (controller) { + if (bytesEnqueued >= totalBytes) { + controller.close() + return + } + const sizeForChunk = Math.min(totalBytes - bytesEnqueued, chunkSize) + const chunk = new Uint8Array(sizeForChunk) + controller.enqueue(chunk.fill(i++)) + bytesEnqueued += sizeForChunk + } + }) +} + +async function streamToUint8Array (stream: ReadableStream): Promise { + return new Response(stream).arrayBuffer().then(buffer => new Uint8Array(buffer)) +} + describe('ByteRangeContext', () => { const logger = prefixLogger('test') @@ -18,8 +50,8 @@ describe('ByteRangeContext', () => { it('should correctly set body and calculate fileSize', () => { const context = new ByteRangeContext(logger, {}) const body = new Uint8Array([1, 2, 3, 4, 5]) - context.body = body - expect(context.body).to.equal(body) + context.setBody(body) + expect(context.getBody()).to.equal(body) expect(context.fileSize).to.equal(body.length) }) @@ -37,45 +69,168 @@ describe('ByteRangeContext', () => { }) // it('should correctly slice body with valid range headers', () => { + const array = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] + const uint8arrayRangeTests = [ + // full ranges: + { type: 'Uint8Array', range: 'bytes=0-11', contentRange: 'bytes 0-11/11', body: new Uint8Array(array), expected: new Uint8Array(array) }, + { type: 'Uint8Array', range: 'bytes=-11', contentRange: 'bytes 0-11/11', body: new Uint8Array(array), expected: new Uint8Array(array) }, + { type: 'Uint8Array', range: 'bytes=0-', contentRange: 'bytes 0-11/11', body: new Uint8Array(array), expected: new Uint8Array(array) }, + + // partial ranges: + { type: 'Uint8Array', range: 'bytes=0-1', contentRange: 'bytes 0-1/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2]) }, + { type: 'Uint8Array', range: 'bytes=0-2', contentRange: 'bytes 0-2/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3]) }, + { type: 'Uint8Array', range: 'bytes=0-3', contentRange: 'bytes 0-3/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4]) }, + { type: 'Uint8Array', range: 'bytes=0-4', contentRange: 'bytes 0-4/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5]) }, + { type: 'Uint8Array', range: 'bytes=0-5', contentRange: 'bytes 0-5/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6]) }, + { type: 'Uint8Array', range: 'bytes=0-6', contentRange: 'bytes 0-6/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7]) }, + { type: 'Uint8Array', range: 'bytes=0-7', contentRange: 'bytes 0-7/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]) }, + { type: 'Uint8Array', range: 'bytes=0-8', contentRange: 'bytes 0-8/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9]) }, + { type: 'Uint8Array', range: 'bytes=0-9', contentRange: 'bytes 0-9/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) }, + { type: 'Uint8Array', range: 'bytes=0-10', contentRange: 'bytes 0-10/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]) }, + { type: 'Uint8Array', range: 'bytes=1-', contentRange: 'bytes 1-11/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(1)) }, + { type: 'Uint8Array', range: 'bytes=2-', contentRange: 'bytes 2-11/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(2)) }, + { type: 'Uint8Array', range: 'bytes=-2', contentRange: 'bytes 10-11/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(-2)) }, + + // single byte ranges: + { type: 'Uint8Array', range: 'bytes=1-1', contentRange: 'bytes 1-1/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(1, 2)) }, + { type: 'Uint8Array', range: 'bytes=-1', contentRange: 'bytes 11-11/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(-1)) } + + ] const validRanges = [ - // Uint8Arrays - { type: 'Uint8Array', range: 'bytes=0-2', contentRange: 'bytes 0-2/5', body: new Uint8Array([1, 2, 3, 4, 5]), expected: new Uint8Array([1, 2, 3]) }, - { type: 'Uint8Array', range: 'bytes=2-', contentRange: 'bytes 2-5/5', body: new Uint8Array([1, 2, 3, 4, 5]), expected: new Uint8Array([3, 4, 5]) }, - { type: 'Uint8Array', range: 'bytes=-2', contentRange: 'bytes 3-5/5', body: new Uint8Array([1, 2, 3, 4, 5]), expected: new Uint8Array([4, 5]) }, - // ArrayBuffers - { type: 'ArrayBuffer', range: 'bytes=0-2', contentRange: 'bytes 0-2/5', body: new Uint8Array([1, 2, 3, 4, 5]).buffer, expected: new Uint8Array([1, 2, 3]).buffer }, - { type: 'ArrayBuffer', range: 'bytes=2-', contentRange: 'bytes 2-5/5', body: new Uint8Array([1, 2, 3, 4, 5]).buffer, expected: new Uint8Array([3, 4, 5]).buffer }, - { type: 'ArrayBuffer', range: 'bytes=-2', contentRange: 'bytes 3-5/5', body: new Uint8Array([1, 2, 3, 4, 5]).buffer, expected: new Uint8Array([4, 5]).buffer }, - // Blobs - { type: 'Blob', range: 'bytes=0-2', contentRange: 'bytes 0-2/5', body: new Blob([new Uint8Array([1, 2, 3, 4, 5])]), expected: new Blob([new Uint8Array([1, 2, 3])]) }, - { type: 'Blob', range: 'bytes=2-', contentRange: 'bytes 2-5/5', body: new Blob([new Uint8Array([1, 2, 3, 4, 5])]), expected: new Blob([new Uint8Array([3, 4, 5])]) }, - { type: 'Blob', range: 'bytes=-2', contentRange: 'bytes 3-5/5', body: new Blob([new Uint8Array([1, 2, 3, 4, 5])]), expected: new Blob([new Uint8Array([4, 5])]) } + ...uint8arrayRangeTests, + ...uint8arrayRangeTests.map(({ range, contentRange, body, expected }) => ({ + type: 'ArrayBuffer', + range, + contentRange, + body: body.buffer, + expected: expected.buffer + })), + ...uint8arrayRangeTests.map(({ range, contentRange, body, expected }) => ({ + type: 'Blob', + range, + contentRange, + body: new Blob([body]), + expected: new Blob([expected]) + })) + // // Uint8Arrays + // // suffix range only + // // explicit full range + // // ArrayBuffers + // { type: 'ArrayBuffer', range: 'bytes=0-2', contentRange: 'bytes 0-2/5', body: new Uint8Array(array).buffer, expected: new Uint8Array([1, 2, 3]).buffer }, + // { type: 'ArrayBuffer', range: 'bytes=0-5', contentRange: 'bytes 0-5/11', body: new Uint8Array(array).buffer, expected: new Uint8Array([1, 2, 3, 4, 5, 6]).buffer }, + // { type: 'ArrayBuffer', range: 'bytes=1-1', contentRange: 'bytes 1-1/5', body: new Uint8Array(array).buffer, expected: new Uint8Array([2]).buffer }, + // { type: 'ArrayBuffer', range: 'bytes=2-', contentRange: 'bytes 2-5/5', body: new Uint8Array(array).buffer, expected: new Uint8Array([3, 4, 5]).buffer }, + // { type: 'ArrayBuffer', range: 'bytes=-2', contentRange: 'bytes 4-5/5', body: new Uint8Array(array).buffer, expected: new Uint8Array([4, 5]).buffer }, + // // Blobs + // { type: 'Blob', range: 'bytes=0-2', contentRange: 'bytes 0-2/5', body: new Blob([new Uint8Array(array)]), expected: new Blob([new Uint8Array([1, 2, 3])]) }, + // { type: 'Blob', range: 'bytes=0-5', contentRange: 'bytes 0-5/11', body: new Blob([new Uint8Array(array)]), expected: new Blob([new Uint8Array([1, 2, 3, 4, 5, 6])]) }, + // { type: 'Blob', range: 'bytes=1-1', contentRange: 'bytes 1-1/5', body: new Blob([new Uint8Array(array)]), expected: new Blob([new Uint8Array([2])]) }, + // { type: 'Blob', range: 'bytes=2-', contentRange: 'bytes 2-5/5', body: new Blob([new Uint8Array(array)]), expected: new Blob([new Uint8Array([3, 4, 5])]) }, + // { type: 'Blob', range: 'bytes=-2', contentRange: 'bytes 4-5/5', body: new Blob([new Uint8Array(array)]), expected: new Blob([new Uint8Array([4, 5])]) } ] validRanges.forEach(({ type, range, expected, body, contentRange }) => { it(`should correctly slice ${type} body with range ${range}`, () => { const context = new ByteRangeContext(logger, { Range: range }) - context.body = body - expect(context.body).to.deep.equal(expected) + context.setBody(body) + expect(context.getBody()).to.deep.equal(expected) expect(context.contentRangeHeaderValue).to.equal(contentRange) }) }) - // }) - it('should correctly handle ReadableStreams', () => { - const context = new ByteRangeContext(logger, { Range: 'bytes=0-2' }) - let i = 0 - const body = new ReadableStream({ - pull (controller) { - if (i >= 3) { + describe('handling ReadableStreams', () => { + it('readableStreamOfSize', async () => { + await expect(streamToUint8Array(readableStreamOfSize(5))).to.eventually.deep.equal(new Uint8Array([0, 1, 2, 3, 4])) + await expect(streamToUint8Array(readableStreamOfSize(5, 2))).to.eventually.deep.equal(new Uint8Array([0, 0, 1, 1, 2])) + await expect(streamToUint8Array(readableStreamOfSize(5, 3))).to.eventually.deep.equal(new Uint8Array([0, 0, 0, 1, 1])) + await expect(streamToUint8Array(readableStreamOfSize(5, 4))).to.eventually.deep.equal(new Uint8Array([0, 0, 0, 0, 1])) + await expect(streamToUint8Array(readableStreamOfSize(5, 5))).to.eventually.deep.equal(new Uint8Array([0, 0, 0, 0, 0])) + }) + it('should correctly handle bytes=1-5', async () => { + const context = new ByteRangeContext(logger, { Range: 'bytes=1-5' }) + const body = readableStreamOfSize(5, 2) + context.setBody(body) + expect(context.fileSize).to.be.null() + expect(context.contentRangeHeaderValue).to.equal('bytes 1-5/*') + // first three bytes + const processedBody = context.getBody() as ReadableStream + expect(processedBody).to.not.be.null() + await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([0, 1, 1, 2])) + }) + + it('should correctly handle bytes=2-3', async () => { + const context = new ByteRangeContext(logger, { Range: 'bytes=2-3' }) + const body = readableStreamOfSize(5, 2) + context.setBody(body) + expect(context.fileSize).to.be.null() + expect(context.contentRangeHeaderValue).to.equal('bytes 2-3/*') + const processedBody = context.getBody() as ReadableStream + expect(processedBody).to.not.be.null() + await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([1, 1])) + }) + + it('should correctly handle "bytes=1-"', async () => { + const context = new ByteRangeContext(logger, { Range: 'bytes=1-' }) + const body = readableStreamOfSize(5) + context.setBody(body) + expect(context.fileSize).to.be.null() + expect(context.contentRangeHeaderValue).to.equal('bytes 1-*/*') + const processedBody = context.getBody() as ReadableStream + const result = await streamToUint8Array(processedBody) + expect(processedBody).to.not.be.null() + expect(result).to.be.ok() + expect(result).to.deep.equal(new Uint8Array([1, 2, 3, 4])) + }) + + it('should correctly handle bytes=-3', async () => { + const context = new ByteRangeContext(logger, { Range: 'bytes=-3' }) + const body = readableStreamOfSize(5, 2) // Uint8Array(5) [0, 0, 1, 1, 2] + context.setBody(body) + expect(context.fileSize).to.be.null() + expect(context.contentRangeHeaderValue).to.equal('bytes *-3/*') // check TODO in contentRangeHeaderValue getter if this feels wrong. + const processedBody = context.getBody() as ReadableStream + expect(processedBody).to.not.be.null() + await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([1, 1, 2])) + }) + + it('should correctly handle bytes=0-5', async () => { + const context = new ByteRangeContext(logger, { Range: 'bytes=0-5' }) + const body = new ReadableStream({ + pull (controller) { + controller.enqueue(new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11])) controller.close() - return } - controller.enqueue(new Uint8Array([i++])) - } + }) + context.setBody(body) + expect(context.fileSize).to.be.null() + expect(context.contentRangeHeaderValue).to.equal('bytes 0-5/*') // check TODO in contentRangeHeaderValue getter if this feels wrong. + const processedBody = context.getBody() as ReadableStream + expect(processedBody).to.not.be.null() + await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([0, 1, 2, 3, 4, 5])) + }) + + it('should correctly handle bytes=-4', async () => { + let context = new ByteRangeContext(logger, { Range: 'bytes=-4' }) + let body = readableStreamOfSize(11, 2) // Uint8Array(5) [0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5] + context.setBody(body) + expect(context.fileSize).to.be.null() + expect(context.contentRangeHeaderValue).to.equal('bytes *-4/*') // check TODO in contentRangeHeaderValue getter if this feels wrong. + let processedBody = context.getBody() as ReadableStream + expect(processedBody).to.not.be.null() + await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([3, 4, 4, 5])) + + context = new ByteRangeContext(logger, { Range: 'bytes=-4' }) + body = new ReadableStream({ + pull (controller) { + controller.enqueue(new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11])) + controller.close() + } + }) + context.setBody(body) + expect(context.fileSize).to.be.null() + expect(context.contentRangeHeaderValue).to.equal('bytes *-4/*') // check TODO in contentRangeHeaderValue getter if this feels wrong. + processedBody = context.getBody() as ReadableStream + expect(processedBody).to.not.be.null() + await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([8, 9, 10, 11])) }) - context.body = body - expect(context.body).to.equal(body) - expect(context.fileSize).to.be.null() - expect(context.contentRangeHeaderValue).to.equal('bytes 0-2/*') }) }) diff --git a/packages/verified-fetch/test/utils/splicing-transform-stream.spec.ts b/packages/verified-fetch/test/utils/splicing-transform-stream.spec.ts new file mode 100644 index 00000000..769a8b01 --- /dev/null +++ b/packages/verified-fetch/test/utils/splicing-transform-stream.spec.ts @@ -0,0 +1,125 @@ +import { prefixLogger } from '@libp2p/logger' +import { expect } from 'aegir/chai' +import toBrowserReadableStream from 'it-to-browser-readablestream' +import { splicingTransformStream } from '../../src/utils/splicing-transform-stream.js' + +describe('splicingTransformStream', () => { + const logger = prefixLogger('test') + const streamRepresentations: Array<() => Generator> = [ + function * () { + // the whole stream in one chunk + yield new Uint8Array([0, 1, 2, 3, 4, 5]) + }, + function * () { + // 5 byte chunk, then 1 byte chunk + yield new Uint8Array([0, 1, 2, 3, 4]) + yield new Uint8Array([5]) + }, + function * () { + // 4 byte chunk, then 2 byte chunk + yield new Uint8Array([0, 1, 2, 3]) + yield new Uint8Array([4, 5]) + }, + function * () { + // 3 byte chunks + yield new Uint8Array([0, 1, 2]) + yield new Uint8Array([3, 4, 5]) + }, + function * () { + // 2 byte chunk, then 4 byte chunk + yield new Uint8Array([0, 1]) + yield new Uint8Array([2, 3, 4, 5]) + }, + function * () { + // 1 byte chunk, then 5 byte chunk + yield new Uint8Array([0]) + yield new Uint8Array([1, 2, 3, 4, 5]) + }, + function * () { + // 2 byte chunks + yield new Uint8Array([0, 1]) + yield new Uint8Array([2, 3]) + yield new Uint8Array([4, 5]) + }, + function * () { + // 2 byte chunk, 1 byte chunk, then 3 byte chunk + yield new Uint8Array([0, 1]) + yield new Uint8Array([2]) + yield new Uint8Array([3, 4, 5]) + }, + function * () { + // 3 byte chunk, 1 byte chunk, then 2 byte chunk + yield new Uint8Array([0, 1, 2]) + yield new Uint8Array([3]) + yield new Uint8Array([4, 5]) + }, + function * () { + // 2 byte chunk, two 1 byte chunks, then 2 byte chunk + yield new Uint8Array([0, 1]) + yield new Uint8Array([2]) + yield new Uint8Array([3]) + yield new Uint8Array([4, 5]) + }, + function * () { + // 2 byte chunk, then 1 byte chunks + yield new Uint8Array([0, 1]) + yield new Uint8Array([2]) + yield new Uint8Array([3]) + yield new Uint8Array([4]) + yield new Uint8Array([5]) + }, + function * () { + // 1 byte chunks + yield new Uint8Array([0]) + yield new Uint8Array([1]) + yield new Uint8Array([2]) + yield new Uint8Array([3]) + yield new Uint8Array([4]) + yield new Uint8Array([5]) + } + ] + const getActualStreamContent = (): Uint8Array => new Uint8Array([0, 1, 2, 3, 4, 5]) + describe(':offset-only', () => { + streamRepresentations.forEach((streamChunkRep, repIndex) => { + for (let offset = 0; offset <= 5; offset++) { + it(`should correctly splice the stream for "bytes=${offset}-" with streamRepresentations[${repIndex}]`, async () => { + const stream = toBrowserReadableStream(streamChunkRep()) + const transformedStream = splicingTransformStream(stream, offset, undefined, logger) + const result = await new Response(transformedStream).arrayBuffer().then((buf) => new Uint8Array(buf)) + const expected = getActualStreamContent().slice(offset) + expect(result).to.deep.equal(expected) + }) + } + }) + }) + + describe(':offset-and-length', () => { + streamRepresentations.forEach((streamChunkRep, repIndex) => { + for (let offset = 0; offset <= 5; offset++) { + for (let length = 0; length <= 5 - offset; length++) { + it(`should correctly splice the stream for "bytes=${offset}-${offset + length}" with streamRepresentations[${repIndex}]`, async () => { + const stream = toBrowserReadableStream(streamChunkRep()) + const transformedStream = splicingTransformStream(stream, offset, length, logger) + const result = await new Response(transformedStream).arrayBuffer().then((buf) => new Uint8Array(buf)) + const expected = getActualStreamContent().slice(offset, offset + length) + expect(result).to.deep.equal(expected) + }) + } + } + }) + }) + + describe(':length-only', () => { + streamRepresentations.forEach((streamChunkRep, repIndex) => { + for (let length = 1; length <= 6; length++) { + it(`should correctly splice the stream for "bytes=-${length}" with streamRepresentations[${repIndex}]`, async () => { + const stream = toBrowserReadableStream(streamChunkRep()) + const transformedStream = splicingTransformStream(stream, undefined, length, logger) + const result = await new Response(transformedStream).arrayBuffer().then((buf) => new Uint8Array(buf)) + const expected = getActualStreamContent().slice(-length) + expect(result).to.deep.equal(expected) + }) + } + }) + }) +}) From 72618bcf41d7b9ab5a2c3fd41021c0fee9f3952b Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Wed, 6 Mar 2024 15:38:30 -0800 Subject: [PATCH 11/39] chore: fixed some ByteRangeContext tests --- .../src/utils/byte-range-context.ts | 120 ++++++++++++------ .../src/utils/splicing-transform-stream.ts | 2 + 2 files changed, 86 insertions(+), 36 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 370c83a1..e91f1674 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -48,10 +48,10 @@ export class ByteRangeContext { private _isValidRangeRequest: boolean | null = null private _offset: number | undefined = undefined private _length: number | undefined = undefined - private readonly requestRangeStart!: number | undefined - private readonly requestRangeEnd!: number | undefined - private responseRangeStart!: number - private responseRangeEnd!: number + private readonly requestRangeStart: number | null + private readonly requestRangeEnd: number | null + // private responseRangeStart!: number + // private responseRangeEnd!: number constructor (private readonly logger: ComponentLogger, private readonly headers?: HeadersInit) { this.log = logger.forComponent('helia:verified-fetch:byte-range-context') @@ -61,19 +61,21 @@ export class ByteRangeContext { this._isRangeRequest = true try { const { start, end } = getByteRangeFromHeader(this._rangeRequestHeader) - this.requestRangeStart = start == null ? undefined : parseInt(start) - this.requestRangeEnd = end == null ? undefined : parseInt(end) + this.requestRangeStart = start != null ? parseInt(start) : null + this.requestRangeEnd = end != null ? parseInt(end) : null } catch (e) { this.log.error('error parsing range request header: %o', e) this.isValidRangeRequest = false + this.requestRangeStart = null + this.requestRangeEnd = null } this.setOffsetDetails() } else { this.log.trace('No range request detected') this._isRangeRequest = false - this.requestRangeStart = undefined - this.requestRangeEnd = undefined + this.requestRangeStart = null + this.requestRangeEnd = null } } @@ -98,7 +100,7 @@ export class ByteRangeContext { return body } const offset = this.offset - const length = this.length + const length = this.length != null ? this.length : undefined if (offset != null || length != null) { this.log.trace('returning body with offset %o and length %o', offset, length) if (body instanceof Uint8Array) { @@ -118,7 +120,7 @@ export class ByteRangeContext { private getSlicedBody (body: T, offset: number | undefined, length: number | undefined): T { if (offset != null && length != null) { - return body.slice(offset, offset + length + 1) as T + return body.slice(offset, offset + length) as T } else if (offset != null) { return body.slice(offset) as T } else if (length != null) { @@ -146,13 +148,17 @@ export class ByteRangeContext { public get isValidRangeRequest (): boolean { if (this.length != null && this.length < 0) { + this.log.trace('invalid range request, length is less than 0') this._isValidRangeRequest = false } else if (this.offset != null && this.offset < 0) { + this.log.trace('invalid range request, offset is less than 0') this._isValidRangeRequest = false } else if (this.length != null && this._fileSize != null && this.length > this._fileSize) { + this.log.trace('invalid range request, length(%d) is greater than fileSize(%d)', this.length, this._fileSize) this._isValidRangeRequest = false } else if (this.requestRangeStart != null && this.requestRangeEnd != null) { if (this.requestRangeStart > this.requestRangeEnd) { + this.log.trace('invalid range request, start is greater than end') this._isValidRangeRequest = false } } @@ -162,7 +168,7 @@ export class ByteRangeContext { } private set offset (val: number | undefined) { - this._offset = this._offset ?? val + this._offset = this._offset ?? val ?? undefined this.log.trace('set _offset to %o', this._offset) } @@ -201,40 +207,72 @@ export class ByteRangeContext { this.log.trace('requestRangeStart and requestRangeEnd are null') return } + if (this.requestRangeStart != null && this.requestRangeEnd != null) { // we have a specific rangeRequest start & end this.offset = this.requestRangeStart - this.length = this.requestRangeEnd - this.requestRangeStart - this.responseRangeStart = this.requestRangeStart - this.responseRangeEnd = this.requestRangeEnd + this.length = this.requestRangeEnd + // this.responseRangeEnd = this.requestRangeEnd } else if (this.requestRangeStart != null) { - // we have a specific rangeRequest start this.offset = this.requestRangeStart - this.responseRangeStart = this.offset + // we have a specific rangeRequest start if (this._fileSize != null) { - this.length = this._fileSize - this.offset - this.responseRangeEnd = this.offset + this.length + const length = this._fileSize - this.offset + this.log('only got offset, setting length to fileSize - offset', length) + this.length = length } else { - this.length = undefined + this.log.trace('only got offset, no fileSize') } - } else if (this.requestRangeEnd != null && this._fileSize != null) { - // we have a specific rangeRequest end (suffix-length) & filesize - const lengthRequested = this.requestRangeEnd - // if the user requested length of N, the offset is N bytes from the end of the file - this.offset = this._fileSize - lengthRequested - this.length = lengthRequested - this.responseRangeStart = this.offset === 0 ? this.offset : this.offset + 1 - this.responseRangeEnd = this._fileSize + // this.responseRangeEnd = this.length != null ? this.offset + this.length - 1 : 0 // Assign a default value of 0 if length is undefined } else if (this.requestRangeEnd != null) { - this.log.trace('requestRangeEnd %o, but no fileSize', this.requestRangeEnd) - // we have a specific rangeRequest end (suffix-length) but no filesize - this.offset = undefined + // we have a specific rangeRequest end (suffix-length) this.length = this.requestRangeEnd - this.responseRangeEnd = this.requestRangeEnd + this.offset = this._fileSize != null ? this._fileSize - this.length : undefined + // this.responseRangeStart = this.offset != null ? this.offset : undefined + // this.responseRangeEnd = this._fileSize != null ? this._fileSize - 1 : this.requestRangeEnd } else { this.log.trace('Not enough information to set offset and length') } } + // private setOffsetDetails (): void { + // if (this.requestRangeStart == null && this.requestRangeEnd == null) { + // this.log.trace('requestRangeStart and requestRangeEnd are null') + // return + // } + // if (this.requestRangeStart != null && this.requestRangeEnd != null) { + // // we have a specific rangeRequest start & end + // this.offset = this.requestRangeStart + // this.length = this.requestRangeEnd - this.requestRangeStart + // this.responseRangeStart = this.requestRangeStart + // this.responseRangeEnd = this.requestRangeEnd + // } else if (this.requestRangeStart != null) { + // // we have a specific rangeRequest start + // this.offset = this.requestRangeStart + // this.responseRangeStart = this.offset + // if (this._fileSize != null) { + // this.length = this._fileSize - this.offset + // this.responseRangeEnd = this.offset + this.length + // } else { + // this.length = undefined + // } + // } else if (this.requestRangeEnd != null && this._fileSize != null) { + // // we have a specific rangeRequest end (suffix-length) & filesize + // const lengthRequested = this.requestRangeEnd + // // if the user requested length of N, the offset is N bytes from the end of the file + // this.offset = this._fileSize - lengthRequested + // this.length = lengthRequested + // this.responseRangeStart = this.offset === 0 ? this.offset : this.offset + 1 + // this.responseRangeEnd = this._fileSize + // } else if (this.requestRangeEnd != null) { + // this.log.trace('requestRangeEnd %o, but no fileSize', this.requestRangeEnd) + // // we have a specific rangeRequest end (suffix-length) but no filesize + // this.offset = undefined + // this.length = this.requestRangeEnd + // this.responseRangeEnd = this.requestRangeEnd + // } else { + // this.log.trace('Not enough information to set offset and length') + // } + // } // This function returns the value of the "content-range" header. // Content-Range: -/ @@ -256,13 +294,23 @@ export class ByteRangeContext { // "bytes *-/*" // or "bytes */*" ? Are these valid? // or do we want to consume the stream and calculate the size? + let rangeStart = '*' + let rangeEnd = '*' if (this._body instanceof ReadableStream) { - const rangeStart = this.responseRangeStart ?? this.requestRangeStart ?? '*' - const rangeEnd = this.responseRangeEnd ?? this.requestRangeEnd ?? '*' - this._contentRangeHeaderValue = `bytes ${rangeStart}-${rangeEnd}/${fileSize}` - } else { - this._contentRangeHeaderValue = `bytes ${this.responseRangeStart}-${this.responseRangeEnd}/${fileSize}` + rangeStart = this.requestRangeStart?.toString() ?? '*' + rangeEnd = this.requestRangeEnd?.toString() ?? '*' + } else if (this.requestRangeEnd != null && this.requestRangeStart != null) { + rangeStart = this.requestRangeStart.toString() + rangeEnd = this.requestRangeEnd.toString() + } else if (this.requestRangeStart != null) { + rangeStart = this.requestRangeStart.toString() + rangeEnd = this._fileSize != null ? this._fileSize.toString() : '*' + } else if (this.requestRangeEnd != null) { + rangeEnd = this.requestRangeEnd.toString() + rangeStart = this._fileSize != null ? (this._fileSize - this.requestRangeEnd).toString() : '*' } + this._contentRangeHeaderValue = `bytes ${rangeStart}-${rangeEnd}/${fileSize}` + this.log.trace('contentRangeHeaderValue: %o', this._contentRangeHeaderValue) return this._contentRangeHeaderValue } } diff --git a/packages/verified-fetch/src/utils/splicing-transform-stream.ts b/packages/verified-fetch/src/utils/splicing-transform-stream.ts index 0835a57e..a864e476 100644 --- a/packages/verified-fetch/src/utils/splicing-transform-stream.ts +++ b/packages/verified-fetch/src/utils/splicing-transform-stream.ts @@ -5,12 +5,14 @@ import type { ComponentLogger } from '@libp2p/logger' */ export function splicingTransformStream (originalStream: ReadableStream, offset: number | undefined, length: number | undefined, logger: ComponentLogger): ReadableStream { const log = logger.forComponent('helia:splicing-transform-stream') + log.trace('splicingTransformStream: offset=%O, length=%O', offset, length) // default is noop let transform: TransformerTransformCallback = async () => {} let flush: TransformerFlushCallback | undefined const offsetOnlyUseCase = offset != null && length == null // read from the offset to the end of the stream const lengthOnlyUseCase = offset == null && length != null // only enqueue the last bytes of the stream const offsetAndLengthUseCase = offset != null && length != null // read bytes from the offset + log.trace('splicingTransformStream: offsetOnlyUseCase=%O, lengthOnlyUseCase=%O, offsetAndLengthUseCase=%O', offsetOnlyUseCase, lengthOnlyUseCase, offsetAndLengthUseCase) if (lengthOnlyUseCase) { const bufferSize = length // The size of the buffer to keep the last 'length' bytes From 698ee8ff9eb545703fa6c4bbc8b58834011f9f42 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 09:13:15 -0800 Subject: [PATCH 12/39] test: add back header helpers --- .../src/utils/request-headers.ts | 33 +++++++++++++++++++ .../src/utils/response-headers.ts | 21 ++++++++++++ .../test/utils/response-headers.spec.ts | 25 ++++++++++++++ 3 files changed, 79 insertions(+) create mode 100644 packages/verified-fetch/src/utils/response-headers.ts create mode 100644 packages/verified-fetch/test/utils/response-headers.spec.ts diff --git a/packages/verified-fetch/src/utils/request-headers.ts b/packages/verified-fetch/src/utils/request-headers.ts index 1686f54d..08b401a0 100644 --- a/packages/verified-fetch/src/utils/request-headers.ts +++ b/packages/verified-fetch/src/utils/request-headers.ts @@ -16,3 +16,36 @@ export function getHeader (headers: HeadersInit | undefined, header: string): st return headers[key] } + +/** + * Given two ints from a Range header, and potential fileSize, returns: + * 1. number of bytes the response should contain. + * 2. the start index of the range. // inclusive + * 3. the end index of the range. // inclusive + */ +export function calculateByteRangeIndexes (start: number | undefined, end: number | undefined, fileSize?: number): { byteSize?: number, start?: number, end?: number } { + if (start != null && end != null) { + if (start > end) { + throw new Error('Invalid range') + } + + return { byteSize: end - start + 1, start, end } + } else if (start == null && end != null) { + // suffix byte range requested + if (fileSize == null) { + return { end } + } + const result = { byteSize: end, start: fileSize - end, end: fileSize } + return result + } else if (start != null && end == null) { + if (fileSize == null) { + return { start } + } + const byteSize = fileSize - start + 1 + const end = fileSize + return { byteSize, start, end } + } + + // both start and end are undefined + return { byteSize: fileSize } +} diff --git a/packages/verified-fetch/src/utils/response-headers.ts b/packages/verified-fetch/src/utils/response-headers.ts new file mode 100644 index 00000000..710221ac --- /dev/null +++ b/packages/verified-fetch/src/utils/response-headers.ts @@ -0,0 +1,21 @@ +/** + * This function returns the value of the `Content-Range` header for a given range. + * If you know the total size of the body, you should pass it in the `options` object. + * + * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range + */ +export function getContentRangeHeader ({ byteStart, byteEnd, contentSize }: { byteStart: number | undefined, byteEnd: number | undefined, contentSize: number | undefined }): string { + const total = contentSize ?? '*' // if we don't know the total size, we should use * + + if (byteStart == null && byteEnd == null) { + return `bytes */${total}` + } + if (byteStart != null && byteEnd == null) { + return `bytes ${byteStart}-*/${total}` + } + if (byteStart == null && byteEnd != null) { + return `bytes */${total}` + } + + return `bytes ${byteStart}-${byteEnd}/${total}` +} diff --git a/packages/verified-fetch/test/utils/response-headers.spec.ts b/packages/verified-fetch/test/utils/response-headers.spec.ts new file mode 100644 index 00000000..3af988ff --- /dev/null +++ b/packages/verified-fetch/test/utils/response-headers.spec.ts @@ -0,0 +1,25 @@ +import { expect } from 'aegir/chai' +import { getContentRangeHeader } from '../../src/utils/response-headers.js' + +describe('response-headers', () => { + describe('getContentRangeHeader', () => { + it('should return correct content range header when all options are set', () => { + const start = 0 + const end = 500 + const total = 1000 + expect(getContentRangeHeader({ byteStart: start, byteEnd: end, contentSize: total })).to.equal(`bytes ${start}-${end}/${total}`) + }) + + it('should return correct content range header when only byteStart is provided', () => { + expect(getContentRangeHeader({ byteStart: 500, byteEnd: undefined, contentSize: undefined })).to.equal('bytes 500-*/*') + }) + + it('should return correct content range header when only byteEnd is provided', () => { + expect(getContentRangeHeader({ byteStart: undefined, byteEnd: 500, contentSize: undefined })).to.equal('bytes */*') + }) + + it('should return content range header with when only contentSize is provided', () => { + expect(getContentRangeHeader({ byteStart: undefined, byteEnd: undefined, contentSize: 50 })).to.equal('bytes */50') + }) + }) +}) From e413fa584c73238cafd89de508104517e7d8cba7 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 10:25:47 -0800 Subject: [PATCH 13/39] fix: unixfs tests are passing --- .../src/utils/byte-range-context.ts | 234 ++++++++---------- .../src/utils/request-headers.ts | 2 +- .../src/utils/response-headers.ts | 9 +- .../verified-fetch/src/utils/responses.ts | 2 - packages/verified-fetch/src/verified-fetch.ts | 39 +-- .../test/range-requests.spec.ts | 56 +++-- .../test/utils/request-headers.spec.ts | 30 ++- .../test/utils/response-headers.spec.ts | 20 +- 8 files changed, 208 insertions(+), 184 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index e91f1674..1c9ffef2 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -1,5 +1,6 @@ -import { getHeader } from './request-headers.js' -import { splicingTransformStream } from './splicing-transform-stream.js' +import { calculateByteRangeIndexes, getHeader } from './request-headers.js' +// import { splicingTransformStream } from './splicing-transform-stream.js' +import { getContentRangeHeader } from './response-headers.js' import type { SupportedBodyTypes } from '../types.js' import type { ComponentLogger, Logger } from '@libp2p/logger' @@ -41,19 +42,24 @@ function getByteRangeFromHeader (rangeHeader: string): { start: string, end: str export class ByteRangeContext { private readonly _isRangeRequest: boolean private _fileSize: number | null | undefined - private _contentRangeHeaderValue: string | undefined + private readonly _contentRangeHeaderValue: string | undefined private _body: SupportedBodyTypes | null = null private readonly _rangeRequestHeader: string | undefined private readonly log: Logger private _isValidRangeRequest: boolean | null = null - private _offset: number | undefined = undefined - private _length: number | undefined = undefined + // private _offset: number | undefined = undefined + // private _length: number | undefined = undefined private readonly requestRangeStart: number | null private readonly requestRangeEnd: number | null + byteStart: number | undefined + byteEnd: number | undefined + byteSize: number | undefined + + // private readonly actualLength: number | null // private responseRangeStart!: number // private responseRangeEnd!: number - constructor (private readonly logger: ComponentLogger, private readonly headers?: HeadersInit) { + constructor (logger: ComponentLogger, private readonly headers?: HeadersInit) { this.log = logger.forComponent('helia:verified-fetch:byte-range-context') this._rangeRequestHeader = getHeader(this.headers, 'Range') if (this._rangeRequestHeader != null) { @@ -63,11 +69,18 @@ export class ByteRangeContext { const { start, end } = getByteRangeFromHeader(this._rangeRequestHeader) this.requestRangeStart = start != null ? parseInt(start) : null this.requestRangeEnd = end != null ? parseInt(end) : null + if (this.requestRangeStart != null && this.requestRangeEnd != null) { + // we can set the length here + + } else { + // this.actualLength = null + } } catch (e) { this.log.error('error parsing range request header: %o', e) this.isValidRangeRequest = false this.requestRangeStart = null this.requestRangeEnd = null + // this.actualLength = null } this.setOffsetDetails() @@ -76,6 +89,7 @@ export class ByteRangeContext { this._isRangeRequest = false this.requestRangeStart = null this.requestRangeEnd = null + // this.actualLength = null } } @@ -83,13 +97,11 @@ export class ByteRangeContext { this._body = body // if fileSize was set explicitly or already set, don't recalculate it this.fileSize = this.fileSize ?? getBodySizeSync(body) - this.setOffsetDetails() this.log.trace('set request body with fileSize %o', this._fileSize) } public getBody (): SupportedBodyTypes { - this.log.trace('getting body, getBody') const body = this._body if (body == null) { this.log.trace('body is null') @@ -99,39 +111,71 @@ export class ByteRangeContext { this.log.trace('returning body unmodified') return body } - const offset = this.offset - const length = this.length != null ? this.length : undefined - if (offset != null || length != null) { - this.log.trace('returning body with offset %o and length %o', offset, length) + const byteStart = this.byteStart + const byteEnd = this.byteEnd + const byteSize = this.byteSize + if (byteStart != null || byteEnd != null) { + this.log.trace('returning body with byteStart %o byteEnd %o byteSize', byteStart, byteEnd, byteSize) if (body instanceof Uint8Array) { this.log.trace('body is Uint8Array') - return this.getSlicedBody(body, offset, length) + return this.getSlicedBody(body) } else if (body instanceof ArrayBuffer) { - return this.getSlicedBody(body, offset, length) + return this.getSlicedBody(body) } else if (body instanceof Blob) { - return this.getSlicedBody(body, offset, length) + return this.getSlicedBody(body) } else if (body instanceof ReadableStream) { - return splicingTransformStream(body, offset, length, this.logger) + // stream should already be spliced by dagPb/unixfs + return body + // return splicingTransformStream(body, offset, length, this.logger) } } // offset and length are not set, so not a range request, return body untouched. return body } - private getSlicedBody (body: T, offset: number | undefined, length: number | undefined): T { - if (offset != null && length != null) { - return body.slice(offset, offset + length) as T - } else if (offset != null) { - return body.slice(offset) as T - } else if (length != null) { - return body.slice(0, length + 1) as T - } else { - return body + private getSlicedBody (body: T): T { + // if (offset != null && length != null) { + // this.log.trace('sliced body with offset %o and length %o', offset, length) + // return body.slice(offset, offset + length) as T + // } else if (offset != null) { + // this.log.trace('sliced body with offset %o', offset) + // return body.slice(offset) as T + // } else if (length != null) { + // this.log.trace('sliced body with length %o', length) + // return body.slice(0, length + 1) as T + // } + if (this.byteStart != null && this.byteEnd != null) { + this.log.trace('sliced body with byteStart %o and byteEnd %o', this.byteStart, this.byteEnd) + return body.slice(this.byteStart, this.byteSize ?? this.byteEnd + 1) as T + } else if (this.byteStart != null) { + this.log.trace('sliced body with byteStart %o', this.byteStart) + return body.slice(this.byteStart) as T + } else if (this.byteEnd != null) { + this.log.trace('sliced body with byteEnd %o', this.byteStart) + return body.slice(-this.byteEnd) as T + } else if (this.byteSize != null) { + this.log.trace('sliced body with byteSize %o', this.byteSize) + return body.slice(0, this.byteSize + 1) as T } + + this.log.trace('returning body unmodified') + return body } + private get isSuffixLengthRequest (): boolean { + return this.requestRangeStart == null && this.requestRangeEnd != null + } + + private get isPrefixLengthRequest (): boolean { + return this.requestRangeStart != null && this.requestRangeEnd == null + } + + // sometimes, we need to set the fileSize explicitly because we can't calculate the size of the body (e.g. for unixfs content where we call .stat) public set fileSize (size: number | bigint | null) { this._fileSize = size != null ? Number(size) : null + this.log.trace('set _fileSize to %o', this._fileSize) + // if fileSize was set explicitly, we need to recalculate the offset details + this.setOffsetDetails() } public get fileSize (): number | null | undefined { @@ -167,22 +211,36 @@ export class ByteRangeContext { return this._isValidRangeRequest } - private set offset (val: number | undefined) { - this._offset = this._offset ?? val ?? undefined - this.log.trace('set _offset to %o', this._offset) - } - - public get offset (): number | undefined { - return this._offset - } - - private set length (val: number | undefined) { - this._length = val - this.log.trace('set _length to %o', this._length) + /** + * Given all the information we have, this function returns the offset that will be used when: + * 1. calling unixfs.cat + * 2. slicing the body + */ + public get offset (): number { + if (this.byteStart == null || this.byteStart === 0) { + return 0 + } + // if length is undefined, unixfs.cat will not use an inclusive offset, so we have to subtract by 1 + // TODO: file tracking issue to fix this in unixfs.cat + // if (this.length == null) { + if (this.isPrefixLengthRequest || this.isSuffixLengthRequest) { + return this.byteStart - 1 + } + // this value will be passed to unixfs.cat + return this.byteStart } + /** + * Given all the information we have, this function returns the length that will be used when: + * 1. calling unixfs.cat + * 2. slicing the body + */ public get length (): number | undefined { - return this._length + // this value will be passed to unixfs.cat. + if (this.requestRangeEnd == null) { + return undefined // this is a suffix-length request and unixfs has a bug where it doesn't always respect the length parameter + } + return this.byteSize ?? undefined } /** @@ -208,71 +266,12 @@ export class ByteRangeContext { return } - if (this.requestRangeStart != null && this.requestRangeEnd != null) { - // we have a specific rangeRequest start & end - this.offset = this.requestRangeStart - this.length = this.requestRangeEnd - // this.responseRangeEnd = this.requestRangeEnd - } else if (this.requestRangeStart != null) { - this.offset = this.requestRangeStart - // we have a specific rangeRequest start - if (this._fileSize != null) { - const length = this._fileSize - this.offset - this.log('only got offset, setting length to fileSize - offset', length) - this.length = length - } else { - this.log.trace('only got offset, no fileSize') - } - // this.responseRangeEnd = this.length != null ? this.offset + this.length - 1 : 0 // Assign a default value of 0 if length is undefined - } else if (this.requestRangeEnd != null) { - // we have a specific rangeRequest end (suffix-length) - this.length = this.requestRangeEnd - this.offset = this._fileSize != null ? this._fileSize - this.length : undefined - // this.responseRangeStart = this.offset != null ? this.offset : undefined - // this.responseRangeEnd = this._fileSize != null ? this._fileSize - 1 : this.requestRangeEnd - } else { - this.log.trace('Not enough information to set offset and length') - } + const { start, end, byteSize } = calculateByteRangeIndexes(this.requestRangeStart ?? undefined, this.requestRangeEnd ?? undefined, this._fileSize ?? undefined) + this.log.trace('set byteStart to %o, byteEnd to %o, byteSize to %o', start, end, byteSize) + this.byteStart = start + this.byteEnd = end + this.byteSize = byteSize } - // private setOffsetDetails (): void { - // if (this.requestRangeStart == null && this.requestRangeEnd == null) { - // this.log.trace('requestRangeStart and requestRangeEnd are null') - // return - // } - // if (this.requestRangeStart != null && this.requestRangeEnd != null) { - // // we have a specific rangeRequest start & end - // this.offset = this.requestRangeStart - // this.length = this.requestRangeEnd - this.requestRangeStart - // this.responseRangeStart = this.requestRangeStart - // this.responseRangeEnd = this.requestRangeEnd - // } else if (this.requestRangeStart != null) { - // // we have a specific rangeRequest start - // this.offset = this.requestRangeStart - // this.responseRangeStart = this.offset - // if (this._fileSize != null) { - // this.length = this._fileSize - this.offset - // this.responseRangeEnd = this.offset + this.length - // } else { - // this.length = undefined - // } - // } else if (this.requestRangeEnd != null && this._fileSize != null) { - // // we have a specific rangeRequest end (suffix-length) & filesize - // const lengthRequested = this.requestRangeEnd - // // if the user requested length of N, the offset is N bytes from the end of the file - // this.offset = this._fileSize - lengthRequested - // this.length = lengthRequested - // this.responseRangeStart = this.offset === 0 ? this.offset : this.offset + 1 - // this.responseRangeEnd = this._fileSize - // } else if (this.requestRangeEnd != null) { - // this.log.trace('requestRangeEnd %o, but no fileSize', this.requestRangeEnd) - // // we have a specific rangeRequest end (suffix-length) but no filesize - // this.offset = undefined - // this.length = this.requestRangeEnd - // this.responseRangeEnd = this.requestRangeEnd - // } else { - // this.log.trace('Not enough information to set offset and length') - // } - // } // This function returns the value of the "content-range" header. // Content-Range: -/ @@ -287,30 +286,11 @@ export class ByteRangeContext { this.log.error('cannot get contentRangeHeaderValue for invalid range request') throw new Error('Invalid range request') } - const fileSize = this._fileSize ?? '*' - // TODO: if the body is a Stream and we don't have the offset or length... - // do we set "content-range" to: - // "bytes -*/*" - // "bytes *-/*" - // or "bytes */*" ? Are these valid? - // or do we want to consume the stream and calculate the size? - let rangeStart = '*' - let rangeEnd = '*' - if (this._body instanceof ReadableStream) { - rangeStart = this.requestRangeStart?.toString() ?? '*' - rangeEnd = this.requestRangeEnd?.toString() ?? '*' - } else if (this.requestRangeEnd != null && this.requestRangeStart != null) { - rangeStart = this.requestRangeStart.toString() - rangeEnd = this.requestRangeEnd.toString() - } else if (this.requestRangeStart != null) { - rangeStart = this.requestRangeStart.toString() - rangeEnd = this._fileSize != null ? this._fileSize.toString() : '*' - } else if (this.requestRangeEnd != null) { - rangeEnd = this.requestRangeEnd.toString() - rangeStart = this._fileSize != null ? (this._fileSize - this.requestRangeEnd).toString() : '*' - } - this._contentRangeHeaderValue = `bytes ${rangeStart}-${rangeEnd}/${fileSize}` - this.log.trace('contentRangeHeaderValue: %o', this._contentRangeHeaderValue) - return this._contentRangeHeaderValue + + return getContentRangeHeader({ + byteStart: this.byteStart, + byteEnd: this.byteEnd, + byteSize: this._fileSize ?? undefined + }) } } diff --git a/packages/verified-fetch/src/utils/request-headers.ts b/packages/verified-fetch/src/utils/request-headers.ts index 08b401a0..3eed63a5 100644 --- a/packages/verified-fetch/src/utils/request-headers.ts +++ b/packages/verified-fetch/src/utils/request-headers.ts @@ -35,7 +35,7 @@ export function calculateByteRangeIndexes (start: number | undefined, end: numbe if (fileSize == null) { return { end } } - const result = { byteSize: end, start: fileSize - end, end: fileSize } + const result = { byteSize: end, start: fileSize - end + 1, end: fileSize } return result } else if (start != null && end == null) { if (fileSize == null) { diff --git a/packages/verified-fetch/src/utils/response-headers.ts b/packages/verified-fetch/src/utils/response-headers.ts index 710221ac..09dd9399 100644 --- a/packages/verified-fetch/src/utils/response-headers.ts +++ b/packages/verified-fetch/src/utils/response-headers.ts @@ -4,8 +4,8 @@ * * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range */ -export function getContentRangeHeader ({ byteStart, byteEnd, contentSize }: { byteStart: number | undefined, byteEnd: number | undefined, contentSize: number | undefined }): string { - const total = contentSize ?? '*' // if we don't know the total size, we should use * +export function getContentRangeHeader ({ byteStart, byteEnd, byteSize }: { byteStart: number | undefined, byteEnd: number | undefined, byteSize: number | undefined }): string { + const total = byteSize ?? '*' // if we don't know the total size, we should use * if (byteStart == null && byteEnd == null) { return `bytes */${total}` @@ -14,7 +14,10 @@ export function getContentRangeHeader ({ byteStart, byteEnd, contentSize }: { by return `bytes ${byteStart}-*/${total}` } if (byteStart == null && byteEnd != null) { - return `bytes */${total}` + if (byteSize == null) { + return `bytes */${total}` + } + return `bytes ${byteSize - byteEnd + 1}-${byteSize}/${byteSize}` } return `bytes ${byteStart}-${byteEnd}/${total}` diff --git a/packages/verified-fetch/src/utils/responses.ts b/packages/verified-fetch/src/utils/responses.ts index ee574dbf..e1f9a1b8 100644 --- a/packages/verified-fetch/src/utils/responses.ts +++ b/packages/verified-fetch/src/utils/responses.ts @@ -124,8 +124,6 @@ export function okRangeResponse (url: string, body: SupportedBodyTypes, { byteRa } if (!byteRangeContext.isValidRangeRequest) { - // eslint-disable-next-line no-console - console.error('Invalid range request', byteRangeContext) return badRangeResponse(url, body, init) } diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index 4db27531..7424e89a 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -332,11 +332,22 @@ export class VerifiedFetch { } } + if (byteRangeContext.isRangeRequest && byteRangeContext.isValidRangeRequest) { + stat = stat ?? await this.unixfs.stat(resolvedCID, { + signal: options?.signal, + onProgress: options?.onProgress + }) + this.log.trace('stat for %c/%s: %o', resolvedCID, path, stat) + byteRangeContext.fileSize = stat.fileSize + } + const offset = byteRangeContext.offset + const length = byteRangeContext.length + this.log.trace('calling unixfs.cat for %c/%s with offset=%o & length=%o', resolvedCID, path, offset, length) const asyncIter = this.unixfs.cat(resolvedCID, { signal: options?.signal, onProgress: options?.onProgress, - offset: byteRangeContext.offset, - length: byteRangeContext.length + offset, + length }) this.log('got async iterator for %c/%s', cid, path) @@ -344,14 +355,7 @@ export class VerifiedFetch { const { stream, firstChunk } = await getStreamFromAsyncIterable(asyncIter, path ?? '', this.helia.logger, { onProgress: options?.onProgress }) - if (byteRangeContext.isRangeRequest && byteRangeContext.isValidRangeRequest) { - stat = stat ?? await this.unixfs.stat(resolvedCID, { - signal: options?.signal, - onProgress: options?.onProgress - }) - byteRangeContext.fileSize = stat.fileSize - byteRangeContext.setBody(stream) - } + byteRangeContext.setBody(stream) // if not a valid range request, okRangeRequest will call okResponse const response = okRangeResponse(resource, byteRangeContext.getBody(), { byteRangeContext }, { redirected @@ -378,14 +382,13 @@ export class VerifiedFetch { const byteRangeContext = new ByteRangeContext(this.helia.logger, options?.headers) const result = await this.helia.blockstore.get(cid, options) byteRangeContext.setBody(result) - let response: Response - if (byteRangeContext.isRangeRequest) { - response = okRangeResponse(resource, byteRangeContext.getBody(), { byteRangeContext }, { - redirected: false - }) - } else { - response = okResponse(resource, result) - } + const response = okRangeResponse(resource, byteRangeContext.getBody(), { byteRangeContext }, { + redirected: false + }) + // if (byteRangeContext.isRangeRequest) { + // } else { + // response = okResponse(resource, result) + // } // if the user has specified an `Accept` header that corresponds to a raw // type, honour that header, so for example they don't request diff --git a/packages/verified-fetch/test/range-requests.spec.ts b/packages/verified-fetch/test/range-requests.spec.ts index d860035d..5ac3f420 100644 --- a/packages/verified-fetch/test/range-requests.spec.ts +++ b/packages/verified-fetch/test/range-requests.spec.ts @@ -14,6 +14,7 @@ import type { Helia } from '@helia/interface' describe('range requests', () => { let helia: Helia let verifiedFetch: VerifiedFetch + const content = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) beforeEach(async () => { helia = await createHelia() @@ -27,8 +28,9 @@ describe('range requests', () => { }) interface SuccessfulTestExpectation { - contentRange?: string - byteSize?: number + contentRange: string + // byteSize?: number + bytes: Uint8Array } async function testRange (cid: CID, headerRange: string, expected: SuccessfulTestExpectation): Promise { const response = await verifiedFetch.fetch(cid, { @@ -45,8 +47,9 @@ describe('range requests', () => { expect(contentRange).to.be.ok() expect(contentRange).to.equal(expected.contentRange) // the response should include the range that was requested - const content = await response.arrayBuffer() - expect(content.byteLength).to.equal(expected.byteSize) // the length of the data should match the requested range + const responseContent = await response.arrayBuffer() + expect(new Uint8Array(responseContent)).to.deep.equal(expected.bytes) + // expect(responseContent.byteLength).to.equal(expected.byteSize) // the length of the data should match the requested range } async function assertFailingRange (response: Promise): Promise { @@ -60,28 +63,34 @@ describe('range requests', () => { beforeEach(async () => { cid = await getCid() }) - it('should return correct 206 Partial Content response for byte=0-5', async () => { - const expected: SuccessfulTestExpectation = { + const validTestCases = [ + { byteSize: 6, - contentRange: 'bytes 0-5/11' - } - await testRange(cid, 'bytes=0-5', expected) - }) - - it('should return correct 206 Partial Content response for byte=4-', async () => { - const expected = { - byteSize: 7, - contentRange: 'bytes 4-11/11' - } - await testRange(cid, 'bytes=4-', expected) - }) - - it('should return correct 206 Partial Content response for byte=-9', async () => { - const expected = { + contentRange: 'bytes 0-5/11', + rangeHeader: 'bytes=0-5', + bytes: new Uint8Array([0, 1, 2, 3, 4, 5]) + }, + { + byteSize: 8, + contentRange: 'bytes 4-11/11', + rangeHeader: 'bytes=4-', + bytes: new Uint8Array([3, 4, 5, 6, 7, 8, 9, 10]) + }, + { byteSize: 9, - contentRange: 'bytes 3-11/11' + contentRange: 'bytes 3-11/11', + rangeHeader: 'bytes=-9', + bytes: new Uint8Array([2, 3, 4, 5, 6, 7, 8, 9, 10]) } - await testRange(cid, 'bytes=-9', expected) + ] + validTestCases.forEach(({ bytes, contentRange, rangeHeader }) => { + it(`should return correct 206 Partial Content response for ${rangeHeader}`, async () => { + const expected: SuccessfulTestExpectation = { + bytes, + contentRange + } + await testRange(cid, rangeHeader, expected) + }) }) it('should return 416 Range Not Satisfiable when the range is invalid', async () => { @@ -131,7 +140,6 @@ describe('range requests', () => { }) } - const content = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) const testTuples = [ ['unixfs', async () => { return unixfs(helia).addFile({ diff --git a/packages/verified-fetch/test/utils/request-headers.spec.ts b/packages/verified-fetch/test/utils/request-headers.spec.ts index 53697c09..1a202969 100644 --- a/packages/verified-fetch/test/utils/request-headers.spec.ts +++ b/packages/verified-fetch/test/utils/request-headers.spec.ts @@ -1,5 +1,5 @@ import { expect } from 'aegir/chai' -import { getHeader } from '../../src/utils/request-headers.js' +import { getHeader, calculateByteRangeIndexes } from '../../src/utils/request-headers.js' describe('request-headers', () => { describe('getHeader', () => { @@ -28,4 +28,32 @@ describe('request-headers', () => { expect(getHeader(headers, 'dummy')).to.equal('value') }) }) + + describe('calculateByteRangeIndexes', () => { + const testCases = [ + // Range: bytes=5- + { start: 5, end: undefined, fileSize: 10, expected: { byteSize: 6, start: 5, end: 10 } }, + // Range: bytes=-5 + { start: undefined, end: 5, fileSize: 10, expected: { byteSize: 5, start: 6, end: 10 } }, + // Range: bytes=0-0 + { start: 0, end: 0, fileSize: 10, expected: { byteSize: 1, start: 0, end: 0 } }, + // Range: bytes=5- with unknown filesize + { start: 5, end: undefined, fileSize: undefined, expected: { start: 5 } }, + // Range: bytes=-5 with unknown filesize + { start: undefined, end: 5, fileSize: undefined, expected: { end: 5 } }, + // Range: bytes=0-0 with unknown filesize + { start: 0, end: 0, fileSize: undefined, expected: { byteSize: 1, start: 0, end: 0 } }, + // Range: bytes=-9 & fileSize=11 + { start: undefined, end: 9, fileSize: 11, expected: { byteSize: 9, start: 3, end: 11 } } + ] + testCases.forEach(({ start, end, fileSize, expected }) => { + it(`should return expected result for bytes=${start ?? ''}-${end ?? ''} and fileSize=${fileSize}`, () => { + const result = calculateByteRangeIndexes(start, end, fileSize) + expect(result).to.deep.equal(expected) + }) + }) + it('throws error for invalid range', () => { + expect(() => calculateByteRangeIndexes(5, 4, 10)).to.throw('Invalid range') + }) + }) }) diff --git a/packages/verified-fetch/test/utils/response-headers.spec.ts b/packages/verified-fetch/test/utils/response-headers.spec.ts index 3af988ff..d4ed4f5a 100644 --- a/packages/verified-fetch/test/utils/response-headers.spec.ts +++ b/packages/verified-fetch/test/utils/response-headers.spec.ts @@ -4,22 +4,26 @@ import { getContentRangeHeader } from '../../src/utils/response-headers.js' describe('response-headers', () => { describe('getContentRangeHeader', () => { it('should return correct content range header when all options are set', () => { - const start = 0 - const end = 500 - const total = 1000 - expect(getContentRangeHeader({ byteStart: start, byteEnd: end, contentSize: total })).to.equal(`bytes ${start}-${end}/${total}`) + const byteStart = 0 + const byteEnd = 500 + const byteSize = 1000 + expect(getContentRangeHeader({ byteStart, byteEnd, byteSize })).to.equal(`bytes ${byteStart}-${byteEnd}/${byteSize}`) + }) + + it('should return correct content range header when only byteEnd and byteSize are provided', () => { + expect(getContentRangeHeader({ byteStart: undefined, byteEnd: 9, byteSize: 11 })).to.equal('bytes 3-11/11') }) it('should return correct content range header when only byteStart is provided', () => { - expect(getContentRangeHeader({ byteStart: 500, byteEnd: undefined, contentSize: undefined })).to.equal('bytes 500-*/*') + expect(getContentRangeHeader({ byteStart: 500, byteEnd: undefined, byteSize: undefined })).to.equal('bytes 500-*/*') }) it('should return correct content range header when only byteEnd is provided', () => { - expect(getContentRangeHeader({ byteStart: undefined, byteEnd: 500, contentSize: undefined })).to.equal('bytes */*') + expect(getContentRangeHeader({ byteStart: undefined, byteEnd: 500, byteSize: undefined })).to.equal('bytes */*') }) - it('should return content range header with when only contentSize is provided', () => { - expect(getContentRangeHeader({ byteStart: undefined, byteEnd: undefined, contentSize: 50 })).to.equal('bytes */50') + it('should return content range header with when only byteSize is provided', () => { + expect(getContentRangeHeader({ byteStart: undefined, byteEnd: undefined, byteSize: 50 })).to.equal('bytes */50') }) }) }) From 96c7f00f3af3cac8d8d8e4cc7b1712103b924782 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 10:42:00 -0800 Subject: [PATCH 14/39] fix: range-requests on raw content --- .../src/utils/byte-range-context.ts | 65 +++++++------------ 1 file changed, 23 insertions(+), 42 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 1c9ffef2..54709607 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -1,5 +1,4 @@ import { calculateByteRangeIndexes, getHeader } from './request-headers.js' -// import { splicingTransformStream } from './splicing-transform-stream.js' import { getContentRangeHeader } from './response-headers.js' import type { SupportedBodyTypes } from '../types.js' import type { ComponentLogger, Logger } from '@libp2p/logger' @@ -47,18 +46,12 @@ export class ByteRangeContext { private readonly _rangeRequestHeader: string | undefined private readonly log: Logger private _isValidRangeRequest: boolean | null = null - // private _offset: number | undefined = undefined - // private _length: number | undefined = undefined private readonly requestRangeStart: number | null private readonly requestRangeEnd: number | null byteStart: number | undefined byteEnd: number | undefined byteSize: number | undefined - // private readonly actualLength: number | null - // private responseRangeStart!: number - // private responseRangeEnd!: number - constructor (logger: ComponentLogger, private readonly headers?: HeadersInit) { this.log = logger.forComponent('helia:verified-fetch:byte-range-context') this._rangeRequestHeader = getHeader(this.headers, 'Range') @@ -69,18 +62,11 @@ export class ByteRangeContext { const { start, end } = getByteRangeFromHeader(this._rangeRequestHeader) this.requestRangeStart = start != null ? parseInt(start) : null this.requestRangeEnd = end != null ? parseInt(end) : null - if (this.requestRangeStart != null && this.requestRangeEnd != null) { - // we can set the length here - - } else { - // this.actualLength = null - } } catch (e) { this.log.error('error parsing range request header: %o', e) this.isValidRangeRequest = false this.requestRangeStart = null this.requestRangeEnd = null - // this.actualLength = null } this.setOffsetDetails() @@ -89,7 +75,6 @@ export class ByteRangeContext { this._isRangeRequest = false this.requestRangeStart = null this.requestRangeEnd = null - // this.actualLength = null } } @@ -134,32 +119,19 @@ export class ByteRangeContext { } private getSlicedBody (body: T): T { - // if (offset != null && length != null) { - // this.log.trace('sliced body with offset %o and length %o', offset, length) - // return body.slice(offset, offset + length) as T - // } else if (offset != null) { - // this.log.trace('sliced body with offset %o', offset) - // return body.slice(offset) as T - // } else if (length != null) { - // this.log.trace('sliced body with length %o', length) - // return body.slice(0, length + 1) as T - // } - if (this.byteStart != null && this.byteEnd != null) { - this.log.trace('sliced body with byteStart %o and byteEnd %o', this.byteStart, this.byteEnd) - return body.slice(this.byteStart, this.byteSize ?? this.byteEnd + 1) as T - } else if (this.byteStart != null) { + if (this.isPrefixLengthRequest) { this.log.trace('sliced body with byteStart %o', this.byteStart) - return body.slice(this.byteStart) as T - } else if (this.byteEnd != null) { - this.log.trace('sliced body with byteEnd %o', this.byteStart) - return body.slice(-this.byteEnd) as T - } else if (this.byteSize != null) { - this.log.trace('sliced body with byteSize %o', this.byteSize) - return body.slice(0, this.byteSize + 1) as T + return body.slice(this.offset) as T } + if (this.isSuffixLengthRequest && this.length != null) { + this.log.trace('sliced body with length %o', -this.length) + return body.slice(-this.length) as T + } + const offset = this.byteStart ?? 0 + const length = this.byteEnd == null ? undefined : this.byteEnd + 1 ?? undefined + this.log.trace('returning body with offset %o and length %o', offset, length) - this.log.trace('returning body unmodified') - return body + return body.slice(offset, length) as T } private get isSuffixLengthRequest (): boolean { @@ -191,7 +163,10 @@ export class ByteRangeContext { } public get isValidRangeRequest (): boolean { - if (this.length != null && this.length < 0) { + if (this.byteSize != null && this.byteSize < 0) { + this.log.trace('invalid range request, byteSize is less than 0') + this._isValidRangeRequest = false + } else if (this.length != null && this.length < 0) { this.log.trace('invalid range request, length is less than 0') this._isValidRangeRequest = false } else if (this.offset != null && this.offset < 0) { @@ -207,6 +182,14 @@ export class ByteRangeContext { } } this._isValidRangeRequest = this._isValidRangeRequest ?? true + this.log.trace('isValidRangeRequest is %o. details: %o', this._isValidRangeRequest, { + offset: this.offset, + length: this.length, + fileSize: this._fileSize, + byteStart: this.byteStart, + byteEnd: this.byteEnd, + byteSize: this.byteSize + }) return this._isValidRangeRequest } @@ -220,9 +203,7 @@ export class ByteRangeContext { if (this.byteStart == null || this.byteStart === 0) { return 0 } - // if length is undefined, unixfs.cat will not use an inclusive offset, so we have to subtract by 1 - // TODO: file tracking issue to fix this in unixfs.cat - // if (this.length == null) { + // if length is undefined, unixfs.cat and ArrayBuffer.slice will not use an inclusive offset, so we have to subtract by 1 if (this.isPrefixLengthRequest || this.isSuffixLengthRequest) { return this.byteStart - 1 } From deb2f2b8960aa8ae63db0ef17947806dbf253354 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 14:32:11 -0800 Subject: [PATCH 15/39] feat: tests are passing moved transform stream over to https://github.com/SgtPooki/streams --- .../src/utils/byte-range-context.ts | 57 ++++-- .../src/utils/splicing-transform-stream.ts | 111 ----------- .../test/utils/byte-range-context.spec.ts | 182 ++++-------------- .../test/utils/request-headers.spec.ts | 4 +- .../utils/splicing-transform-stream.spec.ts | 125 ------------ 5 files changed, 82 insertions(+), 397 deletions(-) delete mode 100644 packages/verified-fetch/src/utils/splicing-transform-stream.ts delete mode 100644 packages/verified-fetch/test/utils/splicing-transform-stream.spec.ts diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 54709607..a80f03d2 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -128,7 +128,7 @@ export class ByteRangeContext { return body.slice(-this.length) as T } const offset = this.byteStart ?? 0 - const length = this.byteEnd == null ? undefined : this.byteEnd + 1 ?? undefined + const length = this.byteEnd == null ? undefined : this.byteEnd + 1 this.log.trace('returning body with offset %o and length %o', offset, length) return body.slice(offset, length) as T @@ -158,38 +158,55 @@ export class ByteRangeContext { return this._isRangeRequest } + private isValidByteStart (): boolean { + if (this.byteStart != null) { + if (this.byteStart < 0) { + return false + } + if (this.fileSize != null && this.byteStart > this.fileSize) { + return false + } + } + return true + } + + private isValidByteEnd (): boolean { + if (this.byteEnd != null) { + if (this.byteEnd < 0) { + return false + } + if (this.fileSize != null && this.byteEnd > this.fileSize) { + return false + } + } + return true + } + public set isValidRangeRequest (val: boolean) { this._isValidRangeRequest = val } public get isValidRangeRequest (): boolean { - if (this.byteSize != null && this.byteSize < 0) { - this.log.trace('invalid range request, byteSize is less than 0') - this._isValidRangeRequest = false - } else if (this.length != null && this.length < 0) { - this.log.trace('invalid range request, length is less than 0') + if (!this.isValidByteStart()) { + this.log.trace('invalid range request, byteStart is less than 0 or greater than fileSize') this._isValidRangeRequest = false - } else if (this.offset != null && this.offset < 0) { - this.log.trace('invalid range request, offset is less than 0') + } else if (!this.isValidByteEnd()) { + this.log.trace('invalid range request, byteEnd is less than 0 or greater than fileSize') this._isValidRangeRequest = false - } else if (this.length != null && this._fileSize != null && this.length > this._fileSize) { - this.log.trace('invalid range request, length(%d) is greater than fileSize(%d)', this.length, this._fileSize) - this._isValidRangeRequest = false - } else if (this.requestRangeStart != null && this.requestRangeEnd != null) { + } else if (this.requestRangeEnd != null && this.requestRangeStart != null) { + // we may not have enough info.. base check on requested bytes if (this.requestRangeStart > this.requestRangeEnd) { this.log.trace('invalid range request, start is greater than end') this._isValidRangeRequest = false + } else if (this.requestRangeStart < 0) { + this.log.trace('invalid range request, start is less than 0') + this._isValidRangeRequest = false + } else if (this.requestRangeEnd < 0) { + this.log.trace('invalid range request, end is less than 0') + this._isValidRangeRequest = false } } this._isValidRangeRequest = this._isValidRangeRequest ?? true - this.log.trace('isValidRangeRequest is %o. details: %o', this._isValidRangeRequest, { - offset: this.offset, - length: this.length, - fileSize: this._fileSize, - byteStart: this.byteStart, - byteEnd: this.byteEnd, - byteSize: this.byteSize - }) return this._isValidRangeRequest } diff --git a/packages/verified-fetch/src/utils/splicing-transform-stream.ts b/packages/verified-fetch/src/utils/splicing-transform-stream.ts deleted file mode 100644 index a864e476..00000000 --- a/packages/verified-fetch/src/utils/splicing-transform-stream.ts +++ /dev/null @@ -1,111 +0,0 @@ -import type { ComponentLogger } from '@libp2p/logger' - -/** - * Given a stream and a byte range, returns a new stream that only contains the requested range - */ -export function splicingTransformStream (originalStream: ReadableStream, offset: number | undefined, length: number | undefined, logger: ComponentLogger): ReadableStream { - const log = logger.forComponent('helia:splicing-transform-stream') - log.trace('splicingTransformStream: offset=%O, length=%O', offset, length) - // default is noop - let transform: TransformerTransformCallback = async () => {} - let flush: TransformerFlushCallback | undefined - const offsetOnlyUseCase = offset != null && length == null // read from the offset to the end of the stream - const lengthOnlyUseCase = offset == null && length != null // only enqueue the last bytes of the stream - const offsetAndLengthUseCase = offset != null && length != null // read bytes from the offset - log.trace('splicingTransformStream: offsetOnlyUseCase=%O, lengthOnlyUseCase=%O, offsetAndLengthUseCase=%O', offsetOnlyUseCase, lengthOnlyUseCase, offsetAndLengthUseCase) - - if (lengthOnlyUseCase) { - const bufferSize = length // The size of the buffer to keep the last 'length' bytes - const buffer = new Uint8Array(bufferSize) // Initialize the buffer - let bufferFillSize = 0 // Track how much of the buffer is currently used - - transform = async (chunk, controller) => { - if (chunk.byteLength >= bufferSize) { - // If the chunk is larger than the entire buffer, just take the last 'bufferSize' bytes - buffer.set(chunk.slice(-bufferSize), 0) - bufferFillSize = bufferSize // The buffer is now fully filled - } else { - if (chunk.byteLength + bufferFillSize <= bufferSize) { - // If the chunk fits in the remaining space in the buffer, just add it at the end - buffer.set(chunk, bufferFillSize) - bufferFillSize += chunk.byteLength - } else { - // We need to shift existing data and add the new chunk at the end - const bytesToShift = bufferFillSize + chunk.byteLength - bufferSize - - // Shift existing data to the left by bytesToShift - buffer.copyWithin(0, bytesToShift) - - // Set the new chunk in the freed space, ensuring we don't exceed the buffer bounds - const newChunkStartIndex = Math.max(bufferFillSize - bytesToShift, 0) - buffer.set(chunk, newChunkStartIndex) - - bufferFillSize = Math.min(bufferFillSize + chunk.byteLength, bufferSize) // Ensure bufferFillSize doesn't exceed bufferSize - } - } - } - flush = async (controller) => { - if (bufferFillSize > 0) { - // Enqueue only the filled part of the buffer - controller.enqueue(buffer.slice(0, bufferFillSize)) - } - } - } else if (offsetAndLengthUseCase) { - let currentOffset = offset // Initialize with the given offset - let remainingLength = length // Track the remaining length to process - - transform = async (chunk, controller) => { - if (currentOffset >= chunk.byteLength) { - // Entire chunk is before the offset, skip it - currentOffset -= chunk.byteLength - } else if (remainingLength > 0) { - // Process chunk starting from the offset, considering the remaining length - const start = currentOffset - const end = Math.min(chunk.byteLength, start + remainingLength) - const processedChunk = chunk.slice(start, end) - controller.enqueue(processedChunk) - - // Update remaining length and reset currentOffset after first chunk - remainingLength -= processedChunk.byteLength - currentOffset = 0 - - if (remainingLength <= 0) { - // All required bytes processed, terminate the stream - controller.terminate() - } - } - } - } else if (offsetOnlyUseCase) { - // only offset provided - let currentOffset = offset - transform = async (chunk, controller) => { - if (currentOffset >= chunk.byteLength) { - // Entire chunk is before the offset, skip it - currentOffset -= chunk.byteLength - } else { - // Process chunk starting from the offset - const start = currentOffset - const processedChunk = chunk.slice(start) - controller.enqueue(processedChunk) - - // Reset currentOffset after first chunk - currentOffset = 0 - } - } - } else { - // noop - } - const { readable, writable } = new TransformStream({ - transform, - flush - }) - - void originalStream.pipeTo(writable).catch((err): void => { - if (err.message.includes('TransformStream') === true) { - // calling .terminate() on the controller will cause the transform stream to throw an error - return - } - log.error('Error piping original stream to splicing transform stream', err) - }) - return readable -} diff --git a/packages/verified-fetch/test/utils/byte-range-context.spec.ts b/packages/verified-fetch/test/utils/byte-range-context.spec.ts index bbcc5c4f..d96328fc 100644 --- a/packages/verified-fetch/test/utils/byte-range-context.spec.ts +++ b/packages/verified-fetch/test/utils/byte-range-context.spec.ts @@ -1,38 +1,14 @@ -import { prefixLogger } from '@libp2p/logger' +import { unixfs, type UnixFS } from '@helia/unixfs' +import { stop } from '@libp2p/interface' +import { defaultLogger, prefixLogger } from '@libp2p/logger' import { expect } from 'aegir/chai' +import toIt from 'browser-readablestream-to-it' +import all from 'it-all' import { ByteRangeContext } from '../../src/utils/byte-range-context.js' - -/** - * You can construct a readable stream that contains a certain number of bytes, - * of a given size (chunkSize) or 1 byte by default. The Uint8Arrays in each chunk - * will be filled with the current index of the chunk. - * - * @example - * const stream = readableStreamOfSize(5) // Uint8Array(5) [0, 1, 2, 3, 4] - * const stream = readableStreamOfSize(5, 2) // Uint8Array(5) [0, 0, 1, 1, 2] - * const stream = readableStreamOfSize(5, 3) // Uint8Array(5) [0, 0, 0, 1, 1] - * const stream = readableStreamOfSize(5, 5) // Uint8Array(5) [0, 0, 0, 0, 0] - */ -function readableStreamOfSize (totalBytes: number, chunkSize: number = 1): ReadableStream { - let i = 0 - let bytesEnqueued = 0 - return new ReadableStream({ - pull (controller) { - if (bytesEnqueued >= totalBytes) { - controller.close() - return - } - const sizeForChunk = Math.min(totalBytes - bytesEnqueued, chunkSize) - const chunk = new Uint8Array(sizeForChunk) - controller.enqueue(chunk.fill(i++)) - bytesEnqueued += sizeForChunk - } - }) -} - -async function streamToUint8Array (stream: ReadableStream): Promise { - return new Response(stream).arrayBuffer().then(buffer => new Uint8Array(buffer)) -} +import { getStreamFromAsyncIterable } from '../../src/utils/get-stream-from-async-iterable.js' +import { createHelia } from '../fixtures/create-offline-helia.js' +import type { Helia } from 'helia' +import type { CID } from 'multiformats/cid' describe('ByteRangeContext', () => { const logger = prefixLogger('test') @@ -68,12 +44,11 @@ describe('ByteRangeContext', () => { }) }) - // it('should correctly slice body with valid range headers', () => { const array = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] const uint8arrayRangeTests = [ // full ranges: { type: 'Uint8Array', range: 'bytes=0-11', contentRange: 'bytes 0-11/11', body: new Uint8Array(array), expected: new Uint8Array(array) }, - { type: 'Uint8Array', range: 'bytes=-11', contentRange: 'bytes 0-11/11', body: new Uint8Array(array), expected: new Uint8Array(array) }, + { type: 'Uint8Array', range: 'bytes=-11', contentRange: 'bytes 1-11/11', body: new Uint8Array(array), expected: new Uint8Array(array) }, { type: 'Uint8Array', range: 'bytes=0-', contentRange: 'bytes 0-11/11', body: new Uint8Array(array), expected: new Uint8Array(array) }, // partial ranges: @@ -87,8 +62,8 @@ describe('ByteRangeContext', () => { { type: 'Uint8Array', range: 'bytes=0-8', contentRange: 'bytes 0-8/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9]) }, { type: 'Uint8Array', range: 'bytes=0-9', contentRange: 'bytes 0-9/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) }, { type: 'Uint8Array', range: 'bytes=0-10', contentRange: 'bytes 0-10/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]) }, - { type: 'Uint8Array', range: 'bytes=1-', contentRange: 'bytes 1-11/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(1)) }, - { type: 'Uint8Array', range: 'bytes=2-', contentRange: 'bytes 2-11/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(2)) }, + { type: 'Uint8Array', range: 'bytes=1-', contentRange: 'bytes 1-11/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]) }, + { type: 'Uint8Array', range: 'bytes=2-', contentRange: 'bytes 2-11/11', body: new Uint8Array(array), expected: new Uint8Array([2, 3, 4, 5, 6, 7, 8, 9, 10, 11]) }, { type: 'Uint8Array', range: 'bytes=-2', contentRange: 'bytes 10-11/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(-2)) }, // single byte ranges: @@ -112,21 +87,6 @@ describe('ByteRangeContext', () => { body: new Blob([body]), expected: new Blob([expected]) })) - // // Uint8Arrays - // // suffix range only - // // explicit full range - // // ArrayBuffers - // { type: 'ArrayBuffer', range: 'bytes=0-2', contentRange: 'bytes 0-2/5', body: new Uint8Array(array).buffer, expected: new Uint8Array([1, 2, 3]).buffer }, - // { type: 'ArrayBuffer', range: 'bytes=0-5', contentRange: 'bytes 0-5/11', body: new Uint8Array(array).buffer, expected: new Uint8Array([1, 2, 3, 4, 5, 6]).buffer }, - // { type: 'ArrayBuffer', range: 'bytes=1-1', contentRange: 'bytes 1-1/5', body: new Uint8Array(array).buffer, expected: new Uint8Array([2]).buffer }, - // { type: 'ArrayBuffer', range: 'bytes=2-', contentRange: 'bytes 2-5/5', body: new Uint8Array(array).buffer, expected: new Uint8Array([3, 4, 5]).buffer }, - // { type: 'ArrayBuffer', range: 'bytes=-2', contentRange: 'bytes 4-5/5', body: new Uint8Array(array).buffer, expected: new Uint8Array([4, 5]).buffer }, - // // Blobs - // { type: 'Blob', range: 'bytes=0-2', contentRange: 'bytes 0-2/5', body: new Blob([new Uint8Array(array)]), expected: new Blob([new Uint8Array([1, 2, 3])]) }, - // { type: 'Blob', range: 'bytes=0-5', contentRange: 'bytes 0-5/11', body: new Blob([new Uint8Array(array)]), expected: new Blob([new Uint8Array([1, 2, 3, 4, 5, 6])]) }, - // { type: 'Blob', range: 'bytes=1-1', contentRange: 'bytes 1-1/5', body: new Blob([new Uint8Array(array)]), expected: new Blob([new Uint8Array([2])]) }, - // { type: 'Blob', range: 'bytes=2-', contentRange: 'bytes 2-5/5', body: new Blob([new Uint8Array(array)]), expected: new Blob([new Uint8Array([3, 4, 5])]) }, - // { type: 'Blob', range: 'bytes=-2', contentRange: 'bytes 4-5/5', body: new Blob([new Uint8Array(array)]), expected: new Blob([new Uint8Array([4, 5])]) } ] validRanges.forEach(({ type, range, expected, body, contentRange }) => { it(`should correctly slice ${type} body with range ${range}`, () => { @@ -137,100 +97,42 @@ describe('ByteRangeContext', () => { }) }) - describe('handling ReadableStreams', () => { - it('readableStreamOfSize', async () => { - await expect(streamToUint8Array(readableStreamOfSize(5))).to.eventually.deep.equal(new Uint8Array([0, 1, 2, 3, 4])) - await expect(streamToUint8Array(readableStreamOfSize(5, 2))).to.eventually.deep.equal(new Uint8Array([0, 0, 1, 1, 2])) - await expect(streamToUint8Array(readableStreamOfSize(5, 3))).to.eventually.deep.equal(new Uint8Array([0, 0, 0, 1, 1])) - await expect(streamToUint8Array(readableStreamOfSize(5, 4))).to.eventually.deep.equal(new Uint8Array([0, 0, 0, 0, 1])) - await expect(streamToUint8Array(readableStreamOfSize(5, 5))).to.eventually.deep.equal(new Uint8Array([0, 0, 0, 0, 0])) - }) - it('should correctly handle bytes=1-5', async () => { - const context = new ByteRangeContext(logger, { Range: 'bytes=1-5' }) - const body = readableStreamOfSize(5, 2) - context.setBody(body) - expect(context.fileSize).to.be.null() - expect(context.contentRangeHeaderValue).to.equal('bytes 1-5/*') - // first three bytes - const processedBody = context.getBody() as ReadableStream - expect(processedBody).to.not.be.null() - await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([0, 1, 1, 2])) - }) - - it('should correctly handle bytes=2-3', async () => { - const context = new ByteRangeContext(logger, { Range: 'bytes=2-3' }) - const body = readableStreamOfSize(5, 2) - context.setBody(body) - expect(context.fileSize).to.be.null() - expect(context.contentRangeHeaderValue).to.equal('bytes 2-3/*') - const processedBody = context.getBody() as ReadableStream - expect(processedBody).to.not.be.null() - await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([1, 1])) - }) - - it('should correctly handle "bytes=1-"', async () => { - const context = new ByteRangeContext(logger, { Range: 'bytes=1-' }) - const body = readableStreamOfSize(5) - context.setBody(body) - expect(context.fileSize).to.be.null() - expect(context.contentRangeHeaderValue).to.equal('bytes 1-*/*') - const processedBody = context.getBody() as ReadableStream - const result = await streamToUint8Array(processedBody) - expect(processedBody).to.not.be.null() - expect(result).to.be.ok() - expect(result).to.deep.equal(new Uint8Array([1, 2, 3, 4])) - }) + describe.skip('handling ReadableStreams', () => { + let helia: Helia + let fs: UnixFS + let cid: CID + const getBodyStream = async (offset?: number, length?: number): Promise> => { + const iter = fs.cat(cid, { offset, length }) + const { stream } = await getStreamFromAsyncIterable(iter, 'test.txt', defaultLogger()) + return stream + } - it('should correctly handle bytes=-3', async () => { - const context = new ByteRangeContext(logger, { Range: 'bytes=-3' }) - const body = readableStreamOfSize(5, 2) // Uint8Array(5) [0, 0, 1, 1, 2] - context.setBody(body) - expect(context.fileSize).to.be.null() - expect(context.contentRangeHeaderValue).to.equal('bytes *-3/*') // check TODO in contentRangeHeaderValue getter if this feels wrong. - const processedBody = context.getBody() as ReadableStream - expect(processedBody).to.not.be.null() - await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([1, 1, 2])) + before(async () => { + helia = await createHelia() + fs = unixfs(helia) }) - it('should correctly handle bytes=0-5', async () => { - const context = new ByteRangeContext(logger, { Range: 'bytes=0-5' }) - const body = new ReadableStream({ - pull (controller) { - controller.enqueue(new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11])) - controller.close() - } - }) - context.setBody(body) - expect(context.fileSize).to.be.null() - expect(context.contentRangeHeaderValue).to.equal('bytes 0-5/*') // check TODO in contentRangeHeaderValue getter if this feels wrong. - const processedBody = context.getBody() as ReadableStream - expect(processedBody).to.not.be.null() - await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([0, 1, 2, 3, 4, 5])) + after(async () => { + await stop(helia) }) - it('should correctly handle bytes=-4', async () => { - let context = new ByteRangeContext(logger, { Range: 'bytes=-4' }) - let body = readableStreamOfSize(11, 2) // Uint8Array(5) [0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5] - context.setBody(body) - expect(context.fileSize).to.be.null() - expect(context.contentRangeHeaderValue).to.equal('bytes *-4/*') // check TODO in contentRangeHeaderValue getter if this feels wrong. - let processedBody = context.getBody() as ReadableStream - expect(processedBody).to.not.be.null() - await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([3, 4, 4, 5])) - - context = new ByteRangeContext(logger, { Range: 'bytes=-4' }) - body = new ReadableStream({ - pull (controller) { - controller.enqueue(new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11])) - controller.close() - } + uint8arrayRangeTests.forEach(({ range, expected, body, contentRange }) => { + it(`should correctly slice Stream with range ${range}`, async () => { + const context = new ByteRangeContext(logger, { Range: range }) + cid = await fs.addFile({ + content: body + }, { + rawLeaves: false, + leafType: 'file' + }) + const stat = await fs.stat(cid) + context.fileSize = stat.fileSize + + context.setBody(await getBodyStream(context.offset, context.length)) + const bodyResult = await all(toIt(context.getBody() as ReadableStream)) + expect(bodyResult).to.deep.equal(expected) + expect(context.contentRangeHeaderValue).to.equal(contentRange) }) - context.setBody(body) - expect(context.fileSize).to.be.null() - expect(context.contentRangeHeaderValue).to.equal('bytes *-4/*') // check TODO in contentRangeHeaderValue getter if this feels wrong. - processedBody = context.getBody() as ReadableStream - expect(processedBody).to.not.be.null() - await expect(streamToUint8Array(processedBody)).to.eventually.deep.equal(new Uint8Array([8, 9, 10, 11])) }) }) }) diff --git a/packages/verified-fetch/test/utils/request-headers.spec.ts b/packages/verified-fetch/test/utils/request-headers.spec.ts index 1a202969..77c1697e 100644 --- a/packages/verified-fetch/test/utils/request-headers.spec.ts +++ b/packages/verified-fetch/test/utils/request-headers.spec.ts @@ -44,7 +44,9 @@ describe('request-headers', () => { // Range: bytes=0-0 with unknown filesize { start: 0, end: 0, fileSize: undefined, expected: { byteSize: 1, start: 0, end: 0 } }, // Range: bytes=-9 & fileSize=11 - { start: undefined, end: 9, fileSize: 11, expected: { byteSize: 9, start: 3, end: 11 } } + { start: undefined, end: 9, fileSize: 11, expected: { byteSize: 9, start: 3, end: 11 } }, + // Range: bytes=0-11 & fileSize=11 + { start: 0, end: 11, fileSize: 11, expected: { byteSize: 12, start: 0, end: 11 } } ] testCases.forEach(({ start, end, fileSize, expected }) => { it(`should return expected result for bytes=${start ?? ''}-${end ?? ''} and fileSize=${fileSize}`, () => { diff --git a/packages/verified-fetch/test/utils/splicing-transform-stream.spec.ts b/packages/verified-fetch/test/utils/splicing-transform-stream.spec.ts deleted file mode 100644 index 769a8b01..00000000 --- a/packages/verified-fetch/test/utils/splicing-transform-stream.spec.ts +++ /dev/null @@ -1,125 +0,0 @@ -import { prefixLogger } from '@libp2p/logger' -import { expect } from 'aegir/chai' -import toBrowserReadableStream from 'it-to-browser-readablestream' -import { splicingTransformStream } from '../../src/utils/splicing-transform-stream.js' - -describe('splicingTransformStream', () => { - const logger = prefixLogger('test') - const streamRepresentations: Array<() => Generator> = [ - function * () { - // the whole stream in one chunk - yield new Uint8Array([0, 1, 2, 3, 4, 5]) - }, - function * () { - // 5 byte chunk, then 1 byte chunk - yield new Uint8Array([0, 1, 2, 3, 4]) - yield new Uint8Array([5]) - }, - function * () { - // 4 byte chunk, then 2 byte chunk - yield new Uint8Array([0, 1, 2, 3]) - yield new Uint8Array([4, 5]) - }, - function * () { - // 3 byte chunks - yield new Uint8Array([0, 1, 2]) - yield new Uint8Array([3, 4, 5]) - }, - function * () { - // 2 byte chunk, then 4 byte chunk - yield new Uint8Array([0, 1]) - yield new Uint8Array([2, 3, 4, 5]) - }, - function * () { - // 1 byte chunk, then 5 byte chunk - yield new Uint8Array([0]) - yield new Uint8Array([1, 2, 3, 4, 5]) - }, - function * () { - // 2 byte chunks - yield new Uint8Array([0, 1]) - yield new Uint8Array([2, 3]) - yield new Uint8Array([4, 5]) - }, - function * () { - // 2 byte chunk, 1 byte chunk, then 3 byte chunk - yield new Uint8Array([0, 1]) - yield new Uint8Array([2]) - yield new Uint8Array([3, 4, 5]) - }, - function * () { - // 3 byte chunk, 1 byte chunk, then 2 byte chunk - yield new Uint8Array([0, 1, 2]) - yield new Uint8Array([3]) - yield new Uint8Array([4, 5]) - }, - function * () { - // 2 byte chunk, two 1 byte chunks, then 2 byte chunk - yield new Uint8Array([0, 1]) - yield new Uint8Array([2]) - yield new Uint8Array([3]) - yield new Uint8Array([4, 5]) - }, - function * () { - // 2 byte chunk, then 1 byte chunks - yield new Uint8Array([0, 1]) - yield new Uint8Array([2]) - yield new Uint8Array([3]) - yield new Uint8Array([4]) - yield new Uint8Array([5]) - }, - function * () { - // 1 byte chunks - yield new Uint8Array([0]) - yield new Uint8Array([1]) - yield new Uint8Array([2]) - yield new Uint8Array([3]) - yield new Uint8Array([4]) - yield new Uint8Array([5]) - } - ] - const getActualStreamContent = (): Uint8Array => new Uint8Array([0, 1, 2, 3, 4, 5]) - describe(':offset-only', () => { - streamRepresentations.forEach((streamChunkRep, repIndex) => { - for (let offset = 0; offset <= 5; offset++) { - it(`should correctly splice the stream for "bytes=${offset}-" with streamRepresentations[${repIndex}]`, async () => { - const stream = toBrowserReadableStream(streamChunkRep()) - const transformedStream = splicingTransformStream(stream, offset, undefined, logger) - const result = await new Response(transformedStream).arrayBuffer().then((buf) => new Uint8Array(buf)) - const expected = getActualStreamContent().slice(offset) - expect(result).to.deep.equal(expected) - }) - } - }) - }) - - describe(':offset-and-length', () => { - streamRepresentations.forEach((streamChunkRep, repIndex) => { - for (let offset = 0; offset <= 5; offset++) { - for (let length = 0; length <= 5 - offset; length++) { - it(`should correctly splice the stream for "bytes=${offset}-${offset + length}" with streamRepresentations[${repIndex}]`, async () => { - const stream = toBrowserReadableStream(streamChunkRep()) - const transformedStream = splicingTransformStream(stream, offset, length, logger) - const result = await new Response(transformedStream).arrayBuffer().then((buf) => new Uint8Array(buf)) - const expected = getActualStreamContent().slice(offset, offset + length) - expect(result).to.deep.equal(expected) - }) - } - } - }) - }) - - describe(':length-only', () => { - streamRepresentations.forEach((streamChunkRep, repIndex) => { - for (let length = 1; length <= 6; length++) { - it(`should correctly splice the stream for "bytes=-${length}" with streamRepresentations[${repIndex}]`, async () => { - const stream = toBrowserReadableStream(streamChunkRep()) - const transformedStream = splicingTransformStream(stream, undefined, length, logger) - const result = await new Response(transformedStream).arrayBuffer().then((buf) => new Uint8Array(buf)) - const expected = getActualStreamContent().slice(-length) - expect(result).to.deep.equal(expected) - }) - } - }) - }) -}) From f357a3d5eaa240a4808c8461cce65619fba2f1e4 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 14:38:47 -0800 Subject: [PATCH 16/39] chore: log string casing --- .../verified-fetch/src/utils/byte-range-context.ts | 4 ++-- .../src/utils/get-stream-from-async-iterable.ts | 2 +- packages/verified-fetch/src/utils/parse-url-string.ts | 10 +++++----- packages/verified-fetch/src/verified-fetch.ts | 9 ++++----- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index a80f03d2..655c56d9 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -56,7 +56,7 @@ export class ByteRangeContext { this.log = logger.forComponent('helia:verified-fetch:byte-range-context') this._rangeRequestHeader = getHeader(this.headers, 'Range') if (this._rangeRequestHeader != null) { - this.log.trace('Range request detected') + this.log.trace('range request detected') this._isRangeRequest = true try { const { start, end } = getByteRangeFromHeader(this._rangeRequestHeader) @@ -71,7 +71,7 @@ export class ByteRangeContext { this.setOffsetDetails() } else { - this.log.trace('No range request detected') + this.log.trace('no range request detected') this._isRangeRequest = false this.requestRangeStart = null this.requestRangeEnd = null diff --git a/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts b/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts index f57e97f0..be5b0282 100644 --- a/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts +++ b/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts @@ -11,7 +11,7 @@ export async function getStreamFromAsyncIterable (iterator: AsyncIterable Date: Thu, 7 Mar 2024 14:43:52 -0800 Subject: [PATCH 17/39] chore: use 502 response instead of 500 --- packages/verified-fetch/src/utils/responses.ts | 6 +++--- packages/verified-fetch/src/verified-fetch.ts | 6 +++--- packages/verified-fetch/test/custom-dns-resolvers.spec.ts | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/verified-fetch/src/utils/responses.ts b/packages/verified-fetch/src/utils/responses.ts index e1f9a1b8..a8ce2336 100644 --- a/packages/verified-fetch/src/utils/responses.ts +++ b/packages/verified-fetch/src/utils/responses.ts @@ -44,11 +44,11 @@ export function okResponse (url: string, body?: SupportedBodyTypes, init?: Respo return response } -export function internalServerErrorResponse (url: string, body?: SupportedBodyTypes, init?: ResponseInit): Response { +export function badGatewayResponse (url: string, body?: SupportedBodyTypes, init?: ResponseInit): Response { const response = new Response(body, { ...(init ?? {}), - status: 500, - statusText: 'Internal Server Error' + status: 502, + statusText: 'Bad Gateway' }) setType(response, 'basic') diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index 7f8ca636..782c4616 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -23,7 +23,7 @@ import { getETag } from './utils/get-e-tag.js' import { getStreamFromAsyncIterable } from './utils/get-stream-from-async-iterable.js' import { tarStream } from './utils/get-tar-stream.js' import { parseResource } from './utils/parse-resource.js' -import { badRequestResponse, movedPermanentlyResponse, notAcceptableResponse, notSupportedResponse, okResponse, badRangeResponse, internalServerErrorResponse, okRangeResponse } from './utils/responses.js' +import { badRequestResponse, movedPermanentlyResponse, notAcceptableResponse, notSupportedResponse, okResponse, badRangeResponse, okRangeResponse, badGatewayResponse } from './utils/responses.js' import { selectOutputType, queryFormatToAcceptHeader } from './utils/select-output-type.js' import { walkPath } from './utils/walk-path.js' import type { CIDDetail, ContentTypeParser, Resource, VerifiedFetchInit as VerifiedFetchOptions } from './index.js' @@ -290,7 +290,7 @@ export class VerifiedFetch { } catch (err) { this.log.error('error walking path %s', path, err) - return internalServerErrorResponse('Error walking path') + return badGatewayResponse('Error walking path') } let resolvedCID = terminalElement?.cid ?? cid @@ -373,7 +373,7 @@ export class VerifiedFetch { if (byteRangeContext.isRangeRequest && err.code === 'ERR_INVALID_PARAMS') { return badRangeResponse(resource) } - return internalServerErrorResponse('Unable to stream content') + return badGatewayResponse('Unable to stream content') } } diff --git a/packages/verified-fetch/test/custom-dns-resolvers.spec.ts b/packages/verified-fetch/test/custom-dns-resolvers.spec.ts index 49a17d2f..300a8673 100644 --- a/packages/verified-fetch/test/custom-dns-resolvers.spec.ts +++ b/packages/verified-fetch/test/custom-dns-resolvers.spec.ts @@ -27,8 +27,8 @@ describe('custom dns-resolvers', () => { dnsResolvers: [customDnsResolver] }) const response = await fetch('ipns://some-non-cached-domain.com') - expect(response.status).to.equal(500) - expect(response.statusText).to.equal('Internal Server Error') + expect(response.status).to.equal(502) + expect(response.statusText).to.equal('Bad Gateway') expect(customDnsResolver.callCount).to.equal(1) expect(customDnsResolver.getCall(0).args).to.deep.equal(['some-non-cached-domain.com', { onProgress: undefined }]) @@ -46,8 +46,8 @@ describe('custom dns-resolvers', () => { }) const response = await verifiedFetch.fetch('ipns://some-non-cached-domain2.com') - expect(response.status).to.equal(500) - expect(response.statusText).to.equal('Internal Server Error') + expect(response.status).to.equal(502) + expect(response.statusText).to.equal('Bad Gateway') expect(customDnsResolver.callCount).to.equal(1) expect(customDnsResolver.getCall(0).args).to.deep.equal(['some-non-cached-domain2.com', { onProgress: undefined }]) From 121747ba5b4b301442317f358f157a34158a1ca1 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 14:45:02 -0800 Subject: [PATCH 18/39] chore: use libp2p/interface for types in src --- packages/verified-fetch/src/utils/byte-range-context.ts | 2 +- .../verified-fetch/src/utils/get-stream-from-async-iterable.ts | 2 +- packages/verified-fetch/src/utils/parse-resource.ts | 2 +- packages/verified-fetch/src/utils/parse-url-string.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 655c56d9..b4cc58c3 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -1,7 +1,7 @@ import { calculateByteRangeIndexes, getHeader } from './request-headers.js' import { getContentRangeHeader } from './response-headers.js' import type { SupportedBodyTypes } from '../types.js' -import type { ComponentLogger, Logger } from '@libp2p/logger' +import type { ComponentLogger, Logger } from '@libp2p/interface' /** * Gets the body size of a given body if it's possible to calculate it synchronously. diff --git a/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts b/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts index be5b0282..02342d59 100644 --- a/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts +++ b/packages/verified-fetch/src/utils/get-stream-from-async-iterable.ts @@ -1,6 +1,6 @@ import { CustomProgressEvent } from 'progress-events' import type { VerifiedFetchInit } from '../index.js' -import type { ComponentLogger } from '@libp2p/logger' +import type { ComponentLogger } from '@libp2p/interface' /** * Converts an async iterator of Uint8Array bytes to a stream and returns the first chunk of bytes diff --git a/packages/verified-fetch/src/utils/parse-resource.ts b/packages/verified-fetch/src/utils/parse-resource.ts index 23b3061a..a20e82d4 100644 --- a/packages/verified-fetch/src/utils/parse-resource.ts +++ b/packages/verified-fetch/src/utils/parse-resource.ts @@ -3,7 +3,7 @@ import { parseUrlString } from './parse-url-string.js' import type { ParsedUrlStringResults } from './parse-url-string.js' import type { Resource } from '../index.js' import type { IPNS, IPNSRoutingEvents, ResolveDnsLinkProgressEvents, ResolveProgressEvents } from '@helia/ipns' -import type { ComponentLogger } from '@libp2p/logger' +import type { ComponentLogger } from '@libp2p/interface' import type { ProgressOptions } from 'progress-events' export interface ParseResourceComponents { diff --git a/packages/verified-fetch/src/utils/parse-url-string.ts b/packages/verified-fetch/src/utils/parse-url-string.ts index 0f804c54..adda386c 100644 --- a/packages/verified-fetch/src/utils/parse-url-string.ts +++ b/packages/verified-fetch/src/utils/parse-url-string.ts @@ -3,7 +3,7 @@ import { CID } from 'multiformats/cid' import { TLRU } from './tlru.js' import type { RequestFormatShorthand } from '../types.js' import type { IPNS, IPNSRoutingEvents, ResolveDnsLinkProgressEvents, ResolveProgressEvents, ResolveResult } from '@helia/ipns' -import type { ComponentLogger } from '@libp2p/logger' +import type { ComponentLogger } from '@libp2p/interface' import type { ProgressOptions } from 'progress-events' const ipnsCache = new TLRU(1000) From 05a6dfb90631b527e078a4f9c61954c0a66e7905 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 14:51:41 -0800 Subject: [PATCH 19/39] chore: failing to create range resp logs error --- packages/verified-fetch/src/utils/responses.ts | 10 +++++----- packages/verified-fetch/src/verified-fetch.ts | 8 ++------ 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/verified-fetch/src/utils/responses.ts b/packages/verified-fetch/src/utils/responses.ts index a8ce2336..667318c6 100644 --- a/packages/verified-fetch/src/utils/responses.ts +++ b/packages/verified-fetch/src/utils/responses.ts @@ -1,5 +1,6 @@ import type { ByteRangeContext } from './byte-range-context' import type { SupportedBodyTypes } from '../types.js' +import type { Logger } from '@libp2p/interface' function setField (response: Response, name: string, value: string | boolean): void { Object.defineProperty(response, name, { @@ -116,9 +117,10 @@ export function movedPermanentlyResponse (url: string, location: string, init?: interface RangeOptions { byteRangeContext: ByteRangeContext + log?: Logger } -export function okRangeResponse (url: string, body: SupportedBodyTypes, { byteRangeContext }: RangeOptions, init?: ResponseOptions): Response { +export function okRangeResponse (url: string, body: SupportedBodyTypes, { byteRangeContext, log }: RangeOptions, init?: ResponseOptions): Response { if (!byteRangeContext.isRangeRequest) { return okResponse(url, body, init) } @@ -138,10 +140,8 @@ export function okRangeResponse (url: string, body: SupportedBodyTypes, { byteRa 'content-range': byteRangeContext.contentRangeHeaderValue } }) - } catch (e) { - // TODO: should we return a different status code here? - // eslint-disable-next-line no-console - console.error('Invalid range request', e) + } catch (e: any) { + log?.error('failed to create range response', e) return badRangeResponse(url, body, init) } diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index 782c4616..1301dbe5 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -357,7 +357,7 @@ export class VerifiedFetch { }) byteRangeContext.setBody(stream) // if not a valid range request, okRangeRequest will call okResponse - const response = okRangeResponse(resource, byteRangeContext.getBody(), { byteRangeContext }, { + const response = okRangeResponse(resource, byteRangeContext.getBody(), { byteRangeContext, log: this.log }, { redirected }) @@ -381,13 +381,9 @@ export class VerifiedFetch { const byteRangeContext = new ByteRangeContext(this.helia.logger, options?.headers) const result = await this.helia.blockstore.get(cid, options) byteRangeContext.setBody(result) - const response = okRangeResponse(resource, byteRangeContext.getBody(), { byteRangeContext }, { + const response = okRangeResponse(resource, byteRangeContext.getBody(), { byteRangeContext, log: this.log }, { redirected: false }) - // if (byteRangeContext.isRangeRequest) { - // } else { - // response = okResponse(resource, result) - // } // if the user has specified an `Accept` header that corresponds to a raw // type, honour that header, so for example they don't request From 9dcd79826925048a138f472fd5d607c6e2f99359 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 15:12:41 -0800 Subject: [PATCH 20/39] chore: Apply suggestions from code review --- .../src/utils/byte-range-context.ts | 19 +++++++++---------- .../src/utils/response-headers.ts | 10 +++++----- packages/verified-fetch/src/verified-fetch.ts | 3 ++- .../test/range-requests.spec.ts | 2 -- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index b4cc58c3..9ad38f84 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -48,9 +48,9 @@ export class ByteRangeContext { private _isValidRangeRequest: boolean | null = null private readonly requestRangeStart: number | null private readonly requestRangeEnd: number | null - byteStart: number | undefined - byteEnd: number | undefined - byteSize: number | undefined + private byteStart: number | undefined + private byteEnd: number | undefined + private byteSize: number | undefined constructor (logger: ComponentLogger, private readonly headers?: HeadersInit) { this.log = logger.forComponent('helia:verified-fetch:byte-range-context') @@ -80,7 +80,7 @@ export class ByteRangeContext { public setBody (body: SupportedBodyTypes): void { this._body = body - // if fileSize was set explicitly or already set, don't recalculate it + // if fileSize was already set, don't recalculate it this.fileSize = this.fileSize ?? getBodySizeSync(body) this.log.trace('set request body with fileSize %o', this._fileSize) @@ -111,7 +111,6 @@ export class ByteRangeContext { } else if (body instanceof ReadableStream) { // stream should already be spliced by dagPb/unixfs return body - // return splicingTransformStream(body, offset, length, this.logger) } } // offset and length are not set, so not a range request, return body untouched. @@ -146,7 +145,7 @@ export class ByteRangeContext { public set fileSize (size: number | bigint | null) { this._fileSize = size != null ? Number(size) : null this.log.trace('set _fileSize to %o', this._fileSize) - // if fileSize was set explicitly, we need to recalculate the offset details + // when fileSize changes, we need to recalculate the offset details this.setOffsetDetails() } @@ -220,11 +219,10 @@ export class ByteRangeContext { if (this.byteStart == null || this.byteStart === 0) { return 0 } - // if length is undefined, unixfs.cat and ArrayBuffer.slice will not use an inclusive offset, so we have to subtract by 1 + // if byteStart is undefined, unixfs.cat and ArrayBuffer.slice will not use an inclusive offset, so we have to subtract by 1 if (this.isPrefixLengthRequest || this.isSuffixLengthRequest) { return this.byteStart - 1 } - // this value will be passed to unixfs.cat return this.byteStart } @@ -235,8 +233,9 @@ export class ByteRangeContext { */ public get length (): number | undefined { // this value will be passed to unixfs.cat. - if (this.requestRangeEnd == null) { - return undefined // this is a suffix-length request and unixfs has a bug where it doesn't always respect the length parameter + if (this.isSuffixLengthRequest) { + // this is a suffix-length request and unixfs has a bug where it doesn't always respect the length parameter + return undefined } return this.byteSize ?? undefined } diff --git a/packages/verified-fetch/src/utils/response-headers.ts b/packages/verified-fetch/src/utils/response-headers.ts index 09dd9399..48e1103b 100644 --- a/packages/verified-fetch/src/utils/response-headers.ts +++ b/packages/verified-fetch/src/utils/response-headers.ts @@ -1,17 +1,17 @@ /** * This function returns the value of the `Content-Range` header for a given range. - * If you know the total size of the body, you should pass it in the `options` object. + * If you know the total size of the body, pass it as `byteSize` * * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range */ export function getContentRangeHeader ({ byteStart, byteEnd, byteSize }: { byteStart: number | undefined, byteEnd: number | undefined, byteSize: number | undefined }): string { const total = byteSize ?? '*' // if we don't know the total size, we should use * - if (byteStart == null && byteEnd == null) { - return `bytes */${total}` - } if (byteStart != null && byteEnd == null) { - return `bytes ${byteStart}-*/${total}` + if (byteSize == null) { + return `bytes */${total}` + } + return `bytes ${byteStart}-${byteSize}/${byteSize}` } if (byteStart == null && byteEnd != null) { if (byteSize == null) { diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index 1301dbe5..e4efa749 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -332,12 +332,13 @@ export class VerifiedFetch { } } + // we have to .stat before .cat so we can get the size and make sure byte offsets are calculated properly if (byteRangeContext.isRangeRequest && byteRangeContext.isValidRangeRequest) { stat = stat ?? await this.unixfs.stat(resolvedCID, { signal: options?.signal, onProgress: options?.onProgress }) - this.log.trace('stat for %c/%s: %o', resolvedCID, path, stat) + this.log.trace('stat.fileSize for rangeRequest %d', stat.fileSize) byteRangeContext.fileSize = stat.fileSize } const offset = byteRangeContext.offset diff --git a/packages/verified-fetch/test/range-requests.spec.ts b/packages/verified-fetch/test/range-requests.spec.ts index 5ac3f420..9df28b5b 100644 --- a/packages/verified-fetch/test/range-requests.spec.ts +++ b/packages/verified-fetch/test/range-requests.spec.ts @@ -29,7 +29,6 @@ describe('range requests', () => { interface SuccessfulTestExpectation { contentRange: string - // byteSize?: number bytes: Uint8Array } async function testRange (cid: CID, headerRange: string, expected: SuccessfulTestExpectation): Promise { @@ -49,7 +48,6 @@ describe('range requests', () => { const responseContent = await response.arrayBuffer() expect(new Uint8Array(responseContent)).to.deep.equal(expected.bytes) - // expect(responseContent.byteLength).to.equal(expected.byteSize) // the length of the data should match the requested range } async function assertFailingRange (response: Promise): Promise { From f296f0ba4ba0ea5d78155c5c6b18b443136f67f4 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 15:35:14 -0800 Subject: [PATCH 21/39] chore: fix broken tests from github PR patches (my own) --- .../src/utils/byte-range-context.ts | 16 +++++++--------- .../verified-fetch/src/utils/response-headers.ts | 8 ++++++++ .../verified-fetch/test/range-requests.spec.ts | 1 + .../test/utils/response-headers.spec.ts | 6 +++++- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 9ad38f84..0d7b4188 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -216,14 +216,17 @@ export class ByteRangeContext { * 2. slicing the body */ public get offset (): number { - if (this.byteStart == null || this.byteStart === 0) { + if (this.byteStart === 0) { return 0 } - // if byteStart is undefined, unixfs.cat and ArrayBuffer.slice will not use an inclusive offset, so we have to subtract by 1 if (this.isPrefixLengthRequest || this.isSuffixLengthRequest) { - return this.byteStart - 1 + if (this.byteStart != null) { + // we have to subtract by 1 because the offset is inclusive + return this.byteStart - 1 + } } - return this.byteStart + + return this.byteStart ?? 0 } /** @@ -232,11 +235,6 @@ export class ByteRangeContext { * 2. slicing the body */ public get length (): number | undefined { - // this value will be passed to unixfs.cat. - if (this.isSuffixLengthRequest) { - // this is a suffix-length request and unixfs has a bug where it doesn't always respect the length parameter - return undefined - } return this.byteSize ?? undefined } diff --git a/packages/verified-fetch/src/utils/response-headers.ts b/packages/verified-fetch/src/utils/response-headers.ts index 48e1103b..9da29a50 100644 --- a/packages/verified-fetch/src/utils/response-headers.ts +++ b/packages/verified-fetch/src/utils/response-headers.ts @@ -8,17 +8,25 @@ export function getContentRangeHeader ({ byteStart, byteEnd, byteSize }: { byteS const total = byteSize ?? '*' // if we don't know the total size, we should use * if (byteStart != null && byteEnd == null) { + // only byteStart in range if (byteSize == null) { return `bytes */${total}` } return `bytes ${byteStart}-${byteSize}/${byteSize}` } + if (byteStart == null && byteEnd != null) { + // only byteEnd in range if (byteSize == null) { return `bytes */${total}` } return `bytes ${byteSize - byteEnd + 1}-${byteSize}/${byteSize}` } + if (byteStart == null && byteEnd == null) { + // neither are provided, we can't return a valid range. + return `bytes */${total}` + } + return `bytes ${byteStart}-${byteEnd}/${total}` } diff --git a/packages/verified-fetch/test/range-requests.spec.ts b/packages/verified-fetch/test/range-requests.spec.ts index 9df28b5b..489ce3c8 100644 --- a/packages/verified-fetch/test/range-requests.spec.ts +++ b/packages/verified-fetch/test/range-requests.spec.ts @@ -82,6 +82,7 @@ describe('range requests', () => { } ] validTestCases.forEach(({ bytes, contentRange, rangeHeader }) => { + // if these fail, check response-headers.spec.ts first it(`should return correct 206 Partial Content response for ${rangeHeader}`, async () => { const expected: SuccessfulTestExpectation = { bytes, diff --git a/packages/verified-fetch/test/utils/response-headers.spec.ts b/packages/verified-fetch/test/utils/response-headers.spec.ts index d4ed4f5a..6197450a 100644 --- a/packages/verified-fetch/test/utils/response-headers.spec.ts +++ b/packages/verified-fetch/test/utils/response-headers.spec.ts @@ -14,8 +14,12 @@ describe('response-headers', () => { expect(getContentRangeHeader({ byteStart: undefined, byteEnd: 9, byteSize: 11 })).to.equal('bytes 3-11/11') }) + it('should return correct content range header when only byteStart and byteSize are provided', () => { + expect(getContentRangeHeader({ byteStart: 5, byteEnd: undefined, byteSize: 11 })).to.equal('bytes 5-11/11') + }) + it('should return correct content range header when only byteStart is provided', () => { - expect(getContentRangeHeader({ byteStart: 500, byteEnd: undefined, byteSize: undefined })).to.equal('bytes 500-*/*') + expect(getContentRangeHeader({ byteStart: 500, byteEnd: undefined, byteSize: undefined })).to.equal('bytes */*') }) it('should return correct content range header when only byteEnd is provided', () => { From 912ee47c2bbca273bf7c7f63ff94b8b84da2edfa Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 15:47:18 -0800 Subject: [PATCH 22/39] chore: re-enable stream tests for ByteRangeContext --- .../test/utils/byte-range-context.spec.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/verified-fetch/test/utils/byte-range-context.spec.ts b/packages/verified-fetch/test/utils/byte-range-context.spec.ts index d96328fc..e70fa792 100644 --- a/packages/verified-fetch/test/utils/byte-range-context.spec.ts +++ b/packages/verified-fetch/test/utils/byte-range-context.spec.ts @@ -2,8 +2,6 @@ import { unixfs, type UnixFS } from '@helia/unixfs' import { stop } from '@libp2p/interface' import { defaultLogger, prefixLogger } from '@libp2p/logger' import { expect } from 'aegir/chai' -import toIt from 'browser-readablestream-to-it' -import all from 'it-all' import { ByteRangeContext } from '../../src/utils/byte-range-context.js' import { getStreamFromAsyncIterable } from '../../src/utils/get-stream-from-async-iterable.js' import { createHelia } from '../fixtures/create-offline-helia.js' @@ -91,13 +89,15 @@ describe('ByteRangeContext', () => { validRanges.forEach(({ type, range, expected, body, contentRange }) => { it(`should correctly slice ${type} body with range ${range}`, () => { const context = new ByteRangeContext(logger, { Range: range }) + context.setBody(body) + expect(context.getBody()).to.deep.equal(expected) expect(context.contentRangeHeaderValue).to.equal(contentRange) }) }) - describe.skip('handling ReadableStreams', () => { + describe('handling ReadableStreams', () => { let helia: Helia let fs: UnixFS let cid: CID @@ -129,8 +129,9 @@ describe('ByteRangeContext', () => { context.fileSize = stat.fileSize context.setBody(await getBodyStream(context.offset, context.length)) - const bodyResult = await all(toIt(context.getBody() as ReadableStream)) - expect(bodyResult).to.deep.equal(expected) + const response = new Response(context.getBody()) + const bodyResult = await response.arrayBuffer() + expect(new Uint8Array(bodyResult)).to.deep.equal(expected) expect(context.contentRangeHeaderValue).to.equal(contentRange) }) }) From b0b6a4a5f778a857e952c986a16e6cc7ecaa0992 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 16:06:06 -0800 Subject: [PATCH 23/39] chore: clean up getBody a bit --- .../verified-fetch/src/utils/byte-range-context.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 0d7b4188..eb80a716 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -93,7 +93,7 @@ export class ByteRangeContext { return body } if (!this.isRangeRequest || !this.isValidRangeRequest) { - this.log.trace('returning body unmodified') + this.log.trace('returning body unmodified for non-range, or invalid range, request') return body } const byteStart = this.byteStart @@ -101,19 +101,17 @@ export class ByteRangeContext { const byteSize = this.byteSize if (byteStart != null || byteEnd != null) { this.log.trace('returning body with byteStart %o byteEnd %o byteSize', byteStart, byteEnd, byteSize) - if (body instanceof Uint8Array) { + if (body instanceof ArrayBuffer || body instanceof Blob || body instanceof Uint8Array) { this.log.trace('body is Uint8Array') return this.getSlicedBody(body) - } else if (body instanceof ArrayBuffer) { - return this.getSlicedBody(body) - } else if (body instanceof Blob) { - return this.getSlicedBody(body) } else if (body instanceof ReadableStream) { // stream should already be spliced by dagPb/unixfs return body } } - // offset and length are not set, so not a range request, return body untouched. + + // we should not reach this point, but return body untouched. + this.log.error('returning unmofified body for valid range request') return body } From f399bedc8690380002efe154e113f62047e09424 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 7 Mar 2024 16:13:03 -0800 Subject: [PATCH 24/39] chore: ByteRangeContext getBody cleanup --- .../verified-fetch/src/utils/byte-range-context.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index eb80a716..49472da5 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -100,14 +100,12 @@ export class ByteRangeContext { const byteEnd = this.byteEnd const byteSize = this.byteSize if (byteStart != null || byteEnd != null) { - this.log.trace('returning body with byteStart %o byteEnd %o byteSize', byteStart, byteEnd, byteSize) - if (body instanceof ArrayBuffer || body instanceof Blob || body instanceof Uint8Array) { - this.log.trace('body is Uint8Array') - return this.getSlicedBody(body) - } else if (body instanceof ReadableStream) { - // stream should already be spliced by dagPb/unixfs + this.log.trace('returning body with byteStart=%o, byteEnd=%o, byteSize=%o', byteStart, byteEnd, byteSize) + if (body instanceof ReadableStream) { + // stream should already be spliced by `unixfs.cat` return body } + return this.getSlicedBody(body) } // we should not reach this point, but return body untouched. @@ -115,7 +113,7 @@ export class ByteRangeContext { return body } - private getSlicedBody (body: T): T { + private getSlicedBody (body: T): T { if (this.isPrefixLengthRequest) { this.log.trace('sliced body with byteStart %o', this.byteStart) return body.slice(this.offset) as T From eb0224bff6d80c61c5310a1e3edd81ec2d6ae3ed Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 14 Mar 2024 23:07:50 -0700 Subject: [PATCH 25/39] chore: apply suggestions from code review Co-authored-by: Alex Potsides --- .../verified-fetch/src/utils/byte-range-context.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 49472da5..40371833 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -109,7 +109,7 @@ export class ByteRangeContext { } // we should not reach this point, but return body untouched. - this.log.error('returning unmofified body for valid range request') + this.log.error('returning unmodified body for valid range request') return body } @@ -264,11 +264,13 @@ export class ByteRangeContext { this.byteSize = byteSize } - // This function returns the value of the "content-range" header. - // Content-Range: -/ - // Content-Range: -/* - // Content-Range: */ - // @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range + /** + * This function returns the value of the "content-range" header. + * Content-Range: -/ + * Content-Range: -/* + * Content-Range: */ + * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range + */ public get contentRangeHeaderValue (): string { if (this._contentRangeHeaderValue != null) { return this._contentRangeHeaderValue From d1e6a82103a24c86dff1c9ac74b360572305f1ef Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 14 Mar 2024 23:41:25 -0700 Subject: [PATCH 26/39] fix: getSlicedBody uses correct types --- .../verified-fetch/src/utils/byte-range-context.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 40371833..9f34345b 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -3,6 +3,8 @@ import { getContentRangeHeader } from './response-headers.js' import type { SupportedBodyTypes } from '../types.js' import type { ComponentLogger, Logger } from '@libp2p/interface' +type SliceableBody = Exclude | null> + /** * Gets the body size of a given body if it's possible to calculate it synchronously. */ @@ -113,20 +115,20 @@ export class ByteRangeContext { return body } - private getSlicedBody (body: T): T { + private getSlicedBody (body: T): SliceableBody { if (this.isPrefixLengthRequest) { this.log.trace('sliced body with byteStart %o', this.byteStart) - return body.slice(this.offset) as T + return body.slice(this.offset) satisfies SliceableBody } if (this.isSuffixLengthRequest && this.length != null) { this.log.trace('sliced body with length %o', -this.length) - return body.slice(-this.length) as T + return body.slice(-this.length) satisfies SliceableBody } const offset = this.byteStart ?? 0 const length = this.byteEnd == null ? undefined : this.byteEnd + 1 this.log.trace('returning body with offset %o and length %o', offset, length) - return body.slice(offset, length) as T + return body.slice(offset, length) satisfies SliceableBody } private get isSuffixLengthRequest (): boolean { From 07ab941a9a120e31f9f396da0fafe0062c37d3e0 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 14 Mar 2024 23:45:33 -0700 Subject: [PATCH 27/39] chore: remove extra stat call --- packages/verified-fetch/src/verified-fetch.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index e4efa749..c6161dd1 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -333,13 +333,10 @@ export class VerifiedFetch { } // we have to .stat before .cat so we can get the size and make sure byte offsets are calculated properly - if (byteRangeContext.isRangeRequest && byteRangeContext.isValidRangeRequest) { - stat = stat ?? await this.unixfs.stat(resolvedCID, { - signal: options?.signal, - onProgress: options?.onProgress - }) - this.log.trace('stat.fileSize for rangeRequest %d', stat.fileSize) - byteRangeContext.fileSize = stat.fileSize + if (byteRangeContext.isRangeRequest && byteRangeContext.isValidRangeRequest && terminalElement.type === 'file') { + byteRangeContext.fileSize = terminalElement.unixfs.fileSize() + + this.log.trace('fileSize for rangeRequest %d', byteRangeContext.fileSize) } const offset = byteRangeContext.offset const length = byteRangeContext.length From ac621a24f252f4af990f7a06da804844987a58f0 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 14 Mar 2024 23:56:56 -0700 Subject: [PATCH 28/39] chore: fix jsdoc with '*/' --- .../verified-fetch/src/utils/byte-range-context.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 9f34345b..1979ad12 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -267,12 +267,17 @@ export class ByteRangeContext { } /** - * This function returns the value of the "content-range" header. - * Content-Range: -/ - * Content-Range: -/* - * Content-Range: */ + * This function returns the values of the "content-range" header. + * * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range + * + * Returns data to support the following content ranges: + * + * @example + * - Content-Range: -/ + * - Content-Range: -/‍* */ + // - Content-Range: */ // this is purposefully not in jsdoc block public get contentRangeHeaderValue (): string { if (this._contentRangeHeaderValue != null) { return this._contentRangeHeaderValue From 46dc13383a8ab471e1cf3cfd624eceaf9044352c Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 15 Mar 2024 01:01:24 -0700 Subject: [PATCH 29/39] chore: fileSize is public property, but should not be used --- .../src/utils/byte-range-context.ts | 37 ++++++++++++------- packages/verified-fetch/src/verified-fetch.ts | 2 +- .../test/utils/byte-range-context.spec.ts | 2 +- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 1979ad12..fe8f4a23 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -42,7 +42,13 @@ function getByteRangeFromHeader (rangeHeader: string): { start: string, end: str export class ByteRangeContext { private readonly _isRangeRequest: boolean - private _fileSize: number | null | undefined + + /** + * This property should only be set by calling `setFileSize` or `setBody`. + * + * @access private + */ + public fileSize: Readonly private readonly _contentRangeHeaderValue: string | undefined private _body: SupportedBodyTypes | null = null private readonly _rangeRequestHeader: string | undefined @@ -80,12 +86,15 @@ export class ByteRangeContext { } } + /** + * When you get a body, it should be set here, and we will calculate the fileSize if possible. + */ public setBody (body: SupportedBodyTypes): void { this._body = body // if fileSize was already set, don't recalculate it - this.fileSize = this.fileSize ?? getBodySizeSync(body) + this.setFileSize(this.fileSize ?? getBodySizeSync(body)) - this.log.trace('set request body with fileSize %o', this._fileSize) + this.log.trace('set request body with fileSize %o', this.fileSize) } public getBody (): SupportedBodyTypes { @@ -139,18 +148,20 @@ export class ByteRangeContext { return this.requestRangeStart != null && this.requestRangeEnd == null } - // sometimes, we need to set the fileSize explicitly because we can't calculate the size of the body (e.g. for unixfs content where we call .stat) - public set fileSize (size: number | bigint | null) { - this._fileSize = size != null ? Number(size) : null - this.log.trace('set _fileSize to %o', this._fileSize) + /** + * Sometimes, we need to set the fileSize explicitly because we can't calculate + * the size of the body (e.g. for unixfs content where we call .stat). + * + * This fileSize should otherwise only be called from `setBody`, and `.fileSize` + * should not be set directly. + */ + public setFileSize (size: number | bigint | null): void { + this.fileSize = size != null ? Number(size) : null + this.log.trace('set _fileSize to %o', this.fileSize) // when fileSize changes, we need to recalculate the offset details this.setOffsetDetails() } - public get fileSize (): number | null | undefined { - return this._fileSize - } - public get isRangeRequest (): boolean { return this._isRangeRequest } @@ -259,7 +270,7 @@ export class ByteRangeContext { return } - const { start, end, byteSize } = calculateByteRangeIndexes(this.requestRangeStart ?? undefined, this.requestRangeEnd ?? undefined, this._fileSize ?? undefined) + const { start, end, byteSize } = calculateByteRangeIndexes(this.requestRangeStart ?? undefined, this.requestRangeEnd ?? undefined, this.fileSize ?? undefined) this.log.trace('set byteStart to %o, byteEnd to %o, byteSize to %o', start, end, byteSize) this.byteStart = start this.byteEnd = end @@ -290,7 +301,7 @@ export class ByteRangeContext { return getContentRangeHeader({ byteStart: this.byteStart, byteEnd: this.byteEnd, - byteSize: this._fileSize ?? undefined + byteSize: this.fileSize ?? undefined }) } } diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index c6161dd1..4c75180c 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -334,7 +334,7 @@ export class VerifiedFetch { // we have to .stat before .cat so we can get the size and make sure byte offsets are calculated properly if (byteRangeContext.isRangeRequest && byteRangeContext.isValidRangeRequest && terminalElement.type === 'file') { - byteRangeContext.fileSize = terminalElement.unixfs.fileSize() + byteRangeContext.setFileSize(terminalElement.unixfs.fileSize()) this.log.trace('fileSize for rangeRequest %d', byteRangeContext.fileSize) } diff --git a/packages/verified-fetch/test/utils/byte-range-context.spec.ts b/packages/verified-fetch/test/utils/byte-range-context.spec.ts index e70fa792..abf7ef99 100644 --- a/packages/verified-fetch/test/utils/byte-range-context.spec.ts +++ b/packages/verified-fetch/test/utils/byte-range-context.spec.ts @@ -126,7 +126,7 @@ describe('ByteRangeContext', () => { leafType: 'file' }) const stat = await fs.stat(cid) - context.fileSize = stat.fileSize + context.setFileSize(stat.fileSize) context.setBody(await getBodyStream(context.offset, context.length)) const response = new Response(context.getBody()) From 36f6c968ea05848a3d1853500c6806317253fb0e Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 15 Mar 2024 01:01:43 -0700 Subject: [PATCH 30/39] test: fix blob comparisons that broke or were never worjing properly --- .../test/utils/byte-range-context.spec.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/verified-fetch/test/utils/byte-range-context.spec.ts b/packages/verified-fetch/test/utils/byte-range-context.spec.ts index abf7ef99..84f18bbb 100644 --- a/packages/verified-fetch/test/utils/byte-range-context.spec.ts +++ b/packages/verified-fetch/test/utils/byte-range-context.spec.ts @@ -87,12 +87,23 @@ describe('ByteRangeContext', () => { })) ] validRanges.forEach(({ type, range, expected, body, contentRange }) => { - it(`should correctly slice ${type} body with range ${range}`, () => { + it(`should correctly slice ${type} body with range ${range}`, async () => { const context = new ByteRangeContext(logger, { Range: range }) context.setBody(body) + const actualBody = context.getBody() + + if (actualBody instanceof Blob || type === 'Blob') { + const bodyAsUint8Array = new Uint8Array(await (actualBody as Blob).arrayBuffer()) + const expectedAsUint8Array = new Uint8Array(await (expected as Blob).arrayBuffer()) + // loop through the bytes and compare them + for (let i = 0; i < bodyAsUint8Array.length; i++) { + expect(bodyAsUint8Array[i]).to.equal(expectedAsUint8Array[i]) + } + } else { + expect(actualBody).to.deep.equal(expected) + } - expect(context.getBody()).to.deep.equal(expected) expect(context.contentRangeHeaderValue).to.equal(contentRange) }) }) From 5fc7ceb8a3354d924389f3f9b6ed949f67b62c5f Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 15 Mar 2024 09:59:04 -0700 Subject: [PATCH 31/39] chore: Update byte-range-context.ts Co-authored-by: Alex Potsides --- packages/verified-fetch/src/utils/byte-range-context.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index fe8f4a23..f151bf07 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -50,7 +50,7 @@ export class ByteRangeContext { */ public fileSize: Readonly private readonly _contentRangeHeaderValue: string | undefined - private _body: SupportedBodyTypes | null = null + private _body: SupportedBodyTypes = null private readonly _rangeRequestHeader: string | undefined private readonly log: Logger private _isValidRangeRequest: boolean | null = null From 19c2713247f4b9a350e07b9f0f545df18fc09357 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 15 Mar 2024 11:25:18 -0700 Subject: [PATCH 32/39] chore: jsdoc cleanup --- packages/verified-fetch/src/utils/byte-range-context.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index f151bf07..39d2f4e4 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -286,7 +286,7 @@ export class ByteRangeContext { * * @example * - Content-Range: -/ - * - Content-Range: -/‍* + * - Content-Range: -/* */ // - Content-Range: */ // this is purposefully not in jsdoc block public get contentRangeHeaderValue (): string { From a1686a35a1b79f6308219adf19d3aaaa082778e5 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 15 Mar 2024 11:25:29 -0700 Subject: [PATCH 33/39] Revert "chore: fileSize is public property, but should not be used" This reverts commit 46dc13383a8ab471e1cf3cfd624eceaf9044352c. --- .../src/utils/byte-range-context.ts | 37 +++++++------------ packages/verified-fetch/src/verified-fetch.ts | 2 +- .../test/utils/byte-range-context.spec.ts | 2 +- 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 39d2f4e4..97e11b55 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -42,13 +42,7 @@ function getByteRangeFromHeader (rangeHeader: string): { start: string, end: str export class ByteRangeContext { private readonly _isRangeRequest: boolean - - /** - * This property should only be set by calling `setFileSize` or `setBody`. - * - * @access private - */ - public fileSize: Readonly + private _fileSize: number | null | undefined private readonly _contentRangeHeaderValue: string | undefined private _body: SupportedBodyTypes = null private readonly _rangeRequestHeader: string | undefined @@ -86,15 +80,12 @@ export class ByteRangeContext { } } - /** - * When you get a body, it should be set here, and we will calculate the fileSize if possible. - */ public setBody (body: SupportedBodyTypes): void { this._body = body // if fileSize was already set, don't recalculate it - this.setFileSize(this.fileSize ?? getBodySizeSync(body)) + this.fileSize = this.fileSize ?? getBodySizeSync(body) - this.log.trace('set request body with fileSize %o', this.fileSize) + this.log.trace('set request body with fileSize %o', this._fileSize) } public getBody (): SupportedBodyTypes { @@ -148,20 +139,18 @@ export class ByteRangeContext { return this.requestRangeStart != null && this.requestRangeEnd == null } - /** - * Sometimes, we need to set the fileSize explicitly because we can't calculate - * the size of the body (e.g. for unixfs content where we call .stat). - * - * This fileSize should otherwise only be called from `setBody`, and `.fileSize` - * should not be set directly. - */ - public setFileSize (size: number | bigint | null): void { - this.fileSize = size != null ? Number(size) : null - this.log.trace('set _fileSize to %o', this.fileSize) + // sometimes, we need to set the fileSize explicitly because we can't calculate the size of the body (e.g. for unixfs content where we call .stat) + public set fileSize (size: number | bigint | null) { + this._fileSize = size != null ? Number(size) : null + this.log.trace('set _fileSize to %o', this._fileSize) // when fileSize changes, we need to recalculate the offset details this.setOffsetDetails() } + public get fileSize (): number | null | undefined { + return this._fileSize + } + public get isRangeRequest (): boolean { return this._isRangeRequest } @@ -270,7 +259,7 @@ export class ByteRangeContext { return } - const { start, end, byteSize } = calculateByteRangeIndexes(this.requestRangeStart ?? undefined, this.requestRangeEnd ?? undefined, this.fileSize ?? undefined) + const { start, end, byteSize } = calculateByteRangeIndexes(this.requestRangeStart ?? undefined, this.requestRangeEnd ?? undefined, this._fileSize ?? undefined) this.log.trace('set byteStart to %o, byteEnd to %o, byteSize to %o', start, end, byteSize) this.byteStart = start this.byteEnd = end @@ -301,7 +290,7 @@ export class ByteRangeContext { return getContentRangeHeader({ byteStart: this.byteStart, byteEnd: this.byteEnd, - byteSize: this.fileSize ?? undefined + byteSize: this._fileSize ?? undefined }) } } diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index d467e54e..833baa33 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -330,7 +330,7 @@ export class VerifiedFetch { // we have to .stat before .cat so we can get the size and make sure byte offsets are calculated properly if (byteRangeContext.isRangeRequest && byteRangeContext.isValidRangeRequest && terminalElement.type === 'file') { - byteRangeContext.setFileSize(terminalElement.unixfs.fileSize()) + byteRangeContext.fileSize = terminalElement.unixfs.fileSize() this.log.trace('fileSize for rangeRequest %d', byteRangeContext.fileSize) } diff --git a/packages/verified-fetch/test/utils/byte-range-context.spec.ts b/packages/verified-fetch/test/utils/byte-range-context.spec.ts index 84f18bbb..0dc8c8f5 100644 --- a/packages/verified-fetch/test/utils/byte-range-context.spec.ts +++ b/packages/verified-fetch/test/utils/byte-range-context.spec.ts @@ -137,7 +137,7 @@ describe('ByteRangeContext', () => { leafType: 'file' }) const stat = await fs.stat(cid) - context.setFileSize(stat.fileSize) + context.fileSize = stat.fileSize context.setBody(await getBodyStream(context.offset, context.length)) const response = new Response(context.getBody()) From e7e3fd01f2168535e1dfc5925517577d0d4265be Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 15 Mar 2024 11:27:21 -0700 Subject: [PATCH 34/39] chore: jsdoc comments explaining .fileSize use --- .../verified-fetch/src/utils/byte-range-context.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 97e11b55..d76809a5 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -42,6 +42,10 @@ function getByteRangeFromHeader (rangeHeader: string): { start: string, end: str export class ByteRangeContext { private readonly _isRangeRequest: boolean + + /** + * This property is purposefully only set in `set fileSize` and should not be set directly. + */ private _fileSize: number | null | undefined private readonly _contentRangeHeaderValue: string | undefined private _body: SupportedBodyTypes = null @@ -139,7 +143,13 @@ export class ByteRangeContext { return this.requestRangeStart != null && this.requestRangeEnd == null } - // sometimes, we need to set the fileSize explicitly because we can't calculate the size of the body (e.g. for unixfs content where we call .stat) + /** + * Sometimes, we need to set the fileSize explicitly because we can't calculate + * the size of the body (e.g. for unixfs content where we call .stat). + * + * This fileSize should otherwise only be called from `setBody`, and `.fileSize` + * should not be set directly. + */ public set fileSize (size: number | bigint | null) { this._fileSize = size != null ? Number(size) : null this.log.trace('set _fileSize to %o', this._fileSize) From c184e2a236f87b96811d597ee39fb044b0d71849 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 15 Mar 2024 11:55:03 -0700 Subject: [PATCH 35/39] chore: isRangeRequest is public --- .../src/utils/byte-range-context.ts | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index d76809a5..cd2a455d 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -41,7 +41,7 @@ function getByteRangeFromHeader (rangeHeader: string): { start: string, end: str } export class ByteRangeContext { - private readonly _isRangeRequest: boolean + private readonly isRangeRequest: boolean /** * This property is purposefully only set in `set fileSize` and should not be set directly. @@ -49,7 +49,7 @@ export class ByteRangeContext { private _fileSize: number | null | undefined private readonly _contentRangeHeaderValue: string | undefined private _body: SupportedBodyTypes = null - private readonly _rangeRequestHeader: string | undefined + private readonly rangeRequestHeader: string | undefined private readonly log: Logger private _isValidRangeRequest: boolean | null = null private readonly requestRangeStart: number | null @@ -60,25 +60,26 @@ export class ByteRangeContext { constructor (logger: ComponentLogger, private readonly headers?: HeadersInit) { this.log = logger.forComponent('helia:verified-fetch:byte-range-context') - this._rangeRequestHeader = getHeader(this.headers, 'Range') - if (this._rangeRequestHeader != null) { + this.rangeRequestHeader = getHeader(this.headers, 'Range') + if (this.rangeRequestHeader != null) { this.log.trace('range request detected') - this._isRangeRequest = true try { - const { start, end } = getByteRangeFromHeader(this._rangeRequestHeader) + const { start, end } = getByteRangeFromHeader(this.rangeRequestHeader) this.requestRangeStart = start != null ? parseInt(start) : null this.requestRangeEnd = end != null ? parseInt(end) : null + this.isRangeRequest = true } catch (e) { this.log.error('error parsing range request header: %o', e) this.isValidRangeRequest = false this.requestRangeStart = null this.requestRangeEnd = null + this.isRangeRequest = false } this.setOffsetDetails() } else { this.log.trace('no range request detected') - this._isRangeRequest = false + this.isRangeRequest = false this.requestRangeStart = null this.requestRangeEnd = null } @@ -161,10 +162,6 @@ export class ByteRangeContext { return this._fileSize } - public get isRangeRequest (): boolean { - return this._isRangeRequest - } - private isValidByteStart (): boolean { if (this.byteStart != null) { if (this.byteStart < 0) { From d6334569dcf33ac63098ee8890ad3caab1114d3b Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 15 Mar 2024 12:17:40 -0700 Subject: [PATCH 36/39] chore: getters/setters update --- .../src/utils/byte-range-context.ts | 45 ++++++++++--------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index cd2a455d..5d1f9d66 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -41,7 +41,7 @@ function getByteRangeFromHeader (rangeHeader: string): { start: string, end: str } export class ByteRangeContext { - private readonly isRangeRequest: boolean + public readonly isRangeRequest: boolean /** * This property is purposefully only set in `set fileSize` and should not be set directly. @@ -51,7 +51,6 @@ export class ByteRangeContext { private _body: SupportedBodyTypes = null private readonly rangeRequestHeader: string | undefined private readonly log: Logger - private _isValidRangeRequest: boolean | null = null private readonly requestRangeStart: number | null private readonly requestRangeEnd: number | null private byteStart: number | undefined @@ -62,18 +61,16 @@ export class ByteRangeContext { this.log = logger.forComponent('helia:verified-fetch:byte-range-context') this.rangeRequestHeader = getHeader(this.headers, 'Range') if (this.rangeRequestHeader != null) { + this.isRangeRequest = true this.log.trace('range request detected') try { const { start, end } = getByteRangeFromHeader(this.rangeRequestHeader) this.requestRangeStart = start != null ? parseInt(start) : null this.requestRangeEnd = end != null ? parseInt(end) : null - this.isRangeRequest = true } catch (e) { this.log.error('error parsing range request header: %o', e) - this.isValidRangeRequest = false this.requestRangeStart = null this.requestRangeEnd = null - this.isRangeRequest = false } this.setOffsetDetails() @@ -186,33 +183,41 @@ export class ByteRangeContext { return true } - public set isValidRangeRequest (val: boolean) { - this._isValidRangeRequest = val - } - + /** + * We may get the values required to determine if this is a valid range request at different times + * so we need to calculate it when asked. + */ public get isValidRangeRequest (): boolean { + if (!this.isRangeRequest) { + return false + } + if (this.requestRangeStart == null && this.requestRangeEnd == null) { + this.log.trace('invalid range request, range request values not provided') + return false + } if (!this.isValidByteStart()) { this.log.trace('invalid range request, byteStart is less than 0 or greater than fileSize') - this._isValidRangeRequest = false - } else if (!this.isValidByteEnd()) { + return false + } + if (!this.isValidByteEnd()) { this.log.trace('invalid range request, byteEnd is less than 0 or greater than fileSize') - this._isValidRangeRequest = false - } else if (this.requestRangeEnd != null && this.requestRangeStart != null) { + return false + } + if (this.requestRangeEnd != null && this.requestRangeStart != null) { // we may not have enough info.. base check on requested bytes if (this.requestRangeStart > this.requestRangeEnd) { this.log.trace('invalid range request, start is greater than end') - this._isValidRangeRequest = false + return false } else if (this.requestRangeStart < 0) { this.log.trace('invalid range request, start is less than 0') - this._isValidRangeRequest = false + return false } else if (this.requestRangeEnd < 0) { this.log.trace('invalid range request, end is less than 0') - this._isValidRangeRequest = false + return false } } - this._isValidRangeRequest = this._isValidRangeRequest ?? true - return this._isValidRangeRequest + return true } /** @@ -274,11 +279,11 @@ export class ByteRangeContext { } /** - * This function returns the values of the "content-range" header. + * This function returns the value of the "content-range" header. * * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range * - * Returns data to support the following content ranges: + * Returns a string representing the following content ranges: * * @example * - Content-Range: -/ From 314adca7aae4bceddf0f4db9f75bacae0933874b Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 15 Mar 2024 12:23:39 -0700 Subject: [PATCH 37/39] chore: remove unnecessary _contentRangeHeaderValue --- packages/verified-fetch/src/utils/byte-range-context.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index 5d1f9d66..c1bc6b2f 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -47,7 +47,6 @@ export class ByteRangeContext { * This property is purposefully only set in `set fileSize` and should not be set directly. */ private _fileSize: number | null | undefined - private readonly _contentRangeHeaderValue: string | undefined private _body: SupportedBodyTypes = null private readonly rangeRequestHeader: string | undefined private readonly log: Logger @@ -291,9 +290,6 @@ export class ByteRangeContext { */ // - Content-Range: */ // this is purposefully not in jsdoc block public get contentRangeHeaderValue (): string { - if (this._contentRangeHeaderValue != null) { - return this._contentRangeHeaderValue - } if (!this.isValidRangeRequest) { this.log.error('cannot get contentRangeHeaderValue for invalid range request') throw new Error('Invalid range request') From 8837738b068247dbb3fc01e7f45e72246a63d7c7 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 15 Mar 2024 12:29:11 -0700 Subject: [PATCH 38/39] chore: ByteRangeContext uses setFileSize and getFileSize --- .../verified-fetch/src/utils/byte-range-context.ts | 13 ++++++------- packages/verified-fetch/src/verified-fetch.ts | 4 ++-- .../test/utils/byte-range-context.spec.ts | 4 ++-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/verified-fetch/src/utils/byte-range-context.ts b/packages/verified-fetch/src/utils/byte-range-context.ts index c1bc6b2f..54df115e 100644 --- a/packages/verified-fetch/src/utils/byte-range-context.ts +++ b/packages/verified-fetch/src/utils/byte-range-context.ts @@ -84,7 +84,7 @@ export class ByteRangeContext { public setBody (body: SupportedBodyTypes): void { this._body = body // if fileSize was already set, don't recalculate it - this.fileSize = this.fileSize ?? getBodySizeSync(body) + this.setFileSize(this._fileSize ?? getBodySizeSync(body)) this.log.trace('set request body with fileSize %o', this._fileSize) } @@ -144,17 +144,16 @@ export class ByteRangeContext { * Sometimes, we need to set the fileSize explicitly because we can't calculate * the size of the body (e.g. for unixfs content where we call .stat). * - * This fileSize should otherwise only be called from `setBody`, and `.fileSize` - * should not be set directly. + * This fileSize should otherwise only be called from `setBody`. */ - public set fileSize (size: number | bigint | null) { + public setFileSize (size: number | bigint | null): void { this._fileSize = size != null ? Number(size) : null this.log.trace('set _fileSize to %o', this._fileSize) // when fileSize changes, we need to recalculate the offset details this.setOffsetDetails() } - public get fileSize (): number | null | undefined { + public getFileSize (): number | null | undefined { return this._fileSize } @@ -163,7 +162,7 @@ export class ByteRangeContext { if (this.byteStart < 0) { return false } - if (this.fileSize != null && this.byteStart > this.fileSize) { + if (this._fileSize != null && this.byteStart > this._fileSize) { return false } } @@ -175,7 +174,7 @@ export class ByteRangeContext { if (this.byteEnd < 0) { return false } - if (this.fileSize != null && this.byteEnd > this.fileSize) { + if (this._fileSize != null && this.byteEnd > this._fileSize) { return false } } diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index 833baa33..eb129d5e 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -330,9 +330,9 @@ export class VerifiedFetch { // we have to .stat before .cat so we can get the size and make sure byte offsets are calculated properly if (byteRangeContext.isRangeRequest && byteRangeContext.isValidRangeRequest && terminalElement.type === 'file') { - byteRangeContext.fileSize = terminalElement.unixfs.fileSize() + byteRangeContext.setFileSize(terminalElement.unixfs.fileSize()) - this.log.trace('fileSize for rangeRequest %d', byteRangeContext.fileSize) + this.log.trace('fileSize for rangeRequest %d', byteRangeContext.getFileSize()) } const offset = byteRangeContext.offset const length = byteRangeContext.length diff --git a/packages/verified-fetch/test/utils/byte-range-context.spec.ts b/packages/verified-fetch/test/utils/byte-range-context.spec.ts index 0dc8c8f5..4f95a5b5 100644 --- a/packages/verified-fetch/test/utils/byte-range-context.spec.ts +++ b/packages/verified-fetch/test/utils/byte-range-context.spec.ts @@ -26,7 +26,7 @@ describe('ByteRangeContext', () => { const body = new Uint8Array([1, 2, 3, 4, 5]) context.setBody(body) expect(context.getBody()).to.equal(body) - expect(context.fileSize).to.equal(body.length) + expect(context.getFileSize()).to.equal(body.length) }) it('should correctly handle invalid range request', () => { @@ -137,7 +137,7 @@ describe('ByteRangeContext', () => { leafType: 'file' }) const stat = await fs.stat(cid) - context.fileSize = stat.fileSize + context.setFileSize(stat.fileSize) context.setBody(await getBodyStream(context.offset, context.length)) const response = new Response(context.getBody()) From 3963006506f7e607dea2faff3848258f2eaea0c4 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 15 Mar 2024 13:27:42 -0700 Subject: [PATCH 39/39] chore: remove .stat changes that are no longer needed --- packages/verified-fetch/src/verified-fetch.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index eb129d5e..77ae665e 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -1,6 +1,6 @@ import { car } from '@helia/car' import { ipns as heliaIpns, type IPNS } from '@helia/ipns' -import { unixfs as heliaUnixFs, type UnixFS as HeliaUnixFs, type UnixFSStats } from '@helia/unixfs' +import { unixfs as heliaUnixFs, type UnixFS as HeliaUnixFs } from '@helia/unixfs' import * as ipldDagCbor from '@ipld/dag-cbor' import * as ipldDagJson from '@ipld/dag-json' import { code as dagPbCode } from '@ipld/dag-pb' @@ -290,7 +290,6 @@ export class VerifiedFetch { } let resolvedCID = terminalElement?.cid ?? cid - let stat: UnixFSStats | undefined if (terminalElement?.type === 'directory') { const dirCid = terminalElement.cid @@ -312,7 +311,7 @@ export class VerifiedFetch { const rootFilePath = 'index.html' try { this.log.trace('found directory at %c/%s, looking for index.html', cid, path) - stat = await this.unixfs.stat(dirCid, { + const stat = await this.unixfs.stat(dirCid, { path: rootFilePath, signal: options?.signal, onProgress: options?.onProgress @@ -328,7 +327,7 @@ export class VerifiedFetch { } } - // we have to .stat before .cat so we can get the size and make sure byte offsets are calculated properly + // we have a validRangeRequest & terminalElement is a file, we know the size and should set it if (byteRangeContext.isRangeRequest && byteRangeContext.isValidRangeRequest && terminalElement.type === 'file') { byteRangeContext.setFileSize(terminalElement.unixfs.fileSize())