From 02c3c456f05260db0e1cc6cd310b2c863ac504cc Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 29 Mar 2024 18:03:09 -0700 Subject: [PATCH 1/6] fix: implicit accept header can be overridden by format query --- .../src/utils/select-output-type.ts | 3 +- packages/verified-fetch/src/verified-fetch.ts | 13 ++++- .../test/verified-fetch.spec.ts | 58 +++++++++++++++++++ 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/packages/verified-fetch/src/utils/select-output-type.ts b/packages/verified-fetch/src/utils/select-output-type.ts index 0f7c40c9..53d76326 100644 --- a/packages/verified-fetch/src/utils/select-output-type.ts +++ b/packages/verified-fetch/src/utils/select-output-type.ts @@ -55,6 +55,7 @@ const CID_TYPE_MAP: Record = { 'application/octet-stream', 'application/vnd.ipld.raw', 'application/vnd.ipfs.ipns-record', + 'application/vnd.ipld.dag-json', 'application/vnd.ipld.car', 'application/x-tar' ] @@ -145,7 +146,7 @@ function parseQFactor (str?: string): number { return factor } -const FORMAT_TO_MIME_TYPE: Record = { +export const FORMAT_TO_MIME_TYPE: Record = { raw: 'application/vnd.ipld.raw', car: 'application/vnd.ipld.car', 'dag-json': 'application/vnd.ipld.dag-json', diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index 304ad6f6..b9edf8a2 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -25,7 +25,7 @@ import { tarStream } from './utils/get-tar-stream.js' import { parseResource } from './utils/parse-resource.js' import { setCacheControlHeader } from './utils/response-headers.js' import { badRequestResponse, movedPermanentlyResponse, notAcceptableResponse, notSupportedResponse, okResponse, badRangeResponse, okRangeResponse, badGatewayResponse } from './utils/responses.js' -import { selectOutputType, queryFormatToAcceptHeader } from './utils/select-output-type.js' +import { selectOutputType, queryFormatToAcceptHeader, FORMAT_TO_MIME_TYPE } 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' @@ -512,7 +512,16 @@ export class VerifiedFetch { this.log('incoming query format "%s", mapped to %s', query.format, queryFormatMapping) } - const acceptHeader = incomingAcceptHeader ?? queryFormatMapping + let acceptHeader: string | undefined + // if the incomingAcceptHeader is autogenerated by the requesting client (browser/curl/fetch/etc) then we may need to override it if query.format is specified + if (queryFormatMapping != null && (incomingAcceptHeader == null || !Object.values(FORMAT_TO_MIME_TYPE).includes(incomingAcceptHeader))) { + this.log('accept header not recognized, but query format provided, setting accept header to %s', queryFormatMapping) + acceptHeader = queryFormatMapping + } else { + acceptHeader = incomingAcceptHeader ?? queryFormatMapping + } + this.log('determined accept header "%s"', acceptHeader) + const accept = selectOutputType(cid, acceptHeader) this.log('output type %s', accept) diff --git a/packages/verified-fetch/test/verified-fetch.spec.ts b/packages/verified-fetch/test/verified-fetch.spec.ts index 64cbc84e..bdd0af98 100644 --- a/packages/verified-fetch/test/verified-fetch.spec.ts +++ b/packages/verified-fetch/test/verified-fetch.spec.ts @@ -710,4 +710,62 @@ describe('@helia/verifed-fetch', () => { expect(output).to.deep.equal(obj) }) }) + + describe('?format', () => { + let helia: Helia + let verifiedFetch: VerifiedFetch + let contentTypeParser: Sinon.SinonStub + + beforeEach(async () => { + contentTypeParser = Sinon.stub() + helia = await createHelia() + verifiedFetch = new VerifiedFetch({ + helia + }, { + contentTypeParser + }) + }) + + afterEach(async () => { + await stop(helia, verifiedFetch) + }) + + it('cbor?format=dag-json should be able to override curl/browser default accept header when query parameter is provided', async () => { + const obj = { + hello: 'world' + } + const c = dagCbor(helia) + const cid = await c.add(obj) + + const resp = await verifiedFetch.fetch(`http://example.com/ipfs/${cid}?format=dag-json`, { + headers: { + // see https://github.com/ipfs/helia-verified-fetch/issues/35 + // accept: '*/*' + } + }) + expect(resp.headers.get('content-type')).to.equal('application/vnd.ipld.dag-json') + const data = ipldDagJson.decode(await resp.arrayBuffer()) + expect(data).to.deep.equal(obj) + }) + + it('raw?format=dag-json should be able to override curl/browser default accept header when query parameter is provided', async () => { + const finalRootFileContent = uint8ArrayFromString(JSON.stringify({ + hello: 'world' + })) + const cid = CID.createV1(raw.code, await sha256.digest(finalRootFileContent)) + await helia.blockstore.put(cid, finalRootFileContent) + + const resp = await verifiedFetch.fetch(`http://example.com/ipfs/${cid}?format=dag-json`, { + headers: { + // see https://github.com/ipfs/helia-verified-fetch/issues/35 + accept: '*/*' + } + }) + expect(resp).to.be.ok() + expect(resp.status).to.equal(200) + expect(resp.statusText).to.equal('OK') + const data = await resp.arrayBuffer() + expect(new Uint8Array(data)).to.equalBytes(finalRootFileContent) + }) + }) }) From b013774406478e85f2025c6090f91ca2b103d005 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 29 Mar 2024 19:03:54 -0700 Subject: [PATCH 2/6] chore: some cleanup and optimizations --- packages/verified-fetch/src/verified-fetch.ts | 38 +++++-------------- .../test/verified-fetch.spec.ts | 1 + 2 files changed, 11 insertions(+), 28 deletions(-) diff --git a/packages/verified-fetch/src/verified-fetch.ts b/packages/verified-fetch/src/verified-fetch.ts index b9edf8a2..56ebb188 100644 --- a/packages/verified-fetch/src/verified-fetch.ts +++ b/packages/verified-fetch/src/verified-fetch.ts @@ -20,12 +20,13 @@ 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 { getResolvedAcceptHeader } from './utils/get-resolved-accept-header.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 { setCacheControlHeader } from './utils/response-headers.js' import { badRequestResponse, movedPermanentlyResponse, notAcceptableResponse, notSupportedResponse, okResponse, badRangeResponse, okRangeResponse, badGatewayResponse } from './utils/responses.js' -import { selectOutputType, queryFormatToAcceptHeader, FORMAT_TO_MIME_TYPE } from './utils/select-output-type.js' +import { selectOutputType } 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' @@ -93,6 +94,7 @@ function convertOptions (options?: VerifiedFetchOptions): (Omit { + private async handleRaw ({ resource, cid, path, options, accept }: FetchHandlerFunctionArg): Promise { const byteRangeContext = new ByteRangeContext(this.helia.logger, options?.headers) const result = await this.helia.blockstore.get(cid, options) byteRangeContext.setBody(result) @@ -396,7 +399,7 @@ export class VerifiedFetch { // if the user has specified an `Accept` header that corresponds to a raw // type, honour that header, so for example they don't request // `application/vnd.ipld.raw` but get `application/octet-stream` - const overriddenContentType = getOverridenRawContentType(options?.headers) + const overriddenContentType = getOverridenRawContentType({ headers: options?.headers, accept }) if (overriddenContentType != null) { response.headers.set('content-type', overriddenContentType) } else { @@ -499,28 +502,7 @@ export class VerifiedFetch { options?.onProgress?.(new CustomProgressEvent('verified-fetch:request:resolve', { cid, path })) - const requestHeaders = new Headers(options?.headers) - const incomingAcceptHeader = requestHeaders.get('accept') - - if (incomingAcceptHeader != null) { - this.log('incoming accept header "%s"', incomingAcceptHeader) - } - - const queryFormatMapping = queryFormatToAcceptHeader(query.format) - - if (query.format != null) { - this.log('incoming query format "%s", mapped to %s', query.format, queryFormatMapping) - } - - let acceptHeader: string | undefined - // if the incomingAcceptHeader is autogenerated by the requesting client (browser/curl/fetch/etc) then we may need to override it if query.format is specified - if (queryFormatMapping != null && (incomingAcceptHeader == null || !Object.values(FORMAT_TO_MIME_TYPE).includes(incomingAcceptHeader))) { - this.log('accept header not recognized, but query format provided, setting accept header to %s', queryFormatMapping) - acceptHeader = queryFormatMapping - } else { - acceptHeader = incomingAcceptHeader ?? queryFormatMapping - } - this.log('determined accept header "%s"', acceptHeader) + const acceptHeader = getResolvedAcceptHeader({ query, headers: options?.headers, logger: this.helia.logger }) const accept = selectOutputType(cid, acceptHeader) this.log('output type %s', accept) @@ -532,7 +514,7 @@ export class VerifiedFetch { let response: Response let reqFormat: RequestFormatShorthand | undefined - const handlerArgs = { resource: resource.toString(), cid, path, accept, options } + const handlerArgs: FetchHandlerFunctionArg = { resource: resource.toString(), cid, path, accept, options } if (accept === 'application/vnd.ipfs.ipns-record') { // the user requested a raw IPNS record diff --git a/packages/verified-fetch/test/verified-fetch.spec.ts b/packages/verified-fetch/test/verified-fetch.spec.ts index bdd0af98..59bb0270 100644 --- a/packages/verified-fetch/test/verified-fetch.spec.ts +++ b/packages/verified-fetch/test/verified-fetch.spec.ts @@ -765,6 +765,7 @@ describe('@helia/verifed-fetch', () => { expect(resp.status).to.equal(200) expect(resp.statusText).to.equal('OK') const data = await resp.arrayBuffer() + expect(resp.headers.get('content-type')).to.equal('application/vnd.ipld.dag-json') expect(new Uint8Array(data)).to.equalBytes(finalRootFileContent) }) }) From 54b7454baa8e9e7d4ac93cdf53b06ad08c7be51a Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 29 Mar 2024 19:15:30 -0700 Subject: [PATCH 3/6] chore: forgot to add files --- .../src/utils/get-resolved-accept-header.ts | 42 +++++++++++++++++++ .../src/utils/is-accept-explicit.ts | 32 ++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 packages/verified-fetch/src/utils/get-resolved-accept-header.ts create mode 100644 packages/verified-fetch/src/utils/is-accept-explicit.ts diff --git a/packages/verified-fetch/src/utils/get-resolved-accept-header.ts b/packages/verified-fetch/src/utils/get-resolved-accept-header.ts new file mode 100644 index 00000000..05d4d5de --- /dev/null +++ b/packages/verified-fetch/src/utils/get-resolved-accept-header.ts @@ -0,0 +1,42 @@ +import { isExplicitAcceptHeader, isExplicitFormatQuery, isExplicitIpldAcceptRequest } from './is-accept-explicit.js' +import { queryFormatToAcceptHeader } from './select-output-type.js' +import type { ParsedUrlStringResults } from './parse-url-string.js' +import type { ComponentLogger } from '@libp2p/interface' + +export interface ResolvedAcceptHeaderOptions { + query?: ParsedUrlStringResults['query'] + headers?: RequestInit['headers'] + logger?: ComponentLogger +} + +export function getResolvedAcceptHeader ({ query, headers, logger }: ResolvedAcceptHeaderOptions): string | undefined { + const log = logger?.forComponent('helia:verified-fetch:get-resolved-accept-header') + const requestHeaders = new Headers(headers) + const incomingAcceptHeader = requestHeaders.get('accept') ?? undefined + + if (incomingAcceptHeader != null) { + log?.('incoming accept header "%s"', incomingAcceptHeader) + } + + if (!isExplicitIpldAcceptRequest({ query, headers: requestHeaders })) { + log?.('no explicit IPLD content-type requested, returning incoming accept header %s', incomingAcceptHeader) + return incomingAcceptHeader + } + + const queryFormatMapping = queryFormatToAcceptHeader(query?.format) + + if (query?.format != null) { + log?.('incoming query format "%s", mapped to %s', query.format, queryFormatMapping) + } + + let acceptHeader = incomingAcceptHeader + // if the incomingAcceptHeader is autogenerated by the requesting client (browser/curl/fetch/etc) then we may need to override it if query.format is specified + if (!isExplicitAcceptHeader(requestHeaders) && isExplicitFormatQuery(query)) { + log?.('accept header not recognized, but query format provided, setting accept header to %s', queryFormatMapping) + acceptHeader = queryFormatMapping + } + + log?.('resolved accept header to "%s"', acceptHeader) + + return acceptHeader +} diff --git a/packages/verified-fetch/src/utils/is-accept-explicit.ts b/packages/verified-fetch/src/utils/is-accept-explicit.ts new file mode 100644 index 00000000..934b3348 --- /dev/null +++ b/packages/verified-fetch/src/utils/is-accept-explicit.ts @@ -0,0 +1,32 @@ +import { FORMAT_TO_MIME_TYPE } from './select-output-type.js' +import type { ParsedUrlStringResults } from './parse-url-string.js' + +export interface IsAcceptExplicitOptions { + + query?: ParsedUrlStringResults['query'] + headers: Headers +} + +export function isExplicitAcceptHeader (headers: Headers): boolean { + const incomingAcceptHeader = headers.get('accept') + if (incomingAcceptHeader != null && Object.values(FORMAT_TO_MIME_TYPE).includes(incomingAcceptHeader)) { + return true + } + return false +} + +export function isExplicitFormatQuery (query?: ParsedUrlStringResults['query']): boolean { + const formatQuery = query?.format + if (formatQuery != null && Object.keys(FORMAT_TO_MIME_TYPE).includes(formatQuery)) { + return true + } + return false +} + +/** + * The user can provide an explicit `accept` header in the request headers or a `format` query parameter in the URL. + * If either of these are provided, this function returns true. + */ +export function isExplicitIpldAcceptRequest ({ query, headers }: IsAcceptExplicitOptions): boolean { + return isExplicitAcceptHeader(headers) || isExplicitFormatQuery(query) +} From dd2e4fc8ab1ee7e217e7d7395fde186c176f2e67 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Mon, 8 Apr 2024 09:43:41 -0700 Subject: [PATCH 4/6] test: uncomment accept header in test --- packages/verified-fetch/test/verified-fetch.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/verified-fetch/test/verified-fetch.spec.ts b/packages/verified-fetch/test/verified-fetch.spec.ts index b58436c4..52b086b9 100644 --- a/packages/verified-fetch/test/verified-fetch.spec.ts +++ b/packages/verified-fetch/test/verified-fetch.spec.ts @@ -778,7 +778,7 @@ describe('@helia/verifed-fetch', () => { const resp = await verifiedFetch.fetch(`http://example.com/ipfs/${cid}?format=dag-json`, { headers: { // see https://github.com/ipfs/helia-verified-fetch/issues/35 - // accept: '*/*' + accept: '*/*' } }) expect(resp.headers.get('content-type')).to.equal('application/vnd.ipld.dag-json') From 23bdb260a027f7650fdcad5153141a757426aa03 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Mon, 8 Apr 2024 09:47:25 -0700 Subject: [PATCH 5/6] chore: apply suggestions from code review Co-authored-by: Alex Potsides --- packages/verified-fetch/src/utils/is-accept-explicit.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/verified-fetch/src/utils/is-accept-explicit.ts b/packages/verified-fetch/src/utils/is-accept-explicit.ts index 934b3348..bca7a7e1 100644 --- a/packages/verified-fetch/src/utils/is-accept-explicit.ts +++ b/packages/verified-fetch/src/utils/is-accept-explicit.ts @@ -2,7 +2,6 @@ import { FORMAT_TO_MIME_TYPE } from './select-output-type.js' import type { ParsedUrlStringResults } from './parse-url-string.js' export interface IsAcceptExplicitOptions { - query?: ParsedUrlStringResults['query'] headers: Headers } From 91cad3a41a986cb41c8483d05de67d6cbe076db0 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Mon, 8 Apr 2024 09:46:51 -0700 Subject: [PATCH 6/6] chore: logger is always set, should not be optional --- .../src/utils/get-resolved-accept-header.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/verified-fetch/src/utils/get-resolved-accept-header.ts b/packages/verified-fetch/src/utils/get-resolved-accept-header.ts index 05d4d5de..711b7879 100644 --- a/packages/verified-fetch/src/utils/get-resolved-accept-header.ts +++ b/packages/verified-fetch/src/utils/get-resolved-accept-header.ts @@ -6,37 +6,37 @@ import type { ComponentLogger } from '@libp2p/interface' export interface ResolvedAcceptHeaderOptions { query?: ParsedUrlStringResults['query'] headers?: RequestInit['headers'] - logger?: ComponentLogger + logger: ComponentLogger } export function getResolvedAcceptHeader ({ query, headers, logger }: ResolvedAcceptHeaderOptions): string | undefined { - const log = logger?.forComponent('helia:verified-fetch:get-resolved-accept-header') + const log = logger.forComponent('helia:verified-fetch:get-resolved-accept-header') const requestHeaders = new Headers(headers) const incomingAcceptHeader = requestHeaders.get('accept') ?? undefined if (incomingAcceptHeader != null) { - log?.('incoming accept header "%s"', incomingAcceptHeader) + log('incoming accept header "%s"', incomingAcceptHeader) } if (!isExplicitIpldAcceptRequest({ query, headers: requestHeaders })) { - log?.('no explicit IPLD content-type requested, returning incoming accept header %s', incomingAcceptHeader) + log('no explicit IPLD content-type requested, returning incoming accept header %s', incomingAcceptHeader) return incomingAcceptHeader } const queryFormatMapping = queryFormatToAcceptHeader(query?.format) if (query?.format != null) { - log?.('incoming query format "%s", mapped to %s', query.format, queryFormatMapping) + log('incoming query format "%s", mapped to %s', query.format, queryFormatMapping) } let acceptHeader = incomingAcceptHeader // if the incomingAcceptHeader is autogenerated by the requesting client (browser/curl/fetch/etc) then we may need to override it if query.format is specified if (!isExplicitAcceptHeader(requestHeaders) && isExplicitFormatQuery(query)) { - log?.('accept header not recognized, but query format provided, setting accept header to %s', queryFormatMapping) + log('accept header not recognized, but query format provided, setting accept header to %s', queryFormatMapping) acceptHeader = queryFormatMapping } - log?.('resolved accept header to "%s"', acceptHeader) + log('resolved accept header to "%s"', acceptHeader) return acceptHeader }