Skip to content

Commit

Permalink
fix: switch default URLs to 'localhost'
Browse files Browse the repository at this point in the history
- longer hostnames are truncated and no longer break status popup
- `127.0.0.1` is normalized to `localhost` when Preferences are saved
- IPFS resources on localhost are excluded from gateway redirect
  (this solves problems with multiple nodes running locally)
- closes #291
  • Loading branch information
lidel committed Dec 4, 2017
1 parent 1f48001 commit 27c6d76
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 10 deletions.
2 changes: 1 addition & 1 deletion add-on/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"manifest_version": 2,
"name": "__MSG_manifest_extensionName__",
"short_name": "__MSG_manifest_shortExtensionName__",
"version" : "2.1.0",
"version" : "2.1.1",

"description": "__MSG_manifest_extensionDescription__",
"homepage_url": "https://github.com/ipfs/ipfs-companion",
Expand Down
5 changes: 5 additions & 0 deletions add-on/src/lib/ipfs-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator) {
return
}

// skip all local requests
if (request.url.startsWith('http://127.0.0.1:') || request.url.startsWith('http://localhost:')) {
return
}

// poor-mans protocol handlers - https://github.com/ipfs/ipfs-companion/issues/164#issuecomment-328374052
if (state.catchUnhandledProtocols && mayContainUnhandledIpfsProtocol(request)) {
const fix = normalizedUnhandledIpfsProtocol(request, state.pubGwURLString)
Expand Down
13 changes: 11 additions & 2 deletions add-on/src/lib/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ const optionDefaults = Object.freeze({
preloadAtPublicGateway: true,
catchUnhandledProtocols: true,
displayNotifications: true,
customGatewayUrl: 'http://127.0.0.1:8080',
ipfsApiUrl: 'http://127.0.0.1:5001',
customGatewayUrl: 'http://localhost:8080',
ipfsApiUrl: 'http://localhost:5001',
ipfsApiPollMs: 3000
})

Expand Down Expand Up @@ -42,3 +42,12 @@ function storeMissingOptions (read, defaults, storage) {
}

exports.storeMissingOptions = storeMissingOptions

function normalizeGatewayURL (url) {
// https://github.com/ipfs/ipfs-companion/issues/291
return url
.replace('/127.0.0.1:', '/localhost:')
.replace('/gateway.ipfs.io', '/ipfs.io')

This comment has been minimized.

Copy link
@ivan386

ivan386 Dec 12, 2017

gateway.ipfs.io can be redirected to 127.0.0.1 in hosts file and continue work as gateway. If ipfs.io will be redirected to 127.0.0.1 it stop work as gateway. Use gateway.ipfs.io.

This comment has been minimized.

Copy link
@lidel

lidel Dec 14, 2017

Author Member

Just FYI I removed normalization of gateway.ipfs.io as a part of #328.
I think we can reintroduce it after we move from ipfs.io to something else.

}

exports.normalizeGatewayURL = normalizeGatewayURL
6 changes: 4 additions & 2 deletions add-on/src/options/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
/* eslint-env browser, webextensions */

const browser = require('webextension-polyfill')
const { optionDefaults } = require('../lib/options')
const { optionDefaults, normalizeGatewayURL } = require('../lib/options')
const translateDataAttrs = require('../lib/data-i18n')

translateDataAttrs()
Expand All @@ -18,9 +18,11 @@ async function saveOption (name) {
switch (element.type) {
case 'text':
case 'number':
case 'url':
change[name] = element.value
break
case 'url':
change[name] = normalizeGatewayURL(element.value)
break
case 'textarea':
// normalize input into a string of entries separated by a single space
change[name] = element.value.replace(/[\r\n,;]+/g, ' ').replace(/ +(?= )/g, '').trim()
Expand Down
2 changes: 1 addition & 1 deletion add-on/src/popup/browser-action/gateway-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = function gatewayStatus ({
<ul class="list mv3 ph3">
<li class="flex mb1">
<span class="w-50 f6 ttu">${browser.i18n.getMessage('panel_statusGatewayAddress')}</span>
<span class="w-50 f6 ttu tr">${gatewayAddress == null ? 'unknown' : gatewayAddress}</span>
<span class="w-50 f6 truncate tr">${gatewayAddress == null ? 'unknown' : gatewayAddress}</span>

This comment has been minimized.

Copy link
@lidel

lidel Dec 4, 2017

Author Member

@alanshaw I had to remove ttu because after the change from 127.0.0.1 to localhost the HTTP GATEWAY did not fit anymore. Truncating URL fixed the layout, but a default that is always truncated looks bad, so I ended up with lowercase URL, which fits (at least on my box).

Feel free to change it, if you find a better way to handle long gateway URLs :)

</li>
<li class="flex mb1">
<span class="w-50 f6 ttu">${browser.i18n.getMessage('panel_statusGatewayVersion')}</span>
Expand Down
24 changes: 20 additions & 4 deletions test/functional/lib/ipfs-request.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('modifyRequest', function () {
peerCount: 1,
redirect: true,
catchUnhandledProtocols: true,
gwURLString: 'http://127.0.0.1:8080'
gwURLString: 'http://localhost:8080'
})
const getState = () => state
dnsLink = createDnsLink(getState)
Expand All @@ -36,7 +36,7 @@ describe('modifyRequest', function () {
describe('request for a path matching /ipfs/{CIDv0}', function () {
it('should be served from custom gateway if redirect is enabled', function () {
const request = url2request('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
expect(modifyRequest(request).redirectUrl).to.equal('http://127.0.0.1:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
expect(modifyRequest(request).redirectUrl).to.equal('http://localhost:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
})
it('should be left untouched if redirect is disabled', function () {
state.redirect = false
Expand All @@ -57,12 +57,12 @@ describe('modifyRequest', function () {
dnsLink.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/Qmazvovg6Sic3m9igZMKoAPjkiVZsvbWWc8ZvgjjK1qMss')
// pretend API is online and we can do dns lookups with it
state.peerCount = 1
expect(modifyRequest(request).redirectUrl).to.equal('http://127.0.0.1:8080/ipns/ipfs.git.sexy/index.html?argTest#hashTest')
expect(modifyRequest(request).redirectUrl).to.equal('http://localhost:8080/ipns/ipfs.git.sexy/index.html?argTest#hashTest')
})
it('should be served from custom gateway if {path} starts with a valid CID', function () {
const request = url2request('https://ipfs.io/ipns/QmSWnBwMKZ28tcgMFdihD8XS7p6QzdRSGf71cCybaETSsU/index.html?argTest#hashTest')
dnsLink.readDnslinkFromTxtRecord = sinon.stub().returns(false)
expect(modifyRequest(request).redirectUrl).to.equal('http://127.0.0.1:8080/ipns/QmSWnBwMKZ28tcgMFdihD8XS7p6QzdRSGf71cCybaETSsU/index.html?argTest#hashTest')
expect(modifyRequest(request).redirectUrl).to.equal('http://localhost:8080/ipns/QmSWnBwMKZ28tcgMFdihD8XS7p6QzdRSGf71cCybaETSsU/index.html?argTest#hashTest')
})
it('should be left untouched if redirect is disabled', function () {
state.redirect = false
Expand Down Expand Up @@ -216,6 +216,22 @@ describe('modifyRequest', function () {
})
})

describe('request for IPFS path at a localhost', function () {
// we do not touch local requests, as it may interfere with other nodes running at the same machine
// or could produce false-positives such as redirection from 127.0.0.1:5001/ipfs/path to localhost:8080/ipfs/path
it('should be left untouched if 127.0.0.1 is used', function () {
state.redirect = false
const request = url2request('http://127.0.0.1:5001/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ/')
expect(modifyRequest(request)).to.equal(undefined)
})
it('should be left untouched if localhost is used', function () {
// https://github.com/ipfs/ipfs-companion/issues/291
state.redirect = false
const request = url2request('http://localhost:5001/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ/')
expect(modifyRequest(request)).to.equal(undefined)
})
})

after(() => {
delete global.URL
})
Expand Down

0 comments on commit 27c6d76

Please sign in to comment.