Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

[WIP] feat: dns support for WS #47

Merged
merged 7 commits into from
Mar 23, 2017
Merged

[WIP] feat: dns support for WS #47

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
569eab1
feat: dns support for WS
Feb 19, 2017
7ea8841
chore: update aegir
daviddias Mar 21, 2017
ee32dab
chore: update contributors
daviddias Mar 21, 2017
3e113be
chore: release version v0.9.3
daviddias Mar 21, 2017
029c917
chore: release version v0.9.4
daviddias Mar 21, 2017
9a48319
feat: dns support for WS
Feb 19, 2017
63a9fe7
feat: add more dns tests and refactored existing tests
Mar 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,4 @@
"Greenkeeper <[email protected]>",
"Richard Littauer <[email protected]>"
]
}
}
91 changes: 55 additions & 36 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,57 +4,76 @@ const connect = require('pull-ws/client')
const mafmt = require('mafmt')
const includes = require('lodash.includes')
const Connection = require('interface-connection').Connection
const multiaddr = require('multiaddr')

const debug = require('debug')
const log = debug('libp2p:websockets:dialer')

const createListener = require('./listener')

module.exports = class WebSockets {
dial (ma, options, callback) {
if (typeof options === 'function') {
callback = options
options = {}
}
function maToUrl (ma) {
const maStrSplit = ma.toString().split('/')
const proto = ma.protos()[2].name

if (!callback) {
callback = () => {}
}
if (!(proto === 'ws' || proto === 'wss')) {
throw new Error('invalid multiaddr' + ma.toString())
}

const maOpts = ma.toOptions()
let url = ma.protos()[2].name + '://' + maStrSplit[2]

const url = `ws://${maOpts.host}:${maOpts.port}`
log('dialing %s', url)
const socket = connect(url, {
binary: true,
onConnect: () => callback()
})
if (!multiaddr.isName(ma)) {
url += ':' + maStrSplit[4]
}

const conn = new Connection(socket)
conn.getObservedAddrs = (cb) => cb(null, [ma])
conn.close = (cb) => socket.close(cb)
return url
}

return conn
}
module.exports =
class WebSockets {
dial (ma, options, callback) {
if (typeof options === 'function') {
callback = options
options = {}
}

createListener (options, handler) {
if (typeof options === 'function') {
handler = options
options = {}
if (!callback) {
callback = () => {
}
}

const url = maToUrl(ma)
log('dialing %s', url)
const socket = connect(url, {
binary: true,
onConnect: () => callback()
})

const conn = new Connection(socket)
conn.getObservedAddrs = (cb) => cb(null, [ma])
conn.close = (cb) => socket.close(cb)

return conn
}

return createListener(options, handler)
}
createListener (options, handler) {
if (typeof options === 'function') {
handler = options
options = {}
}

filter (multiaddrs) {
if (!Array.isArray(multiaddrs)) {
multiaddrs = [multiaddrs]
return createListener(options, handler)
}

return multiaddrs.filter((ma) => {
if (includes(ma.protoNames(), 'ipfs')) {
ma = ma.decapsulate('ipfs')
filter (multiaddrs) {
if (!Array.isArray(multiaddrs)) {
multiaddrs = [multiaddrs]
}
return mafmt.WebSockets.matches(ma)
})
}

return multiaddrs.filter((ma) => {
if (includes(ma.protoNames(), 'ipfs')) {
ma = ma.decapsulate('ipfs')
}
return mafmt.WebSockets.matches(ma)
})
}
}
5 changes: 3 additions & 2 deletions test/compliance.node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ describe('compliance', () => {
let ws = new Ws()
const addrs = [
multiaddr('/ip4/127.0.0.1/tcp/9091/ws'),
multiaddr('/ip4/127.0.0.1/tcp/9092/ws'),
multiaddr('/ip4/127.0.0.1/tcp/9093/ws')
multiaddr('/ip4/127.0.0.1/tcp/9092/wss'),
multiaddr('/dns4/awesome-dns-server.com/tcp/9092/ws'),
multiaddr('/dns4/awesome-dns-server.com/tcp/9092/wss')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail as these hosts don't exist.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, those should fail... They are being intentionally filtered out, according to the test.

Not the clearest test case, some refactoring might make sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything should be passing right now. I also refactored the existing tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: compliance addresses, you are right, i was talking about something else. I'll make those point to existing hosts.

]
cb(null, ws, addrs)
},
Expand Down
32 changes: 22 additions & 10 deletions test/node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,25 @@ describe('listen', () => {
})

it('listen, check for callback', (done) => {
const listener = ws.createListener((conn) => {})
const listener = ws.createListener((conn) => {
})
listener.listen(ma, () => {
listener.close(done)
})
})

it('listen, check for listening event', (done) => {
const listener = ws.createListener((conn) => {})
const listener = ws.createListener((conn) => {
})
listener.on('listening', () => {
listener.close(done)
})
listener.listen(ma)
})

it('listen, check for the close event', (done) => {
const listener = ws.createListener((conn) => {})
const listener = ws.createListener((conn) => {
})
listener.on('listening', () => {
listener.on('close', done)
listener.close()
Expand All @@ -51,7 +54,8 @@ describe('listen', () => {

it('listen on addr with /ipfs/QmHASH', (done) => {
const ma = multiaddr('/ip4/127.0.0.1/tcp/9090/ws/ipfs/Qmb6owHp6eaWArVbcJJbQSyifyJBttMMjYV76N2hMbf5Vw')
const listener = ws.createListener((conn) => {})
const listener = ws.createListener((conn) => {
})
listener.listen(ma, () => {
listener.close(done)
})
Expand All @@ -74,7 +78,8 @@ describe('listen', () => {
})

it('getAddrs', (done) => {
const listener = ws.createListener((conn) => {})
const listener = ws.createListener((conn) => {
})
listener.listen(ma, () => {
listener.getAddrs((err, addrs) => {
expect(err).to.not.exist
Expand All @@ -100,7 +105,8 @@ describe('listen', () => {
it('getAddrs preserves IPFS Id', (done) => {
const ma = multiaddr('/ip4/127.0.0.1/tcp/9090/ws/ipfs/Qmb6owHp6eaWArVbcJJbQSyifyJBttMMjYV76N2hMbf5Vw')

const listener = ws.createListener((conn) => {})
const listener = ws.createListener((conn) => {
})
listener.listen(ma, () => {
listener.getAddrs((err, addrs) => {
expect(err).to.not.exist
Expand Down Expand Up @@ -179,11 +185,15 @@ describe('filter addrs', () => {
const mh2 = multiaddr('/ip4/127.0.0.1/udp/9090')
const mh3 = multiaddr('/ip4/127.0.0.1/tcp/9090/ws')
const mh4 = multiaddr('/ip4/127.0.0.1/tcp/9090/ws/ipfs/Qmb6owHp6eaWArVbcJJbQSyifyJBttMMjYV76N2hMbf5Vw')
const mh5 = multiaddr('/dns/ipfs.io/ws')
const mh6 = multiaddr('/dns/ipfs.io/ws/ipfs/Qmb6owHp6eaWArVbcJJbQSyifyJBttMMjYV76N2hMbf5Vw')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to also account for addresses like

/dns/ipfs.io/tcp/80/ws/ipfs/Qmb6owHp6eaWArVbcJJbQSyifyJBttMMjYV76N2hMbf5Vw

otherwise you can't specify the port

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes these need /tcp/80/wss or /tcp/443/wss, and it needs to be /dns4 or /dns6

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah :) I'll add those.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, actually, default for just /ws is port 80 and /wss is port 443, that's it.

@dryajov still up to finish this? It would be sweet for the 0.23 release! :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diasdavid yep, I'll get this in.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome @dryajov :)

Copy link
Member Author

@dryajov dryajov Mar 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diasdavid @lgierth I think we're talking about having both /tcp/80/wss and /tcp/443/ws for non standard ports? They are both valid cases and should be supported, right?

Copy link
Member Author

@dryajov dryajov Mar 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out that we apparently don't support ports in DNS address in js-mafmt, here is a PR that addresses it - multiformats/js-mafmt#15. I'm not entirely sure its the right approach here, but it seems to work.

With this PR in mafmt, I have most of the tests passing here, I should be able to get them all passing soon. I'll push as soon as I have everything working.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dryajov published mafmt for you :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


const valid = ws.filter([mh1, mh2, mh3, mh4])
expect(valid.length).to.equal(2)
const valid = ws.filter([mh1, mh2, mh3, mh4, mh5, mh6])
expect(valid.length).to.equal(4)
expect(valid[0]).to.deep.equal(mh3)
expect(valid[1]).to.deep.equal(mh4)
expect(valid[2]).to.deep.equal(mh5)
expect(valid[3]).to.deep.equal(mh6)
done()
})

Expand Down Expand Up @@ -311,6 +321,8 @@ describe('valid Connection', () => {
})

describe.skip('turbolence', () => {
it('dialer - emits error on the other end is terminated abruptly', (done) => {})
it('listener - emits error on the other end is terminated abruptly', (done) => {})
it('dialer - emits error on the other end is terminated abruptly', (done) => {
})
it('listener - emits error on the other end is terminated abruptly', (done) => {
})
})