-
Notifications
You must be signed in to change notification settings - Fork 845
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
chore: update ipfs deps #1899
chore: update ipfs deps #1899
Conversation
@lidel after upgrading the IPFS-related dependencies, now I get a 403 error whenever we try to call |
c20f7d9
to
edbe0c8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I rebased this PR on top of main
which already included all the other updates)
@hacdias no idea what changed – perhaps some fetch
polyfill deep inside on dependencies? I've seen some dark arts happening around ipfs/js-ipfs-utils#136 👻
Setting Access-Control-Allow-Origin
is a security risk, we need to find a proper fix.
@achingbrain does this look familiar? Did anything land in js-ipfs-http-client (or its deps) that could change the way |
@lidel nothing springs immediately to mind & we're running ipfs/js-ipfs-utils#136 is just to stop using the undocumented
|
@achingbrain upgrading from 47.x to 48.x seems to create the issue. I wonder if it is related to ipfs/js-ipfs#3275 It seems there's an header 'Origin' with the value 'null' being sent. Shouldn't it just be omitted? (Adding 'null' to the allowed origins works, but setting the mode to no-cors does not work) |
License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>
License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>
edbe0c8
to
a2e8917
Compare
@lidel seems that removing our old CORS logic for IPFS Desktop solves the issue successfully. Could you please take a look at this PR? (PS: I could not run the tests locally - electron-userland/spectron#929 - gotta work on #1937) |
We already use i18next-fs-backend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but will merge after I'm back in January :)
License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>
This comment has been minimized.
This comment has been minimized.
This fixes regression introduced by #1899
* chore: smaller dep updates * fix: correctly remove $IPFS_PATH/api file This fixes regression introduced by #1899
Depends on #1895.
License: MIT
Signed-off-by: Henrique Dias hacdias@gmail.com