chore: enable if-none-match conformance tests (#944)

Enables tests for if-none-match headers

Author: achingbrain

package.json

@@ -62,7 +62,7 @@
     "@helia/http": "^3.0.8",
     "@helia/interface": "^6.0.2",
     "@helia/routers": "^4.0.3",
-    "@helia/verified-fetch": "^5.0.4",
+    "@helia/verified-fetch": "^5.1.0",
     "@ipld/dag-cbor": "^9.2.5",
     "@ipld/dag-json": "^10.2.5",
     "@ipld/dag-pb": "^4.1.5",

src/sw/handlers/content-request-handler.ts

@@ -49,7 +49,7 @@ interface FetchHandlerArg {
 const ONE_HOUR_IN_SECONDS = 3600
 
 function getCacheKey (url: URL, headers: Headers, renderPreview: boolean, config: ConfigDb): string {
-  return `${url}-${headers.get('accept') ?? ''}-preview-${renderPreview}-indexes-${config.supportDirectoryIndexes}-redirects-${config.supportWebRedirects}`
+  return `${url}-${headers.get('accept')}-preview-${renderPreview}-indexes-${config.supportDirectoryIndexes}-redirects-${config.supportWebRedirects}-match-${headers.get('if-none-match')}`
 }
 
 async function getResponseFromCacheOrFetch (args: FetchHandlerArg): Promise<Response> {
@@ -295,41 +295,41 @@ async function fetchHandler ({ url, headers, renderPreview, event, logs, subdoma
       renderPreview = shouldRenderDirectory(url, config, accept)
     }
 
-    if (!response.ok) {
+    if (response.status > 399) {
       return fetchErrorPageResponse(resource, init, response, await response.text(), providers, config, firstInstallTime, logs)
     }
 
-    if (renderPreview) {
-      try {
-        return renderEntityPageResponse(url, headers, response, await response.arrayBuffer())
-      } catch (err: any) {
-        log.error('error while loading body to render - %e', err)
-
-        // if the response content involves loading more than one block and
-        // loading a subsequent block fails, the `.arrayBuffer()` promise will
-        // reject with an opaque 'TypeError: Failed to fetch' so show a 502 Bad
-        // Gateway with debugging information
-        return fetchErrorPageResponse(resource, init, new Response('', {
-          status: 502,
-          statusText: 'Bad Gateway',
-          headers: {
-            'Content-Type': 'application/json'
-          }
-        }), JSON.stringify(errorToObject(err), null, 2), providers, config, firstInstallTime, logs)
-      }
-    }
-
-    if (url.searchParams.get('download') === 'true') {
-      // override inline attachments if present
-      let contentDisposition = response.headers.get('content-disposition')
+    if (response.status > 199 && response.status < 300) {
+      if (renderPreview) {
+        try {
+          return renderEntityPageResponse(url, headers, response, await response.arrayBuffer())
+        } catch (err: any) {
+          log.error('error while loading body to render - %e', err)
+
+          // if the response content involves loading more than one block and
+          // loading a subsequent block fails, the `.arrayBuffer()` promise will
+          // reject with an opaque 'TypeError: Failed to fetch' so show a 502 Bad
+          // Gateway with debugging information
+          return fetchErrorPageResponse(resource, init, new Response('', {
+            status: 502,
+            statusText: 'Bad Gateway',
+            headers: {
+              'Content-Type': 'application/json'
+            }
+          }), JSON.stringify(errorToObject(err), null, 2), providers, config, firstInstallTime, logs)
+        }
+      } else if (url.searchParams.get('download') === 'true') {
+        // override inline attachments if present
+        let contentDisposition = response.headers.get('content-disposition')
+
+        if (contentDisposition == null || contentDisposition === '') {
+          contentDisposition = 'attachment'
+        } else if (contentDisposition.startsWith('inline')) {
+          contentDisposition = contentDisposition.replace('inline', 'attachment')
+        }
 
-      if (contentDisposition == null || contentDisposition === '') {
-        contentDisposition = 'attachment'
-      } else if (contentDisposition.startsWith('inline')) {
-        contentDisposition = contentDisposition.replace('inline', 'attachment')
+        response.headers.set('content-disposition', contentDisposition)
       }
-
-      response.headers.set('content-disposition', contentDisposition)
     }
 
     // Create a completely new response object with the same body, status,

test-conformance/fixtures/expected-passing-tests.json

@@ -451,6 +451,16 @@
   "TestGatewayCache/HEAD_for_%2Fipfs%2F_with_only-if-cached_fails_when_not_in_local_datastore/Status_code",
   "TestGatewayCache/GET_for_%2Fipfs%2F_with_only-if-cached_fails_when_not_in_local_datastore",
   "TestGatewayCache/GET_for_%2Fipfs%2F_with_only-if-cached_fails_when_not_in_local_datastore/Status_code",
+  "TestGatewayCache/GET_for_%2Fipfs%2F_file_with_matching_Etag_in_If-None-Match_returns_304_Not_Modified",
+  "TestGatewayCache/GET_for_%2Fipfs%2F_file_with_matching_Etag_in_If-None-Match_returns_304_Not_Modified/Status_code",
+  "TestGatewayCache/GET_for_%2Fipfs%2F_dir_with_index.html_file_with_matching_Etag_in_If-None-Match_returns_304_Not_Modified",
+  "TestGatewayCache/GET_for_%2Fipfs%2F_dir_with_index.html_file_with_matching_Etag_in_If-None-Match_returns_304_Not_Modified/Status_code",
+  "TestGatewayCache/GET_for_%2Fipfs%2F_file_with_matching_third_Etag_in_If-None-Match_returns_304_Not_Modified",
+  "TestGatewayCache/GET_for_%2Fipfs%2F_file_with_matching_third_Etag_in_If-None-Match_returns_304_Not_Modified/Status_code",
+  "TestGatewayCache/GET_for_%2Fipfs%2F_file_with_matching_weak_Etag_in_If-None-Match_returns_304_Not_Modified",
+  "TestGatewayCache/GET_for_%2Fipfs%2F_file_with_matching_weak_Etag_in_If-None-Match_returns_304_Not_Modified/Status_code",
+  "TestGatewayCache/GET_for_%2Fipfs%2F_file_with_wildcard_Etag_in_If-None-Match_returns_304_Not_Modified",
+  "TestGatewayCache/GET_for_%2Fipfs%2F_file_with_wildcard_Etag_in_If-None-Match_returns_304_Not_Modified/Status_code",
   "TestGatewayCacheWithIPNS",
   "TestGatewayCacheWithIPNS/GET_for_%2Fipns%2F_unixfs_dir_as_DAG-JSON_succeeds",
   "TestGatewayCacheWithIPNS/GET_for_%2Fipns%2F_unixfs_dir_as_DAG-JSON_succeeds/Check_0",
@@ -464,6 +474,8 @@
   "TestGatewayCacheWithIPNS/GET_for_%2Fipns%2F_unixfs_dir_as_JSON_succeeds/Check_1",
   "TestGatewayCacheWithIPNS/GET_for_%2Fipns%2F_unixfs_dir_as_JSON_succeeds/Check_1/Check_0",
   "TestGatewayCacheWithIPNS/GET_for_%2Fipns%2F_unixfs_dir_as_JSON_succeeds/Check_1/Check_1",
+  "TestGatewayCacheWithIPNS/GET_for_%2Fipns%2F_file_with_matching_Etag_in_If-None-Match_returns_304_Not_Modified",
+  "TestGatewayCacheWithIPNS/GET_for_%2Fipns%2F_file_with_matching_Etag_in_If-None-Match_returns_304_Not_Modified/Status_code",
   "TestGatewaySymlink",
   "TestGatewaySymlink/Test_the_directory_raw_query",
   "TestGatewaySymlink/Test_the_directory_raw_query/Status_code",
@@ -498,7 +510,6 @@
   "TestPathGatewayMiscellaneous",
   "TestPathGatewayMiscellaneous/GET_for_%2Fipfs%2F_file_whose_filename_contains_percentage-encoded_characters_works",
   "TestPathGatewayMiscellaneous/GET_for_%2Fipfs%2F_file_whose_filename_contains_percentage-encoded_characters_works/Body",
-  "TestRedirectsFileWithIfNoneMatchHeader",
   "TestUnixFSDirectoryListingOnSubdomainGateway",
   "TestGatewaySubdomains",
   "TestTrustlessCarPathing",

test-conformance/fixtures/get-wontfix-tests.ts

@@ -30,15 +30,10 @@ export function getWontFixTests (): string[] {
     'TestGatewayCache/HEAD_for_%2Fipfs%2F_with_only-if-cached_succeeds_when_in_local_datastore',
     'TestGatewayCache/GET_for_%2Fipfs%2F_with_only-if-cached_succeeds_when_in_local_datastore',
 
-    // we cannot send if-none-match header
-    'TestGatewayCache/GET_for_%2Fipfs%2F_file_with_matching_Etag_in_If-None-Match_returns_304_Not_Modified',
-    'TestGatewayCache/GET_for_%2Fipfs%2F_dir_with_index.html_file_with_matching_Etag_in_If-None-Match_returns_304_Not_Modified',
-    'TestGatewayCache/GET_for_%2Fipfs%2F_file_with_matching_third_Etag_in_If-None-Match_returns_304_Not_Modified',
-    'TestGatewayCache/GET_for_%2Fipfs%2F_file_with_matching_weak_Etag_in_If-None-Match_returns_304_Not_Modified',
-    'TestGatewayCache/GET_for_%2Fipfs%2F_file_with_wildcard_Etag_in_If-None-Match_returns_304_Not_Modified',
+    // this test sends an empty weak etag (W/"") which should disable etag
+    // matching but instead it expects it to match and return a 304?
+    // https://github.com/ipfs/gateway-conformance/issues/261
     'TestGatewayCache/GET_for_%2Fipfs%2F_dir_listing_with_matching_weak_Etag_in_If-None-Match_returns_304_Not_Modified',
-    'TestGatewayCacheWithIPNS/GET_for_%2Fipns%2F_file_with_matching_Etag_in_If-None-Match_returns_304_Not_Modified',
-    'TestRedirectsFileWithIfNoneMatchHeader/request_for_%2F%2F%7Bdnslink%7D%2Fmissing-page_with_If-None-Match_returns_304',
 
     // redirects file isn't supported yet
     'TestRedirectsFileSupport',

test-e2e/fixtures/load-ipns-records.ts

@@ -6,11 +6,16 @@ import { glob } from 'glob'
 import itAll from 'it-all'
 import type { KuboNode } from 'ipfsd-ctl'
 
+const FIXTURE_DIRS = [
+  `${process.cwd()}/test-e2e/fixtures/data/**/*.ipns-record`,
+  `${process.cwd()}/test-conformance/fixtures/**/*.ipns-record`
+]
+
 export async function loadIpnsRecords (node: KuboNode): Promise<void> {
   console.info('Loading ipns records')
   let loadedIpnsRecords = 0
 
-  for (const ipnsRecord of await glob([`${process.cwd()}/test-e2e/fixtures/data/**/*.ipns-record`])) {
+  for (const ipnsRecord of await glob(FIXTURE_DIRS)) {
     loadedIpnsRecords++
     console.info('Loading *.ipns-record fixture fullpath:', ipnsRecord)
 

test-e2e/fixtures/load-kubo-fixtures.ts

@@ -27,22 +27,31 @@ const log = logger('kubo-init')
 
 // This needs to match the `repo` property provided to `ipfsd-ctl` in `createKuboNode` so our kubo instance in tests use the same repo
 export const kuboRepoDir = `${tmpdir()}/.ipfs/${Date.now()}`
-export const GWC_FIXTURES_PATH = resolve(__dirname, 'data/gateway-conformance-fixtures')
+export const GWC_FIXTURES_PATH = resolve(__dirname, '../../test-conformance/fixtures/gateway-conformance-fixtures')
 
 export async function downloadFixtures (force = false): Promise<void> {
   if (!force) {
-  // if the fixtures are already downloaded, we don't need to download them again
-    const allFixtures = await glob([`${GWC_FIXTURES_PATH}/**/*.car`, `${GWC_FIXTURES_PATH}/**/*.ipns-record`, `${GWC_FIXTURES_PATH}/dnslinks.json`])
-    if (allFixtures.length > 0) {
-      log('Fixtures already downloaded')
+    // skip downloading fixtures if they are already present
+    const allFixtures = await Promise.all([
+      glob([`${GWC_FIXTURES_PATH}/**/*.car`]),
+      glob([`${GWC_FIXTURES_PATH}/**/*.ipns-record`]),
+      glob([`${GWC_FIXTURES_PATH}/dnslinks.json`])
+    ])
+
+    if (allFixtures.every(fixtures => fixtures.length > 0)) {
+      console.info('Fixtures already downloaded')
       return
     }
   }
 
-  log('Downloading fixtures')
+  console.info('Downloading fixtures')
   try {
     await $`docker run -v ${process.cwd()}:/workspace -w /workspace ghcr.io/ipfs/gateway-conformance:v0.7.1 extract-fixtures --directory ${relative('.', GWC_FIXTURES_PATH)} --merged false`
-  } catch (err) {
+  } catch (err: any) {
+    if (err.message.includes('docker')) {
+      throw err
+    }
+
     log.error('error downloading fixtures, assuming current or previous success - %e', err)
   }
 }
@@ -56,11 +65,16 @@ export async function getIpfsNsMap (): Promise<string> {
   return ipfsNsMap
 }
 
+const FIXTURE_DIRS = [
+  `${process.cwd()}/test-e2e/fixtures/data/**/*.car`,
+  `${process.cwd()}/test-conformance/fixtures/**/*.car`
+]
+
 export async function loadCarFixtures (node: KuboNode): Promise<void> {
   // const execaOptions = getExecaOptions()
   let loadedCarFiles = 0
 
-  for (const carFile of await glob([`${process.cwd()}/test-e2e/fixtures/data/**/*.car`])) {
+  for (const carFile of await glob(FIXTURE_DIRS)) {
     loadedCarFiles++
     console.info('Loading *.car fixture %s', carFile)
 

test-e2e/if-none-match.spec.ts

@@ -0,0 +1,108 @@
+import { stop } from '@libp2p/interface'
+import * as cbor from 'cborg'
+import { createKuboRPCClient } from 'kubo-rpc-client'
+import { testPathRouting as test, expect } from './fixtures/config-test-fixtures.js'
+import { makeFetchRequest } from './fixtures/make-range-request.ts'
+import { setConfig } from './fixtures/set-sw-config.ts'
+import { waitForServiceWorker } from './fixtures/wait-for-service-worker.ts'
+import type { KuboRPCClient } from 'kubo-rpc-client'
+import type { CID } from 'multiformats/cid'
+
+const object = {
+  hello: 'world'
+}
+
+test.describe('if-none-match', () => {
+  let kubo: KuboRPCClient
+  let cid: CID
+  let block: Uint8Array
+
+  test.beforeEach(async ({ page }) => {
+    if (process.env.KUBO_GATEWAY == null || process.env.KUBO_GATEWAY === '') {
+      throw new Error('KUBO_GATEWAY not set')
+    }
+
+    kubo = createKuboRPCClient(process.env.KUBO_RPC)
+
+    block = cbor.encode(object)
+    cid = await kubo.block.put(block, {
+      format: 'cbor'
+    })
+
+    await waitForServiceWorker(page)
+    await setConfig(page, {
+      gateways: [
+        process.env.KUBO_GATEWAY
+      ],
+      routers: [
+        process.env.KUBO_GATEWAY
+      ],
+      acceptOriginIsolationWarning: true,
+      renderHTMLViews: false
+    })
+  })
+
+  test.afterEach(async () => {
+    await stop(kubo)
+  })
+
+  test('should return 304 if the etag matches', async ({ page }) => {
+    const response0 = await makeFetchRequest(page, `/ipfs/${cid}`)
+    expect(response0.status()).toBe(200)
+
+    const etag = await response0.headerValue('etag') ?? ''
+    expect(etag).toContain(cid.toString())
+
+    const response1 = await makeFetchRequest(page, `/ipfs/${cid}`, {
+      headers: {
+        'if-none-match': etag
+      }
+    })
+    expect(response1.status()).toBe(304)
+  })
+
+  test('should return 304 if the etag matches one of the etags', async ({ page }) => {
+    const response0 = await makeFetchRequest(page, `/ipfs/${cid}`)
+    expect(response0.status()).toBe(200)
+
+    const etag = await response0.headerValue('etag') ?? ''
+    expect(etag).toContain(cid.toString())
+
+    const response1 = await makeFetchRequest(page, `/ipfs/${cid}`, {
+      headers: {
+        'if-none-match': `"foo", W/"bar", ${etag}, "baz"`
+      }
+    })
+    expect(response1.status()).toBe(304)
+  })
+
+  test('should return 304 if the etag matches a wildcard', async ({ page }) => {
+    const response0 = await makeFetchRequest(page, `/ipfs/${cid}`)
+    expect(response0.status()).toBe(200)
+
+    const etag = await response0.headerValue('etag') ?? ''
+    expect(etag).toContain(cid.toString())
+
+    const response1 = await makeFetchRequest(page, `/ipfs/${cid}`, {
+      headers: {
+        'if-none-match': '*'
+      }
+    })
+    expect(response1.status()).toBe(304)
+  })
+
+  test('should return 304 if the etag matches a weak value', async ({ page }) => {
+    const response0 = await makeFetchRequest(page, `/ipfs/${cid}`)
+    expect(response0.status()).toBe(200)
+
+    const etag = await response0.headerValue('etag') ?? ''
+    expect(etag).toContain(cid.toString())
+
+    const response1 = await makeFetchRequest(page, `/ipfs/${cid}`, {
+      headers: {
+        'if-none-match': `W/"${cid}.dag-cbor"`
+      }
+    })
+    expect(response1.status()).toBe(304)
+  })
+})