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: identity CIDs use contentTypeParser #49

Conversation

SgtPooki
Copy link
Member

@SgtPooki SgtPooki commented Apr 10, 2024

Title

fix: identity CIDs use contentTypeParser

Description

latest update of @helia/verified-fetch caused failing tests in service-worker-gateway: ipfs/service-worker-gateway#187

Fixes #48

Notes & open questions

This change normalizes how we handle setting content-type for raw CIDs and identity CIDs and was required to ensure that the content-type is set correctly for identity CIDs.

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

@SgtPooki SgtPooki requested a review from a team as a code owner April 10, 2024 23:24
@SgtPooki SgtPooki linked an issue Apr 10, 2024 that may be closed by this pull request
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

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 of test changes. added some info as comments

@@ -97,7 +97,7 @@
"@sgtpooki/file-type": "^1.0.1",
"@types/sinon": "^17.0.3",
"aegir": "^42.2.5",
"blockstore-core": "^4.4.0",
"blockstore-core": "^4.4.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

update to get ipfs/js-stores#303

import Sinon from 'sinon'
import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string'
import { createVerifiedFetch } from '../src/index.js'
import { VerifiedFetch } from '../src/verified-fetch.js'
import { createHelia } from './fixtures/create-offline-helia.js'
Copy link
Member Author

Choose a reason for hiding this comment

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

use the offline helia.

Comment on lines +136 to +146
it('can properly set content type for identity CIDs', async () => {
verifiedFetch = new VerifiedFetch({
helia
}, {
contentTypeParser: async (data) => {
return 'text/plain'
}
})
const resp = await verifiedFetch.fetch(CID.parse('bafkqablimvwgy3y'))
expect(resp.headers.get('content-type')).to.equal('text/plain')
})
Copy link
Member Author

Choose a reason for hiding this comment

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

test for this fix

import { MemoryDatastore } from 'datastore-core'
import type { HeliaHTTPInit } from '@helia/http'
import type { Helia } from '@helia/interface'

export async function createHelia (init: Partial<HeliaHTTPInit> = {}): Promise<Helia> {
const datastore = new MemoryDatastore()
const blockstore = new MemoryBlockstore()
const blockstore = new IdentityBlockstore(new MemoryBlockstore())
Copy link
Member Author

Choose a reason for hiding this comment

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

allow identity blocks when testing

@SgtPooki
Copy link
Member Author

CI action hung for some reason. cancelled and restarted

@SgtPooki SgtPooki merged commit 3014498 into main Apr 11, 2024
17 checks passed
@SgtPooki SgtPooki deleted the 48-bug-requests-for-identity-cid-is-not-being-handled-properly-anymore branch April 11, 2024 00:21
github-actions bot pushed a commit that referenced this pull request Apr 11, 2024
## @helia/verified-fetch [1.3.9](https://github.com/ipfs/helia-verified-fetch/compare/@helia/verified-fetch-1.3.8...@helia/verified-fetch-1.3.9) (2024-04-11)

### Bug Fixes

* identity CIDs use contentTypeParser ([#49](#49)) ([3014498](3014498))
Copy link

🎉 This PR is included in version 1.3.9 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

Copy link

🎉 This PR is included in version 1.16.0 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

Copy link

github-actions bot commented May 9, 2024

🎉 This PR is included in version 1.0.0 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: requests for identity CID is not being handled properly anymore
1 participant