Skip to content

Fix index out of range in patricia tree#33

Merged
soheilhy merged 1 commit into
masterfrom
fix-b32
Jul 15, 2016
Merged

Fix index out of range in patricia tree#33
soheilhy merged 1 commit into
masterfrom
fix-b32

Conversation

@soheilhy

Copy link
Copy Markdown
Owner

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

@tamird

tamird commented Jul 15, 2016

Copy link
Copy Markdown
Contributor

No test?

@soheilhy

Copy link
Copy Markdown
Owner Author

👍 will add the tests. Just want to make sure the patch fixes the bug first.

@kalbasit

Copy link
Copy Markdown

works 👍

@kalbasit

Copy link
Copy Markdown

Although I am getting this:

2016/07/15 16:21:15 http: TLS handshake error from 10.2.64.1:41856: EOF

I'll have to dig more into it, currently on the way to the office, will check in 1/2 an hour.

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
@soheilhy

Copy link
Copy Markdown
Owner Author

Thanks for the confirmation!

@tamird, added the tests broken on master and fixed on this branch. ptal.

@soheilhy

Copy link
Copy Markdown
Owner Author

RE: EOF

 2016/07/15 16:21:15 http: TLS handshake error from 10.2.64.1:41856: EOF

I beleive this is because you're using an http server with a tls config. Instead, we need to create a tls.NewListener and then passing it to an HTTP server with no tls config. example_tls_test.go is a good example.

@kalbasit

Copy link
Copy Markdown

@soheilhy that wouldn't work the grpc-gateway :(

diff --git a/cmd/serve.go b/cmd/serve.go
index 381c7ce..9e61979 100644
--- a/cmd/serve.go
+++ b/cmd/serve.go
@@ -57,6 +57,7 @@ var (
        loggerService   *logger.Service
        serverClosed    chan bool
        readyToShutdown chan bool
+       tlsConfig       *tls.Config
 )

 // serveCmd represents the serve command
@@ -263,9 +264,10 @@ func serve(cmd *cobra.Command, args []string) {
                // create the HTTP server
                mainServer = &http.Server{
                        Handler: grpcHandlerFunc(gRPCServer, mux),
-                       TLSConfig: &tls.Config{
-                               Certificates: []tls.Certificate{keyPair},
-                       },
+               }
+               // create the TLS config
+               tlsConfig = &tls.Config{
+                       Certificates: []tls.Certificate{keyPair},
                }
        }

@@ -357,10 +359,8 @@ func serve(cmd *cobra.Command, args []string) {
                                }
                                // wait for the server to die
                                <-serverClosed
-                               // create a new HTTP server
-                               mainServer.TLSConfig = &tls.Config{
-                                       Certificates: []tls.Certificate{keyPair},
-                               }
+                               // update the certificate in tlsConfig
+                               tlsConfig.Certificates = []tls.Certificate{keyPair}
                                // create a new connection
                                if srvListener, err = net.Listen("tcp", listenAddr); err != nil {
                                        log.Fatalf("error opening a socket on %s: %s", listenAddr, err)
@@ -379,12 +379,12 @@ func startServer(mainServer, healthCheckServer *http.Server, srvListener net.Lis
        log.Printf("starting the HTTP/HTTPS/gRPC server bound to %q", listenAddr)
        // create a new connection multiplexer.
        m := cmux.New(srvListener)
-       // we first match on HTTP 1.1 methods.
-       httpl := m.Match(cmux.HTTP1Fast())
+       // we first match on the ReadinessProbe header
+       httpl := m.Match(cmux.HTTP1HeaderField("ReadinessProbe", "32b62d7a2c4227fbf239a9ebe7bce8b9a32cf069"))
        // if not matched, we assume that its TLS.
        tlsl := m.Match(cmux.Any())
        // start the mainServer
-       go mainServer.Serve(tls.NewListener(tlsl, mainServer.TLSConfig))
+       go mainServer.Serve(tls.NewListener(tlsl, tlsConfig))
        // start the healthCheckServer
        go healthCheckServer.Serve(httpl)
        // boot the connection multiplexer, this should return once the srvListener

when I run it:

2016/07/15 10:34:27 starting the HTTP/HTTPS/gRPC server bound to ":8091"
2016/07/15 10:34:27 transport: http2Client.notifyError got notified that the client transport was broken unexpected EOF.
2016/07/15 10:34:27 transport: http2Client.notifyError got notified that the client transport was broken unexpected EOF.
2016/07/15 10:34:29 grpc: addrConn.resetTransport failed to create client transport: connection error: desc = "transport: write tcp 127.0.0.1:52766->127.0.0.1:8091: write: broken pipe"; Reconnecting to {"127.0.0.1:8091" <nil>}

@soheilhy

Copy link
Copy Markdown
Owner Author

@kalbasit hmm... I don't have a good answer on why TLS doesn't work for you. I tried a few examples and seems to be working fine. Since this isn't related to this pull request, can you please create another issue with a minimal example to reproduce the bug?

Thanks.

@kalbasit

Copy link
Copy Markdown

@soheilhy agree, I'll investigate more and if I'll make an minimal example if needed.

@tamird

tamird commented Jul 15, 2016

Copy link
Copy Markdown
Contributor

lgtm

@soheilhy

Copy link
Copy Markdown
Owner Author

Thanks guys!

@soheilhy soheilhy merged commit b269515 into master Jul 15, 2016
tamird pushed a commit to cockroachdb/cmux that referenced this pull request Jul 15, 2016
Fix index out of range in patricia tree
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants