From 6b0440c0502be4f44a1f33433e1e0e5e842ef294 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 16 Oct 2014 11:35:54 +0200 Subject: [PATCH 1/9] versionExchange: first stab in the dark --- net/swarm/conn.go | 56 ++++++++++++++++++++++++++++++++++++++++ net/version/Makefile | 8 ++++++ net/version/semver.pb.go | 56 ++++++++++++++++++++++++++++++++++++++++ net/version/semver.proto | 8 ++++++ net/version/version.go | 32 +++++++++++++++++++++++ 5 files changed, 160 insertions(+) create mode 100644 net/version/Makefile create mode 100644 net/version/semver.pb.go create mode 100644 net/version/semver.proto create mode 100644 net/version/version.go diff --git a/net/swarm/conn.go b/net/swarm/conn.go index 7d5c47b5c..3bbe665e4 100644 --- a/net/swarm/conn.go +++ b/net/swarm/conn.go @@ -7,7 +7,9 @@ import ( spipe "github.com/jbenet/go-ipfs/crypto/spipe" conn "github.com/jbenet/go-ipfs/net/conn" msg "github.com/jbenet/go-ipfs/net/message" + version "github.com/jbenet/go-ipfs/net/version" + proto "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/goprotobuf/proto" ma "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr" manet "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr/net" ) @@ -109,6 +111,10 @@ func (s *Swarm) connSetup(c *conn.Conn) error { // add address of connection to Peer. Maybe it should happen in connSecure. c.Peer.AddAddress(c.Addr) + if err := s.connVersionExchange(c); err != nil { + return fmt.Errorf("Conn version exchange error: %v", err) + } + // add to conns s.connsLock.Lock() if _, ok := s.conns[c.Peer.Key()]; ok { @@ -152,6 +158,56 @@ func (s *Swarm) connSecure(c *conn.Conn) error { return nil } +// connVersionExchange exchanges local and remote versions and compares them +// closes remote and returns an error in case of major difference +func (s *Swarm) connVersionExchange(remote *conn.Conn) error { + var remoteVersion, myVersion *version.SemVer + myVersion = version.Current() + + myVersionMsg, err := msg.FromObject(s.local, myVersion) + if err != nil { + return fmt.Errorf("connVersionExchange: could not prepare local version: %q", err) + } + + var gotTheirs, sendMine bool + for { + if gotTheirs && sendMine { + break + } + + select { + case <-s.ctx.Done(): + // close Conn. + remote.Close() + return nil // BUG(cryptix): should this be an error? + + case <-remote.Closed: + return errors.New("remote closed connection during version exchange") + + case remote.Secure.Out <- myVersionMsg.Data(): + log.Debug("[peer: %s] Send my version(%s) to %s", s.local, myVersion, remote.Peer) + sendMine = true + + case data, ok := <-remote.Secure.In: + if !ok { + return fmt.Errorf("Error retrieving from conn: %v", remote.Peer) + } + + log.Debug("[peer: %s] Received message [from = %s]", s.local, remote.Peer) + + remoteVersion = new(version.SemVer) + err = proto.Unmarshal(data, remoteVersion) + if err != nil { + s.Close() + return fmt.Errorf("connSetup: could not decode remote version: %q", err) + } + gotTheirs = true + } + } + + return errors.New("not yet") +} + // Handles the unwrapping + sending of messages to the right connection. func (s *Swarm) fanOut() { for { diff --git a/net/version/Makefile b/net/version/Makefile new file mode 100644 index 000000000..85f8c2705 --- /dev/null +++ b/net/version/Makefile @@ -0,0 +1,8 @@ + +all: semver.pb.go + +semver.pb.go: semver.proto + protoc --gogo_out=. --proto_path=../../../../../:/usr/include/google:. $< + +clean: + rm semver.pb.go diff --git a/net/version/semver.pb.go b/net/version/semver.pb.go new file mode 100644 index 000000000..7f55ecb6e --- /dev/null +++ b/net/version/semver.pb.go @@ -0,0 +1,56 @@ +// Code generated by protoc-gen-gogo. +// source: semver.proto +// DO NOT EDIT! + +/* +Package version is a generated protocol buffer package. + +It is generated from these files: + semver.proto + +It has these top-level messages: + SemVer +*/ +package version + +import proto "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/gogoprotobuf/proto" +import math "math" + +// Reference imports to suppress errors if they are not otherwise used. +var _ = proto.Marshal +var _ = math.Inf + +type SemVer struct { + Major *int64 `protobuf:"varint,1,opt,name=major" json:"major,omitempty"` + Minor *int64 `protobuf:"varint,2,opt,name=minor" json:"minor,omitempty"` + Patch *int64 `protobuf:"varint,3,opt,name=patch" json:"patch,omitempty"` + XXX_unrecognized []byte `json:"-"` +} + +func (m *SemVer) Reset() { *m = SemVer{} } +func (m *SemVer) String() string { return proto.CompactTextString(m) } +func (*SemVer) ProtoMessage() {} + +func (m *SemVer) GetMajor() int64 { + if m != nil && m.Major != nil { + return *m.Major + } + return 0 +} + +func (m *SemVer) GetMinor() int64 { + if m != nil && m.Minor != nil { + return *m.Minor + } + return 0 +} + +func (m *SemVer) GetPatch() int64 { + if m != nil && m.Patch != nil { + return *m.Patch + } + return 0 +} + +func init() { +} diff --git a/net/version/semver.proto b/net/version/semver.proto new file mode 100644 index 000000000..20d5690e7 --- /dev/null +++ b/net/version/semver.proto @@ -0,0 +1,8 @@ +package version; + +message SemVer { + optional int64 major = 1; + optional int64 minor = 2; + optional int64 patch = 3; + // BUG(cryptix): do we need PreRelease and Metadata too? +} diff --git a/net/version/version.go b/net/version/version.go new file mode 100644 index 000000000..bb1a67b13 --- /dev/null +++ b/net/version/version.go @@ -0,0 +1,32 @@ +package version + +import semver "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/coreos/go-semver/semver" + +var currentVersion = semver.Version{ + Major: 0, + Minor: 1, + Patch: 0, +} + +// Current returns the current protocol version as a protobuf message +func Current() *SemVer { + return toPBSemVer(currentVersion) +} + +// toPBSemVar converts a coreos/semver to our protobuf SemVer +func toPBSemVer(in semver.Version) (out *SemVer) { + return &SemVer{ + Major: &in.Major, + Minor: &in.Minor, + Patch: &in.Patch, + } +} + +// toPBSemVar converts our protobuf SemVer to a coreos/semver +func fromPBSemVer(in SemVer) *semver.Version { + return &semver.Version{ + Major: *in.Major, + Minor: *in.Minor, + Patch: *in.Patch, + } +} From 0075a352da6af9c16af3ce92fb099929405c10df Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 16 Oct 2014 12:03:35 +0200 Subject: [PATCH 2/9] only send local version once --- net/swarm/conn.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/net/swarm/conn.go b/net/swarm/conn.go index 3bbe665e4..f05c20dcb 100644 --- a/net/swarm/conn.go +++ b/net/swarm/conn.go @@ -169,6 +169,10 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { return fmt.Errorf("connVersionExchange: could not prepare local version: %q", err) } + // buffered channel to send our version just once + outBuf := make(chan []byte, 1) + outBuf <- myVersionMsg.Data() + var gotTheirs, sendMine bool for { if gotTheirs && sendMine { @@ -184,9 +188,13 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { case <-remote.Closed: return errors.New("remote closed connection during version exchange") - case remote.Secure.Out <- myVersionMsg.Data(): - log.Debug("[peer: %s] Send my version(%s) to %s", s.local, myVersion, remote.Peer) - sendMine = true + case our, ok := <-outBuf: + if ok { + remote.Secure.Out <- our + sendMine = true + close(outBuf) // only send local version once + log.Debug("[peer: %s] Send my version(%s) to %s", s.local, myVersion, remote.Peer) + } case data, ok := <-remote.Secure.In: if !ok { @@ -202,6 +210,8 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { return fmt.Errorf("connSetup: could not decode remote version: %q", err) } gotTheirs = true + + // BUG(cryptix): could add another case here to trigger resending our version } } From 2a5b3eaa710b7ec7c9c52cefed0d222d6800e9dc Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 16 Oct 2014 12:09:43 +0200 Subject: [PATCH 3/9] compare versions for compatibility --- net/swarm/conn.go | 8 +++++++- net/version/version.go | 14 +++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/net/swarm/conn.go b/net/swarm/conn.go index f05c20dcb..bf32e0949 100644 --- a/net/swarm/conn.go +++ b/net/swarm/conn.go @@ -164,6 +164,7 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { var remoteVersion, myVersion *version.SemVer myVersion = version.Current() + // BUG(cryptix): do we need to use a NetMessage here? myVersionMsg, err := msg.FromObject(s.local, myVersion) if err != nil { return fmt.Errorf("connVersionExchange: could not prepare local version: %q", err) @@ -215,7 +216,12 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { } } - return errors.New("not yet") + if !version.Compatible(myVersion, remoteVersion) { + remote.Close() + return errors.New("protocol missmatch") + } + + return nil } // Handles the unwrapping + sending of messages to the right connection. diff --git a/net/version/version.go b/net/version/version.go index bb1a67b13..be8e194f5 100644 --- a/net/version/version.go +++ b/net/version/version.go @@ -2,7 +2,7 @@ package version import semver "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/coreos/go-semver/semver" -var currentVersion = semver.Version{ +var currentVersion = &semver.Version{ Major: 0, Minor: 1, Patch: 0, @@ -13,8 +13,16 @@ func Current() *SemVer { return toPBSemVer(currentVersion) } +// Compatible checks wether two versions are compatible +func Compatible(a, b *SemVer) bool { + aConv := fromPBSemVer(a) + bConv := fromPBSemVer(b) + return aConv.LessThan(*bConv) +} + // toPBSemVar converts a coreos/semver to our protobuf SemVer -func toPBSemVer(in semver.Version) (out *SemVer) { +func toPBSemVer(in *semver.Version) (out *SemVer) { + return &SemVer{ Major: &in.Major, Minor: &in.Minor, @@ -23,7 +31,7 @@ func toPBSemVer(in semver.Version) (out *SemVer) { } // toPBSemVar converts our protobuf SemVer to a coreos/semver -func fromPBSemVer(in SemVer) *semver.Version { +func fromPBSemVer(in *SemVer) *semver.Version { return &semver.Version{ Major: *in.Major, Minor: *in.Minor, From 78eb349360d5b27d34cca8bdcedd5182f28bc0d6 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 16 Oct 2014 12:29:33 +0200 Subject: [PATCH 4/9] fixed Compatible and added a small test case --- net/swarm/conn.go | 8 ++++---- net/version/version.go | 12 +++++------- net/version/version_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 11 deletions(-) create mode 100644 net/version/version_test.go diff --git a/net/swarm/conn.go b/net/swarm/conn.go index bf32e0949..4e94fd05c 100644 --- a/net/swarm/conn.go +++ b/net/swarm/conn.go @@ -194,7 +194,7 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { remote.Secure.Out <- our sendMine = true close(outBuf) // only send local version once - log.Debug("[peer: %s] Send my version(%s) to %s", s.local, myVersion, remote.Peer) + log.Debug("Send my version(%s) [to = %s]", myVersion, remote.Peer) } case data, ok := <-remote.Secure.In: @@ -202,8 +202,6 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { return fmt.Errorf("Error retrieving from conn: %v", remote.Peer) } - log.Debug("[peer: %s] Received message [from = %s]", s.local, remote.Peer) - remoteVersion = new(version.SemVer) err = proto.Unmarshal(data, remoteVersion) if err != nil { @@ -211,16 +209,18 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { return fmt.Errorf("connSetup: could not decode remote version: %q", err) } gotTheirs = true + log.Debug("Received remote version(%s) [from = %s]", remoteVersion, remote.Peer) // BUG(cryptix): could add another case here to trigger resending our version } } - if !version.Compatible(myVersion, remoteVersion) { + if !version.Compatible(myVersion.Convert(), remoteVersion.Convert()) { remote.Close() return errors.New("protocol missmatch") } + log.Debug("[peer: %s] Version compatible", remote.Peer) return nil } diff --git a/net/version/version.go b/net/version/version.go index be8e194f5..7b9926aed 100644 --- a/net/version/version.go +++ b/net/version/version.go @@ -14,10 +14,8 @@ func Current() *SemVer { } // Compatible checks wether two versions are compatible -func Compatible(a, b *SemVer) bool { - aConv := fromPBSemVer(a) - bConv := fromPBSemVer(b) - return aConv.LessThan(*bConv) +func Compatible(a, b semver.Version) bool { + return !a.LessThan(b) } // toPBSemVar converts a coreos/semver to our protobuf SemVer @@ -30,9 +28,9 @@ func toPBSemVer(in *semver.Version) (out *SemVer) { } } -// toPBSemVar converts our protobuf SemVer to a coreos/semver -func fromPBSemVer(in *SemVer) *semver.Version { - return &semver.Version{ +// Convert our protobuf SemVer to a coreos/semver +func (in SemVer) Convert() semver.Version { + return semver.Version{ Major: *in.Major, Minor: *in.Minor, Patch: *in.Patch, diff --git a/net/version/version_test.go b/net/version/version_test.go new file mode 100644 index 000000000..97b60d9f8 --- /dev/null +++ b/net/version/version_test.go @@ -0,0 +1,25 @@ +package version + +import ( + "testing" + + semver "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/coreos/go-semver/semver" +) + +func TestCompatible(t *testing.T) { + tcases := []struct { + a, b semver.Version + expected bool + }{ + {semver.Version{Major: 0}, semver.Version{Major: 0}, true}, + {semver.Version{Major: 1}, semver.Version{Major: 0}, true}, + {semver.Version{Major: 1}, semver.Version{Major: 1}, true}, + {semver.Version{Major: 0}, semver.Version{Major: 1}, false}, + } + + for i, tcase := range tcases { + if Compatible(tcase.a, tcase.b) != tcase.expected { + t.Fatalf("case[%d] failed", i) + } + } +} From 9dc58639faec99ffa3ea6d5d4c4ef28bd000cd7e Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 16 Oct 2014 13:00:43 +0200 Subject: [PATCH 5/9] dont need coreos/semver yet --- net/swarm/conn.go | 2 +- net/version/version.go | 39 ++++++++++++------------------------- net/version/version_test.go | 16 ++++++--------- 3 files changed, 19 insertions(+), 38 deletions(-) diff --git a/net/swarm/conn.go b/net/swarm/conn.go index 4e94fd05c..023be37b0 100644 --- a/net/swarm/conn.go +++ b/net/swarm/conn.go @@ -215,7 +215,7 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { } } - if !version.Compatible(myVersion.Convert(), remoteVersion.Convert()) { + if !version.Compatible(myVersion, remoteVersion) { remote.Close() return errors.New("protocol missmatch") } diff --git a/net/version/version.go b/net/version/version.go index 7b9926aed..db9d3a12a 100644 --- a/net/version/version.go +++ b/net/version/version.go @@ -1,38 +1,23 @@ package version -import semver "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/coreos/go-semver/semver" - -var currentVersion = &semver.Version{ - Major: 0, - Minor: 1, - Patch: 0, -} +// currentVersion holds the current protocol version for a client running this code +var currentVersion = NewSemVer(0, 0, 1) // Current returns the current protocol version as a protobuf message func Current() *SemVer { - return toPBSemVer(currentVersion) + return currentVersion } // Compatible checks wether two versions are compatible -func Compatible(a, b semver.Version) bool { - return !a.LessThan(b) +func Compatible(a, b *SemVer) bool { + return *a.Major == *b.Major // protobuf fields are pointers } -// toPBSemVar converts a coreos/semver to our protobuf SemVer -func toPBSemVer(in *semver.Version) (out *SemVer) { - - return &SemVer{ - Major: &in.Major, - Minor: &in.Minor, - Patch: &in.Patch, - } -} - -// Convert our protobuf SemVer to a coreos/semver -func (in SemVer) Convert() semver.Version { - return semver.Version{ - Major: *in.Major, - Minor: *in.Minor, - Patch: *in.Patch, - } +// NewSemVer constructs a new protobuf SemVer +func NewSemVer(major, minor, patch int64) *SemVer { + s := new(SemVer) + s.Major = &major + s.Minor = &minor + s.Patch = &patch + return s } diff --git a/net/version/version_test.go b/net/version/version_test.go index 97b60d9f8..c56334f22 100644 --- a/net/version/version_test.go +++ b/net/version/version_test.go @@ -1,20 +1,16 @@ package version -import ( - "testing" - - semver "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/coreos/go-semver/semver" -) +import "testing" func TestCompatible(t *testing.T) { tcases := []struct { - a, b semver.Version + a, b *SemVer expected bool }{ - {semver.Version{Major: 0}, semver.Version{Major: 0}, true}, - {semver.Version{Major: 1}, semver.Version{Major: 0}, true}, - {semver.Version{Major: 1}, semver.Version{Major: 1}, true}, - {semver.Version{Major: 0}, semver.Version{Major: 1}, false}, + {NewSemVer(0, 0, 0), NewSemVer(0, 0, 0), true}, + {NewSemVer(0, 0, 0), NewSemVer(1, 0, 0), false}, + {NewSemVer(1, 0, 0), NewSemVer(0, 0, 0), false}, + {NewSemVer(1, 0, 0), NewSemVer(1, 0, 0), true}, } for i, tcase := range tcases { From eab1a890a6044a7a23da48cc285192a1cb5ff277 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 16 Oct 2014 13:03:47 +0200 Subject: [PATCH 6/9] don't use my protobuf path --- net/version/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/version/Makefile b/net/version/Makefile index 85f8c2705..4530e3644 100644 --- a/net/version/Makefile +++ b/net/version/Makefile @@ -2,7 +2,7 @@ all: semver.pb.go semver.pb.go: semver.proto - protoc --gogo_out=. --proto_path=../../../../../:/usr/include/google:. $< + protoc --gogo_out=. --proto_path=../../../../../:/usr/local/opt/protobuf/include:. $< clean: rm semver.pb.go From 6f8a89cd8d4d6066d59a901dbdd5f54dff3dcfc7 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 16 Oct 2014 13:37:32 +0200 Subject: [PATCH 7/9] renamed version pkg to handshake --- net/{version => handshake}/Makefile | 0 net/{version => handshake}/semver.pb.go | 6 +++--- net/{version => handshake}/semver.proto | 2 +- net/{version => handshake}/version.go | 2 +- net/{version => handshake}/version_test.go | 2 +- net/swarm/conn.go | 10 +++++----- 6 files changed, 11 insertions(+), 11 deletions(-) rename net/{version => handshake}/Makefile (100%) rename net/{version => handshake}/semver.pb.go (86%) rename net/{version => handshake}/semver.proto (89%) rename net/{version => handshake}/version.go (97%) rename net/{version => handshake}/version_test.go (96%) diff --git a/net/version/Makefile b/net/handshake/Makefile similarity index 100% rename from net/version/Makefile rename to net/handshake/Makefile diff --git a/net/version/semver.pb.go b/net/handshake/semver.pb.go similarity index 86% rename from net/version/semver.pb.go rename to net/handshake/semver.pb.go index 7f55ecb6e..e2557fbae 100644 --- a/net/version/semver.pb.go +++ b/net/handshake/semver.pb.go @@ -3,7 +3,7 @@ // DO NOT EDIT! /* -Package version is a generated protocol buffer package. +Package handshake is a generated protocol buffer package. It is generated from these files: semver.proto @@ -11,9 +11,9 @@ It is generated from these files: It has these top-level messages: SemVer */ -package version +package handshake -import proto "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/gogoprotobuf/proto" +import proto "code.google.com/p/gogoprotobuf/proto" import math "math" // Reference imports to suppress errors if they are not otherwise used. diff --git a/net/version/semver.proto b/net/handshake/semver.proto similarity index 89% rename from net/version/semver.proto rename to net/handshake/semver.proto index 20d5690e7..8069830e0 100644 --- a/net/version/semver.proto +++ b/net/handshake/semver.proto @@ -1,4 +1,4 @@ -package version; +package handshake; message SemVer { optional int64 major = 1; diff --git a/net/version/version.go b/net/handshake/version.go similarity index 97% rename from net/version/version.go rename to net/handshake/version.go index db9d3a12a..fdf027aab 100644 --- a/net/version/version.go +++ b/net/handshake/version.go @@ -1,4 +1,4 @@ -package version +package handshake // currentVersion holds the current protocol version for a client running this code var currentVersion = NewSemVer(0, 0, 1) diff --git a/net/version/version_test.go b/net/handshake/version_test.go similarity index 96% rename from net/version/version_test.go rename to net/handshake/version_test.go index c56334f22..06d07a49b 100644 --- a/net/version/version_test.go +++ b/net/handshake/version_test.go @@ -1,4 +1,4 @@ -package version +package handshake import "testing" diff --git a/net/swarm/conn.go b/net/swarm/conn.go index 023be37b0..3a6287440 100644 --- a/net/swarm/conn.go +++ b/net/swarm/conn.go @@ -6,8 +6,8 @@ import ( spipe "github.com/jbenet/go-ipfs/crypto/spipe" conn "github.com/jbenet/go-ipfs/net/conn" + handshake "github.com/jbenet/go-ipfs/net/handshake" msg "github.com/jbenet/go-ipfs/net/message" - version "github.com/jbenet/go-ipfs/net/version" proto "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/goprotobuf/proto" ma "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr" @@ -161,8 +161,8 @@ func (s *Swarm) connSecure(c *conn.Conn) error { // connVersionExchange exchanges local and remote versions and compares them // closes remote and returns an error in case of major difference func (s *Swarm) connVersionExchange(remote *conn.Conn) error { - var remoteVersion, myVersion *version.SemVer - myVersion = version.Current() + var remoteVersion, myVersion *handshake.SemVer + myVersion = handshake.Current() // BUG(cryptix): do we need to use a NetMessage here? myVersionMsg, err := msg.FromObject(s.local, myVersion) @@ -202,7 +202,7 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { return fmt.Errorf("Error retrieving from conn: %v", remote.Peer) } - remoteVersion = new(version.SemVer) + remoteVersion = new(handshake.SemVer) err = proto.Unmarshal(data, remoteVersion) if err != nil { s.Close() @@ -215,7 +215,7 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { } } - if !version.Compatible(myVersion, remoteVersion) { + if !handshake.Compatible(myVersion, remoteVersion) { remote.Close() return errors.New("protocol missmatch") } From 67e04f0d2950fe0498ee1447735e839a2e33b1d8 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 16 Oct 2014 13:57:45 +0200 Subject: [PATCH 8/9] changed message from SemVer to Handshake1 --- net/handshake/semver.pb.go | 45 +++++++++++++---------------- net/handshake/semver.proto | 14 +++++---- net/handshake/version.go | 53 +++++++++++++++++++++++++++-------- net/handshake/version_test.go | 16 ++++++----- net/swarm/conn.go | 2 +- 5 files changed, 81 insertions(+), 49 deletions(-) diff --git a/net/handshake/semver.pb.go b/net/handshake/semver.pb.go index e2557fbae..e1a5fb5d9 100644 --- a/net/handshake/semver.pb.go +++ b/net/handshake/semver.pb.go @@ -9,47 +9,42 @@ It is generated from these files: semver.proto It has these top-level messages: - SemVer + Handshake1 */ package handshake -import proto "code.google.com/p/gogoprotobuf/proto" +import proto "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/gogoprotobuf/proto" import math "math" // Reference imports to suppress errors if they are not otherwise used. var _ = proto.Marshal var _ = math.Inf -type SemVer struct { - Major *int64 `protobuf:"varint,1,opt,name=major" json:"major,omitempty"` - Minor *int64 `protobuf:"varint,2,opt,name=minor" json:"minor,omitempty"` - Patch *int64 `protobuf:"varint,3,opt,name=patch" json:"patch,omitempty"` - XXX_unrecognized []byte `json:"-"` +type Handshake1 struct { + // protocolVersion determines compatibility between peers + ProtocolVersion *string `protobuf:"bytes,1,opt,name=protocolVersion" json:"protocolVersion,omitempty"` + // agentVersion is like a UserAgent string in browsers, or client version in bittorrent + // includes the client name and client. e.g. "go-ipfs/0.1.0" + AgentVersion *string `protobuf:"bytes,2,opt,name=agentVersion" json:"agentVersion,omitempty"` + XXX_unrecognized []byte `json:"-"` } -func (m *SemVer) Reset() { *m = SemVer{} } -func (m *SemVer) String() string { return proto.CompactTextString(m) } -func (*SemVer) ProtoMessage() {} +func (m *Handshake1) Reset() { *m = Handshake1{} } +func (m *Handshake1) String() string { return proto.CompactTextString(m) } +func (*Handshake1) ProtoMessage() {} -func (m *SemVer) GetMajor() int64 { - if m != nil && m.Major != nil { - return *m.Major +func (m *Handshake1) GetProtocolVersion() string { + if m != nil && m.ProtocolVersion != nil { + return *m.ProtocolVersion } - return 0 + return "" } -func (m *SemVer) GetMinor() int64 { - if m != nil && m.Minor != nil { - return *m.Minor +func (m *Handshake1) GetAgentVersion() string { + if m != nil && m.AgentVersion != nil { + return *m.AgentVersion } - return 0 -} - -func (m *SemVer) GetPatch() int64 { - if m != nil && m.Patch != nil { - return *m.Patch - } - return 0 + return "" } func init() { diff --git a/net/handshake/semver.proto b/net/handshake/semver.proto index 8069830e0..5c6ac2b91 100644 --- a/net/handshake/semver.proto +++ b/net/handshake/semver.proto @@ -1,8 +1,12 @@ package handshake; -message SemVer { - optional int64 major = 1; - optional int64 minor = 2; - optional int64 patch = 3; - // BUG(cryptix): do we need PreRelease and Metadata too? +message Handshake1 { + // protocolVersion determines compatibility between peers + optional string protocolVersion = 1; // semver + + // agentVersion is like a UserAgent string in browsers, or client version in bittorrent + // includes the client name and client. e.g. "go-ipfs/0.1.0" + optional string agentVersion = 2; // semver + + // we'll have more fields here later. } diff --git a/net/handshake/version.go b/net/handshake/version.go index fdf027aab..8b2d3897c 100644 --- a/net/handshake/version.go +++ b/net/handshake/version.go @@ -1,23 +1,54 @@ package handshake +import ( + "errors" + "fmt" + + "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/coreos/go-semver/semver" +) + // currentVersion holds the current protocol version for a client running this code -var currentVersion = NewSemVer(0, 0, 1) +var currentVersion *semver.Version + +func init() { + var err error + currentVersion, err = semver.NewVersion("0.0.1") + if err != nil { + panic(fmt.Errorf("invalid protocol version: %v", err)) + } +} // Current returns the current protocol version as a protobuf message -func Current() *SemVer { +func Current() *semver.Version { return currentVersion } +// ErrVersionMismatch is returned when two clients don't share a protocol version +var ErrVersionMismatch = errors.New("protocol missmatch") + // Compatible checks wether two versions are compatible -func Compatible(a, b *SemVer) bool { - return *a.Major == *b.Major // protobuf fields are pointers +// returns nil if they are fine +func Compatible(handshakeA, handshakeB *Handshake1) error { + a, err := semver.NewVersion(*handshakeA.ProtocolVersion) + if err != nil { + return err + } + b, err := semver.NewVersion(*handshakeB.ProtocolVersion) + if err != nil { + return err + } + + if a.Major != b.Major { + return ErrVersionMismatch + } + + return nil } -// NewSemVer constructs a new protobuf SemVer -func NewSemVer(major, minor, patch int64) *SemVer { - s := new(SemVer) - s.Major = &major - s.Minor = &minor - s.Patch = &patch - return s +// NewHandshake1 creates a new Handshake1 from the two strings +func NewHandshake1(protoVer, agentVer string) *Handshake1 { + return &Handshake1{ + ProtocolVersion: &protoVer, + AgentVersion: &agentVer, + } } diff --git a/net/handshake/version_test.go b/net/handshake/version_test.go index 06d07a49b..de761a7a5 100644 --- a/net/handshake/version_test.go +++ b/net/handshake/version_test.go @@ -4,17 +4,19 @@ import "testing" func TestCompatible(t *testing.T) { tcases := []struct { - a, b *SemVer - expected bool + a, b string + expected error }{ - {NewSemVer(0, 0, 0), NewSemVer(0, 0, 0), true}, - {NewSemVer(0, 0, 0), NewSemVer(1, 0, 0), false}, - {NewSemVer(1, 0, 0), NewSemVer(0, 0, 0), false}, - {NewSemVer(1, 0, 0), NewSemVer(1, 0, 0), true}, + {"0.0.0", "0.0.0", nil}, + {"1.0.0", "1.1.0", nil}, + {"1.0.0", "1.0.1", nil}, + {"0.0.0", "1.0.0", ErrVersionMismatch}, + {"1.0.0", "0.0.0", ErrVersionMismatch}, } for i, tcase := range tcases { - if Compatible(tcase.a, tcase.b) != tcase.expected { + + if Compatible(NewHandshake1(tcase.a, ""), NewHandshake1(tcase.b, "")) != tcase.expected { t.Fatalf("case[%d] failed", i) } } diff --git a/net/swarm/conn.go b/net/swarm/conn.go index 3a6287440..63a5b733a 100644 --- a/net/swarm/conn.go +++ b/net/swarm/conn.go @@ -217,7 +217,7 @@ func (s *Swarm) connVersionExchange(remote *conn.Conn) error { if !handshake.Compatible(myVersion, remoteVersion) { remote.Close() - return errors.New("protocol missmatch") + return handshake.ErrVersionMismatch } log.Debug("[peer: %s] Version compatible", remote.Peer) From 0f47b9300554b78dddd714d521ed8b0de769d569 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 16 Oct 2014 14:07:20 +0200 Subject: [PATCH 9/9] addressed CR by @jbenet --- net/handshake/version.go | 10 +++--- net/swarm/conn.go | 73 +++++++++++++++------------------------- 2 files changed, 33 insertions(+), 50 deletions(-) diff --git a/net/handshake/version.go b/net/handshake/version.go index 8b2d3897c..341522062 100644 --- a/net/handshake/version.go +++ b/net/handshake/version.go @@ -4,7 +4,9 @@ import ( "errors" "fmt" - "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/coreos/go-semver/semver" + updates "github.com/jbenet/go-ipfs/updates" + + semver "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/coreos/go-semver/semver" ) // currentVersion holds the current protocol version for a client running this code @@ -18,9 +20,9 @@ func init() { } } -// Current returns the current protocol version as a protobuf message -func Current() *semver.Version { - return currentVersion +// CurrentHandshake returns the current protocol version as a protobuf message +func CurrentHandshake() *Handshake1 { + return NewHandshake1(currentVersion.String(), "go-ipfs/"+updates.Version) } // ErrVersionMismatch is returned when two clients don't share a protocol version diff --git a/net/swarm/conn.go b/net/swarm/conn.go index 63a5b733a..5aa5a304e 100644 --- a/net/swarm/conn.go +++ b/net/swarm/conn.go @@ -161,63 +161,44 @@ func (s *Swarm) connSecure(c *conn.Conn) error { // connVersionExchange exchanges local and remote versions and compares them // closes remote and returns an error in case of major difference func (s *Swarm) connVersionExchange(remote *conn.Conn) error { - var remoteVersion, myVersion *handshake.SemVer - myVersion = handshake.Current() + var remoteHandshake, localHandshake *handshake.Handshake1 + localHandshake = handshake.CurrentHandshake() - // BUG(cryptix): do we need to use a NetMessage here? - myVersionMsg, err := msg.FromObject(s.local, myVersion) + myVerBytes, err := proto.Marshal(localHandshake) if err != nil { - return fmt.Errorf("connVersionExchange: could not prepare local version: %q", err) + return err } - // buffered channel to send our version just once - outBuf := make(chan []byte, 1) - outBuf <- myVersionMsg.Data() + remote.Secure.Out <- myVerBytes - var gotTheirs, sendMine bool - for { - if gotTheirs && sendMine { - break + log.Debug("Send my version(%s) [to = %s]", localHandshake, remote.Peer) + + select { + case <-s.ctx.Done(): + return s.ctx.Err() + + case <-remote.Closed: + return errors.New("remote closed connection during version exchange") + + case data, ok := <-remote.Secure.In: + if !ok { + return fmt.Errorf("Error retrieving from conn: %v", remote.Peer) } - select { - case <-s.ctx.Done(): - // close Conn. - remote.Close() - return nil // BUG(cryptix): should this be an error? - - case <-remote.Closed: - return errors.New("remote closed connection during version exchange") - - case our, ok := <-outBuf: - if ok { - remote.Secure.Out <- our - sendMine = true - close(outBuf) // only send local version once - log.Debug("Send my version(%s) [to = %s]", myVersion, remote.Peer) - } - - case data, ok := <-remote.Secure.In: - if !ok { - return fmt.Errorf("Error retrieving from conn: %v", remote.Peer) - } - - remoteVersion = new(handshake.SemVer) - err = proto.Unmarshal(data, remoteVersion) - if err != nil { - s.Close() - return fmt.Errorf("connSetup: could not decode remote version: %q", err) - } - gotTheirs = true - log.Debug("Received remote version(%s) [from = %s]", remoteVersion, remote.Peer) - - // BUG(cryptix): could add another case here to trigger resending our version + remoteHandshake = new(handshake.Handshake1) + err = proto.Unmarshal(data, remoteHandshake) + if err != nil { + s.Close() + return fmt.Errorf("connSetup: could not decode remote version: %q", err) } + + log.Debug("Received remote version(%s) [from = %s]", remoteHandshake, remote.Peer) } - if !handshake.Compatible(myVersion, remoteVersion) { + if err := handshake.Compatible(localHandshake, remoteHandshake); err != nil { + log.Info("%s (%s) incompatible version with %s (%s)", s.local, localHandshake, remote.Peer, remoteHandshake) remote.Close() - return handshake.ErrVersionMismatch + return err } log.Debug("[peer: %s] Version compatible", remote.Peer)