Fix index out of range in patricia tree

Bug #32 reported that there is an index out of range error. This
issue was introduced in 703b087.

Fix #32 and add a test to detect this issue

.travis.yml

@@ -24,4 +24,5 @@ before_script:
   - if [[ $TRAVIS_GO_VERSION == 1.6* ]]; then go tool vet --shadow .; fi
 
 script:
-  - go test -v ./...
+  - go test -bench . -v ./...
+  - go test -race -bench . -v ./...

CONTRIBUTORS

@@ -0,0 +1,11 @@
+# The list of people who have contributed code to the cmux repository.
+#
+# Auto-generated with:
+#		git log --oneline --pretty=format:'%an <%aE>' | sort -u
+#
+Dmitri Shuralyov <shurcooL@gmail.com>
+Ethan Mosbaugh <emosbaugh@gmail.com>
+Soheil Hassas Yeganeh <soheil.h.y@gmail.com>
+Soheil Hassas Yeganeh <soheil@cs.toronto.edu>
+Tamir Duberstein <tamir@cockroachlabs.com>
+Tamir Duberstein <tamird@gmail.com>

README.md

@@ -67,3 +67,17 @@ would not be set in your handlers.
 when it's accepted. For example, one connection can be either gRPC or REST, but
 not both. That is, we assume that a client connection is either used for gRPC
 or REST.
+
+* *Java gRPC Clients*: Java gRPC client blocks until it receives a SETTINGS
+frame from the server. If you are using the Java client to connect to a cmux'ed
+gRPC server please match with writers:
+```go
+grpcl := m.MatchWithWriters(cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"))
+```
+
+# Copyright and License
+Copyright 2016 The CMux Authors. All rights reserved.
+
+See [CONTRIBUTORS](https://github.com/soheilhy/cmux/blob/master/CONTRIBUTORS)
+for the CMux Authors. Code is released under
+[the Apache 2 license](https://github.com/soheilhy/cmux/blob/master/LICENSE).

bench_test.go

@@ -1,3 +1,17 @@
+// Copyright 2016 The CMux Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
 package cmux
 
 import (
@@ -6,8 +20,20 @@ import (
 	"net"
 	"sync"
 	"testing"
+
+	"golang.org/x/net/http2"
 )
 
+var (
+	benchHTTP1Payload = make([]byte, 4096)
+	benchHTTP2Payload = make([]byte, 4096)
+)
+
+func init() {
+	copy(benchHTTP1Payload, []byte("GET http://www.w3.org/ HTTP/1.1"))
+	copy(benchHTTP2Payload, http2.ClientPreface)
+}
+
 type mockConn struct {
 	net.Conn
 	r io.Reader
@@ -17,30 +43,95 @@ func (c *mockConn) Read(b []byte) (n int, err error) {
 	return c.r.Read(b)
 }
 
-func BenchmarkCMuxConn(b *testing.B) {
+func discard(l net.Listener) {
-	benchHTTPPayload := make([]byte, 4096)
-	copy(benchHTTPPayload, []byte("GET http://www.w3.org/ HTTP/1.1"))
-
-	m := New(nil).(*cMux)
-	l := m.Match(HTTP1Fast())
-
-	go func() {
 	for {
 		if _, err := l.Accept(); err != nil {
 			return
 		}
 	}
-	}()
+}
+
+func BenchmarkCMuxConnHTTP1(b *testing.B) {
+	m := New(nil).(*cMux)
+	l := m.Match(HTTP1Fast())
+
+	go discard(l)
 
 	donec := make(chan struct{})
 	var wg sync.WaitGroup
 	wg.Add(b.N)
 
 	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
+	b.RunParallel(func(pb *testing.PB) {
-		c := &mockConn{
+		for pb.Next() {
-			r: bytes.NewReader(benchHTTPPayload),
+			wg.Add(1)
-		}
+			m.serve(&mockConn{
-		m.serve(c, donec, &wg)
+				r: bytes.NewReader(benchHTTP1Payload),
+			}, donec, &wg)
 		}
+	})
+}
+
+func BenchmarkCMuxConnHTTP2(b *testing.B) {
+	m := New(nil).(*cMux)
+	l := m.Match(HTTP2())
+	go discard(l)
+
+	donec := make(chan struct{})
+	var wg sync.WaitGroup
+	wg.Add(b.N)
+
+	b.ResetTimer()
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			wg.Add(1)
+			m.serve(&mockConn{
+				r: bytes.NewReader(benchHTTP2Payload),
+			}, donec, &wg)
+		}
+	})
+}
+
+func BenchmarkCMuxConnHTTP1n2(b *testing.B) {
+	m := New(nil).(*cMux)
+	l1 := m.Match(HTTP1Fast())
+	l2 := m.Match(HTTP2())
+
+	go discard(l1)
+	go discard(l2)
+
+	donec := make(chan struct{})
+	var wg sync.WaitGroup
+
+	b.ResetTimer()
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			wg.Add(1)
+			m.serve(&mockConn{
+				r: bytes.NewReader(benchHTTP2Payload),
+			}, donec, &wg)
+		}
+	})
+}
+
+func BenchmarkCMuxConnHTTP2n1(b *testing.B) {
+	m := New(nil).(*cMux)
+	l2 := m.Match(HTTP2())
+	l1 := m.Match(HTTP1Fast())
+
+	go discard(l1)
+	go discard(l2)
+
+	donec := make(chan struct{})
+	var wg sync.WaitGroup
+
+	b.ResetTimer()
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			wg.Add(1)
+			m.serve(&mockConn{
+				r: bytes.NewReader(benchHTTP1Payload),
+			}, donec, &wg)
+		}
+	})
 }

buffer.go

@@ -1,3 +1,17 @@
+// Copyright 2016 The CMux Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
 package cmux
 
 import (

cmux.go

@@ -1,3 +1,17 @@
+// Copyright 2016 The CMux Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
 package cmux
 
 import (
@@ -10,6 +24,9 @@ import (
 // Matcher matches a connection based on its content.
 type Matcher func(io.Reader) bool
 
+// MatchWriter is a match that can also write response (say to do handshake).
+type MatchWriter func(io.Writer, io.Reader) bool
+
 // ErrorHandler handles an error and returns whether
 // the mux should continue serving the listener.
 type ErrorHandler func(error) bool
@@ -60,6 +77,14 @@ type CMux interface {
 	//
 	// The order used to call Match determines the priority of matchers.
 	Match(...Matcher) net.Listener
+	// MatchWithWriters returns a net.Listener that accepts only the
+	// connections that matched by at least of the matcher writers.
+	//
+	// Prefer Matchers over MatchWriters, since the latter can write on the
+	// connection before the actual handler.
+	//
+	// The order used to call Match determines the priority of matchers.
+	MatchWithWriters(...MatchWriter) net.Listener
 	// Serve starts multiplexing the listener. Serve blocks and perhaps
 	// should be invoked concurrently within a go routine.
 	Serve() error
@@ -68,7 +93,7 @@ type CMux interface {
 }
 
 type matchersListener struct {
-	ss []Matcher
+	ss []MatchWriter
 	l  muxListener
 }
 
@@ -80,7 +105,22 @@ type cMux struct {
 	sls    []matchersListener
 }
 
+func matchersToMatchWriters(matchers []Matcher) []MatchWriter {
+	mws := make([]MatchWriter, 0, len(matchers))
+	for _, m := range matchers {
+		mws = append(mws, func(w io.Writer, r io.Reader) bool {
+			return m(r)
+		})
+	}
+	return mws
+}
+
 func (m *cMux) Match(matchers ...Matcher) net.Listener {
+	mws := matchersToMatchWriters(matchers)
+	return m.MatchWithWriters(mws...)
+}
+
+func (m *cMux) MatchWithWriters(matchers ...MatchWriter) net.Listener {
 	ml := muxListener{
 		Listener: m.root,
 		connc:    make(chan net.Conn, m.bufLen),
@@ -125,7 +165,7 @@ func (m *cMux) serve(c net.Conn, donec <-chan struct{}, wg *sync.WaitGroup) {
 	muc := newMuxConn(c)
 	for _, sl := range m.sls {
 		for _, s := range sl.ss {
-			matched := s(muc.startSniffing())
+			matched := s(muc.Conn, muc.startSniffing())
 			if matched {
 				muc.doneSniffing()
 				select {

cmux_test.go

@@ -1,3 +1,17 @@
+// Copyright 2016 The CMux Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
 package cmux
 
 import (
@@ -109,22 +123,24 @@ func runTestHTTPServer(errCh chan<- error, l net.Listener) {
 }
 
 func runTestHTTP1Client(t *testing.T, addr net.Addr) {
-	if r, err := http.Get("http://" + addr.String()); err != nil {
+	r, err := http.Get("http://" + addr.String())
+	if err != nil {
 		t.Fatal(err)
-	} else {
+	}
+
 	defer func() {
-			if err := r.Body.Close(); err != nil {
+		if err = r.Body.Close(); err != nil {
 			t.Fatal(err)
 		}
 	}()
-		if b, err := ioutil.ReadAll(r.Body); err != nil {
+
+	b, err := ioutil.ReadAll(r.Body)
+	if err != nil {
 		t.Fatal(err)
-		} else {
+	}
 	if string(b) != testHTTP1Resp {
 		t.Fatalf("invalid response: want=%s got=%s", testHTTP1Resp, b)
 	}
-		}
-	}
 }
 
 type TestRPCRcvr struct{}
@@ -208,9 +224,12 @@ func TestRead(t *testing.T) {
 	}
 	for i := 0; i < mult; i++ {
 		var b [len(payload)]byte
-		if n, err := muxedConn.Read(b[:]); err != nil {
+		n, err := muxedConn.Read(b[:])
+		if err != nil {
 			t.Error(err)
-		} else if e := len(b); n != e {
+			continue
+		}
+		if e := len(b); n != e {
 			t.Errorf("expected to read %d bytes, but read %d bytes", e, n)
 		}
 	}

doc.go

@@ -0,0 +1,18 @@
+// Copyright 2016 The CMux Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+// Package cmux is a library to multiplex network connections based on
+// their payload. Using cmux, you can serve different protocols from the
+// same listener.
+package cmux

example_recursive_test.go

@@ -1,3 +1,17 @@
+// Copyright 2016 The CMux Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
 package cmux_test
 
 import (

example_test.go

@@ -1,3 +1,17 @@
+// Copyright 2016 The CMux Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
 package cmux_test
 
 import (

example_tls_test.go

@@ -1,3 +1,17 @@
+// Copyright 2016 The CMux Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
 package cmux_test
 
 import (

matchers.go

@@ -1,3 +1,17 @@
+// Copyright 2016 The CMux Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
 package cmux
 
 import (
@@ -94,7 +108,16 @@ func HTTP1HeaderField(name, value string) Matcher {
 // headers frame.
 func HTTP2HeaderField(name, value string) Matcher {
 	return func(r io.Reader) bool {
-		return matchHTTP2Field(r, name, value)
+		return matchHTTP2Field(ioutil.Discard, r, name, value)
+	}
+}
+
+// HTTP2MatchHeaderFieldSendSettings matches the header field and writes the
+// settings to the server. Prefer HTTP2HeaderField over this one, if the client
+// does not block on receiving a SETTING frame.
+func HTTP2MatchHeaderFieldSendSettings(name, value string) MatchWriter {
+	return func(w io.Writer, r io.Reader) bool {
+		return matchHTTP2Field(w, r, name, value)
 	}
 }
 
@@ -116,12 +139,12 @@ func matchHTTP1Field(r io.Reader, name, value string) (matched bool) {
 	return req.Header.Get(name) == value
 }
 
-func matchHTTP2Field(r io.Reader, name, value string) (matched bool) {
+func matchHTTP2Field(w io.Writer, r io.Reader, name, value string) (matched bool) {
 	if !hasHTTP2Preface(r) {
 		return false
 	}
 
-	framer := http2.NewFramer(ioutil.Discard, r)
+	framer := http2.NewFramer(w, r)
 	hdec := hpack.NewDecoder(uint32(4<<10), func(hf hpack.HeaderField) {
 		if hf.Name == name && hf.Value == value {
 			matched = true
@@ -134,6 +157,10 @@ func matchHTTP2Field(r io.Reader, name, value string) (matched bool) {
 		}
 
 		switch f := f.(type) {
+		case *http2.SettingsFrame:
+			if err := framer.WriteSettings(); err != nil {
+				return false
+			}
 		case *http2.HeadersFrame:
 			if _, err := hdec.Write(f.HeaderBlockFragment()); err != nil {
 				return false

patricia.go

@@ -1,3 +1,17 @@
+// Copyright 2016 The CMux Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
 package cmux
 
 import (
@@ -9,11 +23,19 @@ import (
 // and cannot be changed after instantiation.
 type patriciaTree struct {
 	root     *ptNode
+	maxDepth int // max depth of the tree.
 }
 
-func newPatriciaTree(b ...[]byte) *patriciaTree {
+func newPatriciaTree(bs ...[]byte) *patriciaTree {
+	max := 0
+	for _, b := range bs {
+		if max < len(b) {
+			max = len(b)
+		}
+	}
 	return &patriciaTree{
-		root: newNode(b),
+		root:     newNode(bs),
+		maxDepth: max + 1,
 	}
 }
 
@@ -22,17 +44,19 @@ func newPatriciaTreeString(strs ...string) *patriciaTree {
 	for i, s := range strs {
 		b[i] = []byte(s)
 	}
-	return &patriciaTree{
+	return newPatriciaTree(b...)
-		root: newNode(b),
-	}
 }
 
 func (t *patriciaTree) matchPrefix(r io.Reader) bool {
-	return t.root.match(r, true)
+	buf := make([]byte, t.maxDepth)
+	n, _ := io.ReadFull(r, buf)
+	return t.root.match(buf[:n], true)
 }
 
 func (t *patriciaTree) match(r io.Reader) bool {
-	return t.root.match(r, false)
+	buf := make([]byte, t.maxDepth)
+	n, _ := io.ReadFull(r, buf)
+	return t.root.match(buf[:n], false)
 }
 
 type ptNode struct {
@@ -122,52 +146,34 @@ func splitPrefix(bss [][]byte) (prefix []byte, rest [][]byte) {
 	return prefix, rest
 }
 
-func readBytes(r io.Reader, n int) (b []byte, err error) {
+func (n *ptNode) match(b []byte, prefix bool) bool {
-	b = make([]byte, n)
+	l := len(n.prefix)
-	o := 0
+	if l > 0 {
-	for o < n {
+		if l > len(b) {
-		nr, err := r.Read(b[o:])
+			l = len(b)
-		if err != nil && err != io.EOF {
-			return b, err
 		}
-
+		if !bytes.Equal(b[:l], n.prefix) {
-		o += nr
-
-		if err == io.EOF {
-			break
-		}
-	}
-	return b[:o], nil
-}
-
-func (n *ptNode) match(r io.Reader, prefix bool) bool {
-	if l := len(n.prefix); l > 0 {
-		b, err := readBytes(r, l)
-		if err != nil || len(b) != l || !bytes.Equal(b, n.prefix) {
 			return false
 		}
 	}
 
-	if prefix && n.terminal {
+	if n.terminal && (prefix || len(n.prefix) == len(b)) {
 		return true
 	}
 
-	b := make([]byte, 1)
+	if l >= len(b) {
-	for {
-		nr, err := r.Read(b)
-		if nr != 0 {
-			break
-		}
-
-		if err == io.EOF {
-			return n.terminal
-		}
-
-		if err != nil {
 		return false
 	}
+
+	nextN, ok := n.next[b[l]]
+	if !ok {
+		return false
 	}
 
-	nextN, ok := n.next[b[0]]
+	if l == len(b) {
-	return ok && nextN.match(r, prefix)
+		b = b[l:l]
+	} else {
+		b = b[l+1:]
+	}
+	return nextN.match(b, prefix)
 }

patricia_test.go

@@ -1,3 +1,17 @@
+// Copyright 2016 The CMux Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
 package cmux
 
 import (
@@ -19,6 +33,13 @@ func testPTree(t *testing.T, strs ...string) {
 		if pt.match(strings.NewReader(s + s)) {
 			t.Errorf("%s matches %s", s+s, s)
 		}
+
+		// The following tests are just to catch index out of
+		// range and off-by-one errors and not the functionality.
+		pt.matchPrefix(strings.NewReader(s[:len(s)-1]))
+		pt.match(strings.NewReader(s[:len(s)-1]))
+		pt.matchPrefix(strings.NewReader(s + "$"))
+		pt.match(strings.NewReader(s + "$"))
 	}
 }
 
@@ -31,5 +52,5 @@ func TestPatriciaNonOverlapping(t *testing.T) {
 }
 
 func TestPatriciaOverlapping(t *testing.T) {
-	testPTree(t, "foo", "far", "farther", "boo", "bar")
+	testPTree(t, "foo", "far", "farther", "boo", "ba", "bar")
 }