support for fs: URIs without redirecting to http: #70

the8472 · 2016-02-11T16:34:31Z

It works with and without e10s for basic resource loading and I also tested it against the case in #3. But it most likely needs some further testing to weed out edge-cases, so i've marked it as default-off and experimental.

Basically, from firefox's view this all runs under CORS policy because we're running the HTTP requests with the principals of the fs:/ document instead of implementing our own channels which then delegate to a privileged HTTP channel
Getting the URLs a tiny bit wrong causes redirects from the gateways which then can then void the CORS permissions under circumstances.

Implementing as a custom channel wrapper around http channels might be more robust wrt. CORS but currently is way too troublesome under e10s until the FF devs improve support (or documentation) there.

the8472 · 2016-02-11T16:56:19Z

Idk why the tests fail on travis. Running jpm test passes for me.

lidel · 2016-02-11T19:02:51Z

Thank you! I'll do my best to review and merge it this weekend.

For now I did a quick check and tests pass for me too.

It is probably a long shot, but maybe nodejs at Travis is too old?
Try updating .travis.yml in a commit to this branch (4.1 seems to be the max):

node_js:
-    - "0.12"
+    - "4.1"

and let's see if it helps.

the8472 · 2016-02-11T20:16:58Z

added that.

Another thing of note, to make jpm happy locally I had to change the .jpmignore file. Does it work as it is in the git tree for you?

lidel · 2016-02-11T21:18:13Z

test/test-canonical-fs-uris.js

+    url: testpage,
+    onReady: (tab) => {
+
+      // first load somehow doesn't have protocol handlers registered. so load resource:// first, then redirect to fs:/ page


Yeah, this is the same bug as in #33 (comment)
We probably need to do this workaround for now, hopefully it will be fixed in WebExtensions.

lidel · 2016-02-11T21:44:32Z

jpmignore works fine (jpm 1.0.5) for me -- I've built .xpi and contents included only whitelisted files
Travis still fails, so it is not nodejs. Perhaps Travis is slower than our machines, and asynchronous callbacks that are fired after prefs change collide with URI tests from test/test-protocols.js.. or we have some race condition and under slow runner prefs.fsUris is sometimes left as true? No idea.

This seems to confirm that fs: is still present in address bar, while it is expected to be changed into http link.

Perhaps you could make these tests independent of the default value, and explicitly disable fsUris inside of each one (and perhaps add another set for prefs.fsUris = true):
```
+ const { prefs } = require('sdk/simple-prefs')

exports['test newURI(fs:ipfs/<path>)'] = function (assert) {
+  prefs.fsUris = false
 var newURI = fsHandler.newURI('fs:ipfs/' + ipfsPath, 'utf8', null)
assert.equal(newURI.spec, pubGwUri.spec + 'ipfs/' + ipfsPath, 'newURI spec')
}
```
Sadly I don't have any better idea (feel free to add as many commits as you need).

the8472 · 2016-02-11T21:58:27Z

running tests on Firefox 44.0.1/Gecko 44.0.1

That seems to be latest stable. Is there no nightly?

lidel · 2016-02-11T22:08:45Z

No nightly. Currently tests are run against latest stable, beta, and every .0 release since 43.0:
https://travis-ci.org/lidel/ipfs-firefox-addon/builds/108628692

the8472 · 2016-02-12T22:00:17Z

Well, tests pass for me now. I blame whatever doesn't work on travis

lidel · 2016-02-13T14:41:25Z

I did some manual tests locally under 44, 45 and 47a1.

Indeed super weird: automated tests pass for me too, but when I tried to actually use it (enabled fsUris on the Preferences screen) it does not seem to work: i am always redirected to http://127.0.0.1:8080/ipfs/(...).

Interestingly, it seems to work fine if redirect is disabled AND gateway is offline:
I see fs: instead of http://ipfs.io/ipfs/(..)

Could you please check if fs:/ipfs/QmYHNYAaYK5hm3ZhZFx5W9H6xydKDGimjdgJMrMSdnctEm works for you when redirect is enabled and gateway is online?

the8472 · 2016-02-13T15:09:55Z

It displays the fs:/ uri for me in nightly and loads the cat. Works both with local and ipfs.io gateway.

I have had some issues with jpm/npm modules though, maybe you're experiencing the same? Clearing it the generated folders and doing an npm update solved them for me.
Or maybe you're running into the first load issue again?

lidel · 2016-02-13T15:31:50Z

Removed node_modules and installed everything again via npm install.

My versions:

node 5.6.0
npm 3.6.0
jpm 1.0.5

I run local version via node_modules/jpm/bin/jpm -b $FIREFOX_BIN run
Still have the same problem: fs: is replaced with http: when redirect is enabled.

Are you running clear profile without any auto-reloading addons?

the8472 · 2016-02-13T15:46:58Z

node v5.5.0
npm 3.7.1
jpm 1.0.5

I have a dev profile which I use together with jpm post + extension auto installer.

lidel · 2016-02-13T15:52:32Z

Just to be sure it is something on my end,
please try without dev profile and other addons, just dry run with jpm -b $FIREFOX_BIN run.

the8472 · 2016-02-13T15:57:12Z

Ok, yeah, now I'm seeing it too sigh

Will fix.

fixes ipfs#48

the8472 · 2016-02-14T12:22:44Z

should work now.

support for fs: URIs without redirecting to http: - disabled by default - implements #48

lidel · 2016-02-14T17:09:20Z

Merged – thank you!

Let's follow this in related issues:

lidel · 2016-02-16T21:29:32Z

lib/request-fix.js

+catman.addCategoryEntry(in string aCategory, in string aEntry, in string aValue, in boolean aPersist, in boolean aReplace)
+*/
+
+function fixupRequest (e) {


@the8472 I am going over the backlog and trying to have a better understanding of this part of code. I see this is related to protocols.js#L121-L131 but can't infer full picture from code alone.

Could you elaborate what is the purpose of this fixup?

I'm not entirely sure if it's still necessary.

There were some cases with redirects coming from the server which needed the LOAD_REPLACE flag to be removed. If it is not removed the new channel's URI overrides originalURI which leads to a http URI in the address bar.

While implementing #72 I did some cleanup in c75a05d.

I did not notice any regressions, but would appreciate if you took a second look, perhaps I missed something.

the8472 mentioned this pull request Feb 11, 2016

WIP: rewriting ipfs uris to fs: prefix and linkifying inline urls #35

Closed

the8472 mentioned this pull request Feb 11, 2016

potential issues with https sites / https gateway #19

Closed

lidel reviewed Feb 11, 2016
View reviewed changes

lidel added kind/enhancement A net-new feature or improvement to an existing feature UX labels Feb 11, 2016

lidel added this to the v1.6.0 milestone Feb 11, 2016

lidel mentioned this pull request Feb 11, 2016

no-cookies, no-cache-validate channels for /ipfs/ paths? #72

Closed

the8472 force-pushed the fs-uris branch from 869bc27 to 7fcfc18 Compare February 12, 2016 20:33

the8472 added 6 commits February 14, 2016 13:04

support for fs: URIs without redirecting to http:

44c3a82

fixes ipfs#48

tests pass locally, not on travis. let's see if this helps

6804106

backup prefs once, restore from backup before tests

18cb543

protocol handlers emit http urls by default

fb7fb15

pacify eslint

20a53e2

canonical uris with private gateway enabled

0b08570

the8472 force-pushed the fs-uris branch from c15d1f0 to 0b08570 Compare February 14, 2016 12:08

lidel added a commit that referenced this pull request Feb 14, 2016

Merge pull request #70 from the8472/fs-uris

0640e96

support for fs: URIs without redirecting to http: - disabled by default - implements #48

lidel merged commit 0640e96 into ipfs:master Feb 14, 2016

lidel mentioned this pull request Feb 14, 2016

Keep IPFS URIs? #48

Closed

lidel reviewed Feb 16, 2016
View reviewed changes

lidel mentioned this pull request Mar 5, 2016

Some sites always redirect back to http:// #87

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support for fs: URIs without redirecting to http: #70

support for fs: URIs without redirecting to http: #70

the8472 commented Feb 11, 2016

the8472 commented Feb 11, 2016

lidel commented Feb 11, 2016

the8472 commented Feb 11, 2016

lidel Feb 11, 2016

lidel commented Feb 11, 2016

the8472 commented Feb 11, 2016

lidel commented Feb 11, 2016

the8472 commented Feb 12, 2016

lidel commented Feb 13, 2016

the8472 commented Feb 13, 2016

lidel commented Feb 13, 2016

the8472 commented Feb 13, 2016

lidel commented Feb 13, 2016

the8472 commented Feb 13, 2016

the8472 commented Feb 14, 2016

lidel commented Feb 14, 2016

lidel Feb 16, 2016

the8472 Feb 16, 2016

lidel Feb 18, 2016

support for fs: URIs without redirecting to http: #70

support for fs: URIs without redirecting to http: #70

Conversation

the8472 commented Feb 11, 2016

the8472 commented Feb 11, 2016

lidel commented Feb 11, 2016

the8472 commented Feb 11, 2016

lidel Feb 11, 2016

Choose a reason for hiding this comment

lidel commented Feb 11, 2016

the8472 commented Feb 11, 2016

lidel commented Feb 11, 2016

the8472 commented Feb 12, 2016

lidel commented Feb 13, 2016

the8472 commented Feb 13, 2016

lidel commented Feb 13, 2016

the8472 commented Feb 13, 2016

lidel commented Feb 13, 2016

the8472 commented Feb 13, 2016

the8472 commented Feb 14, 2016

lidel commented Feb 14, 2016

lidel Feb 16, 2016

Choose a reason for hiding this comment

the8472 Feb 16, 2016

Choose a reason for hiding this comment

lidel Feb 18, 2016

Choose a reason for hiding this comment