Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix unixfs directory redirects #156

Merged
merged 11 commits into from
Mar 25, 2024
Merged

Conversation

SgtPooki
Copy link
Member

@SgtPooki SgtPooki commented Mar 25, 2024

  • test: add website index.html test
  • deps: @helia/verified-fetch@1.3.2
  • fix: don't manipulate given url
  • test: re-enable trailing slash redirect test

Title

fix: fix unixfs directory redirects

Description

Started as e2e test addition for "Opening a directory with index.html returns index.html" mentioned at #134, ended up fixing #62 and adding a test for it.

Summary of changes:

  1. fixes Trailing / for UnixFS directory normalization is missing  #62
  2. stops manipulating the url received by the service worker and passes it directly to @helia/verified-fetch
  3. adds a test for unixfs dir redirect
  4. adds a test for loading index.html of a given directory CID
  5. adds a way to load fixture data with kubo and use it in e2e tests (similar to how we do it in @helia/verified-fetch-interop)
  6. adds gateway-conformance fixture data car file

Notes & open questions

N/A

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@codecov-commenter
Copy link

codecov-commenter commented Mar 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (2d146e2) to head (6f789cb).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #156   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           35        35           
  Branches         5         5           
=========================================
  Hits            35        35           
Flag Coverage Δ
node 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member Author

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self review


const headers = request.headers
headers.forEach((value, key) => {
log.trace('fetchHandler: request headers: %s: %s', key, value)
})

const response = await verifiedFetch(verifiedFetchUrl, {
const response = await verifiedFetch(event.request.url, {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we no longer need a lot of the URL manipulating code (which was adding a trailing slash and not allowing verified-fetch unixfs dir redirect to work properly)

import drain from 'it-drain'
import type { Controller } from 'ipfsd-ctl'

export async function loadFixtureDataCar (controller: Controller, path: string): Promise<void> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import { path as kuboPath } from 'kubo'
import * as kuboRpcClient from 'kubo-rpc-client'

export async function createKuboNode (): Promise<Controller> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SgtPooki SgtPooki requested a review from lidel March 25, 2024 23:05
@SgtPooki SgtPooki merged commit 4304333 into main Mar 25, 2024
23 checks passed
@SgtPooki SgtPooki deleted the test/e2e-static-website-loading branch March 25, 2024 23:05
@lidel lidel mentioned this pull request Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trailing / for UnixFS directory normalization is missing
2 participants