fix: implicit accept header can be overridden by format query (#36)

* fix: implicit accept header can be overridden by format query * chore: some cleanup and optimizations * chore: forgot to add files * test: uncomment accept header in test * chore: apply suggestions from code review Co-authored-by: Alex Potsides <[email protected]> * chore: logger is always set, should not be optional --------- Co-authored-by: Alex Potsides <[email protected]>
ipfs · Apr 8, 2024 · 75c0b75 · 75c0b75
1 parent e00d41c
commit 75c0b75
Show file tree

Hide file tree

Showing 5 changed files with 144 additions and 20 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
@@ -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
@@ -0,0 +1,31 @@
+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)
+}
diff --git 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<number, string[]> = {
     '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<RequestFormatShorthand, string> = {
+export const FORMAT_TO_MIME_TYPE: Record<RequestFormatShorthand, string> = {
   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
@@ -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 } 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<VerifiedFetchOpt
  * skipped and set to these values.
  */
 const RAW_HEADERS = [
+  'application/vnd.ipld.dag-json',
   'application/vnd.ipld.raw',
   'application/octet-stream'
 ]
@@ -103,8 +105,9 @@ const RAW_HEADERS = [
  * type. This avoids the user from receiving something different when they
  * signal that they want to `Accept` a specific mime type.
  */
-function getOverridenRawContentType (headers?: HeadersInit): string | undefined {
-  const acceptHeader = new Headers(headers).get('accept') ?? ''
+function getOverridenRawContentType ({ headers, accept }: { headers?: HeadersInit, accept?: string }): string | undefined {
+  // accept has already been resolved by getResolvedAcceptHeader, if we have it, use it.
+  const acceptHeader = accept ?? new Headers(headers).get('accept') ?? ''
 
   // e.g. "Accept: text/html, application/xhtml+xml, application/xml;q=0.9, image/webp, */*;q=0.8"
   const acceptHeaders = acceptHeader.split(',')
@@ -385,7 +388,7 @@ export class VerifiedFetch {
     }
   }
 
-  private async handleRaw ({ resource, cid, path, options }: FetchHandlerFunctionArg): Promise<Response> {
+  private async handleRaw ({ resource, cid, path, options, accept }: FetchHandlerFunctionArg): Promise<Response> {
     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 {
@@ -484,20 +487,8 @@ export class VerifiedFetch {
 
     options?.onProgress?.(new CustomProgressEvent<CIDDetail>('verified-fetch:request:resolve', { cid, path }))
 
-    const requestHeaders = new Headers(options?.headers)
-    const incomingAcceptHeader = requestHeaders.get('accept')
+    const acceptHeader = getResolvedAcceptHeader({ query, headers: options?.headers, logger: this.helia.logger })
 
-    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)
-    }
-
-    const acceptHeader = incomingAcceptHeader ?? queryFormatMapping
     const accept = selectOutputType(cid, acceptHeader)
     this.log('output type %s', accept)
 
@@ -508,7 +499,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
@@ -748,4 +748,63 @@ describe('@helia/verifed-fetch', () => {
       expect(new Uint8Array(data)).to.equalBytes(finalRootFileContent)
     })
   })
+
+  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(resp.headers.get('content-type')).to.equal('application/vnd.ipld.dag-json')
+      expect(new Uint8Array(data)).to.equalBytes(finalRootFileContent)
+    })
+  })
 })