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

ipfs://CIDv0 does not work in Firefox 70 #815

Closed
2 tasks done
lidel opened this issue Nov 14, 2019 · 9 comments · Fixed by #824 or #911
Closed
2 tasks done

ipfs://CIDv0 does not work in Firefox 70 #815

lidel opened this issue Nov 14, 2019 · 9 comments · Fixed by #824 or #911
Assignees
Labels
area/cidv1b32 Moving to CIDv1/Base32 by default good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws)

Comments

@lidel
Copy link
Member

lidel commented Nov 14, 2019

Part of https://github.com/ipfs/ipfs/issues/337

Catching unhandled protocols is an experiment enabled by default:

2019-11-15--12-52-39

Issue with address bar / a href

ipfs://QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR
gets lower-cased and produces an error in Firefox 70:

Screenshot_2019-11-14 Issue with Protocol Handler

Issue with img src

<img src="ipfs://QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR" />

Fails to load.

Todo

  • see if it happens before or after handler in webextension
  • update error page to educate about case-sensitivity in native URLs and suggest use of CIDv1b32
    • idea: text field for pasting CIDv0 that does automatic conversion ?

cc #527

@lidel lidel added kind/bug A bug in existing code (including security flaws) UX area/cidv1b32 Moving to CIDv1/Base32 by default good first issue Good issue for new contributors help wanted Seeking public contribution on this issue labels Nov 14, 2019
lidel added a commit that referenced this issue Dec 3, 2019
We are unable to fix this (URI is lowercased before passed to
extension), but we can inform user about manual fix.

This also adds SRI for CSS assets.

Closes #815
@lidel lidel closed this as completed in #824 Dec 3, 2019
lidel added a commit that referenced this issue Dec 3, 2019
We are unable to fix this (URI is lowercased before passed to
extension), but we can inform user about manual fix.

This also adds SRI for CSS assets.

Closes #815
@andrew
Copy link

andrew commented Jun 30, 2020

I ran into this today, the display of the fix in #824 being in a red box didn't help me understand the fix as I skimmed right over that, only after went to report an issue, searched and found this then went back and looked again did I realize what the page was saying, may need some tweaking of the display and/or text? cc @jessicaschilling

@jessicaschilling
Copy link
Contributor

jessicaschilling commented Jun 30, 2020

@andrew -- thanks for raising this. Agreed that we can make the text in the red box more specific and helpful (and therefore less likely to be ignored). I'm currently working through a crib sheet of miscellaneous Companion UI fixes and will add it to that and name-check this issue when the changes make their way into a PR.

@jessicaschilling
Copy link
Contributor

@lidel -- Sorry to ask for the hint, but can I check in on the best way of addressing this when you have the chance? Unless I'm missing something, sounds like the best approach is just to modify the error page and then point Companion to the new page's CID. True?

@lidel
Copy link
Member Author

lidel commented Jul 8, 2020

@jessicaschilling Correct.

This error page is loaded from IPFS because of how Firefox protocol_handler API works (this entire issue is Firefox-specific): the redirect has to be to a remote Origin, so I used public gateway.

If you want to tackle this, update CID with new version in those places:

@jessicaschilling
Copy link
Contributor

@lidel and @andrew -- how about this?
https://gateway.ipfs.io/ipfs/QmTcoFD4WqmcT3uA7z3cF42uMRFJmnHZfPUjmdm8Du5gew

Local screenshots with fake URIs bashed in for the sake of illustration:

@lidel, note inline favicon 😉

If either of you would be kind enough to look through the code for that page, would greatly appreciate -- once we're OK with the page I'll change the CID references in Companion in a new PR.

@jessicaschilling jessicaschilling self-assigned this Jul 13, 2020
@lidel
Copy link
Member Author

lidel commented Jul 15, 2020

Can we add ipns:// example to:

  • A valid CIDv1 – this looks like ipfs://{CIDv1} or ipns://{CIDv1}

And shorten DNSLink one to:

  • A valid DNSLink – this looks like ipns://{domain-name-with-DNSLink}

Otherwise LGTM!
Feel free to open a PR that updates the CID everywhere 👍

@jessicaschilling
Copy link
Contributor

jessicaschilling commented Jul 16, 2020

Updated at https://gateway.ipfs.io/ipfs/QmVGC4uCBDVEhCzsaJmvR5nVDgChM97kcYNehVm7L9jxtc ...

PR is here: #911

@autonome
Copy link

@lidel can we autorewrite v0 to v1 in this case? Or do it in the "cannot process" page on client side, and have "click here to try v1 CID" instead of putting it on the user to do in CID Inspector?

@lidel
Copy link
Member Author

lidel commented Jan 16, 2022

@autonome we are unable to autofix – Firefox performs lowercasing before extension can read the CID, and the lowercasing is a lossy operation which we can't undo.

We could have converter on the error page (instead of linking to https://cid.ipfs.io),
but even then, the user would have to manually paste the original CID/URI to input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cidv1b32 Moving to CIDv1/Base32 by default good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws)
Projects
None yet
4 participants