feat: also check content-type with @sgtpooki/file-type #416
Conversation
| @@ -155,6 +155,7 @@ | |||
| "@ipld/dag-pb": "^4.0.8", | |||
| "@libp2p/interface": "^1.1.2", | |||
| "@libp2p/peer-id": "^4.0.5", | |||
| "@sgtpooki/file-type": "^1.0.0-1e871f8", | |||
There was a problem hiding this comment.
Does this need the specifier or can it be an actual version?
There was a problem hiding this comment.
I published an RC version while messing with exports. I can publish a new version. I believe I updated the NPM_token and publish from github CI should work now. I'll add you to maintainers on that package
|
file-type really looks like it needs a new module that strips out all the old node stuff. We now have |
You can find this out with Somewhat eye-opening: 
Do we really need to be able to detect all of these mime types? |
@achingbrain honestly, no. and mime-types only does extension recognition, where file-type does magic byte recognition (which we need more) so I think removing mime-type would be ideal, and replacing with file-type
yea, if we're open to migrating to a file-type fork I can work on re-adding stream support using browser compatible streams, and also splitting up the node stuff so file reading works as well |
|
removing
|
|
Closing this in favor of #422 |
* adds `contentTypeParser` function to createVerifiedFetch options & implements it. * renamed `getStreamAndContentType` to `getStreamFromAsyncIterable` that now returns a stream with the firstChunk seen, so we can pass it to the `contentTypeParser` function. * updates tests in packages/verified-fetch & packages/interop * updates packageDocumentation with example Related #416 Fixes #422 --------- Co-authored-by: achingbrain <alex@achingbrain.net>
* adds `contentTypeParser` function to createVerifiedFetch options & implements it. * renamed `getStreamAndContentType` to `getStreamFromAsyncIterable` that now returns a stream with the firstChunk seen, so we can pass it to the `contentTypeParser` function. * updates tests in packages/verified-fetch & packages/interop * updates packageDocumentation with example Related ipfs/helia#416 Fixes ipfs/helia#422 --------- Co-authored-by: achingbrain <alex@achingbrain.net>
This PR adds an additional check for content-type via a fork of the
file-typelibrary, similar to how it's used in helia-http-gateway.I forked https://github.com/sindresorhus/file-type because it doesn't currently support browsers due to direct or dependency consumption of node:stream, node:buffer, node:fs, or node:process.
The fork could still use some updates to support streams and confirm that memory usage is reasonable, but it's a start and confirmed working in helia-service-worker-gateway.
Using this fork, we would be able to remove the
file-typedependency from helia-http-gateway and de-dupe the content-type checking code.Related issues in the original repo: