From 1b5deaf340802a706d16e7ad34e9763b8ea257b7 Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Fri, 29 Jul 2016 13:42:59 -0700 Subject: [PATCH 01/10] clean a transport test --- transport/transport_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/transport/transport_test.go b/transport/transport_test.go index f4af68b7..95d3a87b 100644 --- a/transport/transport_test.go +++ b/transport/transport_test.go @@ -697,14 +697,15 @@ func TestClientWithMisbehavedServer(t *testing.T) { Host: "localhost", Method: "foo.MaxFrame", } + d := make([]byte, 1) // Make the server flood the traffic to violate flow control window size of the connection. - for { + for i := 0; i < int(initialConnWindowSize/initialWindowSize+10); i++ { s, err := ct.NewStream(context.Background(), callHdr) if err != nil { - break + t.Fatalf("Failed to create a new stream: %v", err) } - if err := ct.Write(s, expectedRequest, &Options{Last: true, Delay: false}); err != nil { - break + if err := ct.Write(s, d, &Options{Last: true, Delay: false}); err != nil { + t.Fatalf("Failed to write data to %v: %v", s, err) } } // http2Client.errChan is closed due to connection flow control window size violation. From 77ec0f0a5b6ac0eaa12f4698d753a0f85df1ffd3 Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Fri, 29 Jul 2016 15:48:44 -0700 Subject: [PATCH 02/10] refactor the test --- transport/transport_test.go | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/transport/transport_test.go b/transport/transport_test.go index 50c03d79..ec26887a 100644 --- a/transport/transport_test.go +++ b/transport/transport_test.go @@ -111,13 +111,17 @@ func (h *testStreamHandler) handleStreamMisbehave(t *testing.T, s *Stream) { if !ok { t.Fatalf("Failed to convert %v to *http2Server", s.ServerTransport()) } - size := 1 + var end, size int if s.Method() == "foo.MaxFrame" { - size = http2MaxFrameLen + size = http2MaxFrameLen - 1 + end = initialWindowSize + } else { + size = 1 + end = initialWindowSize + 1 } - // Drain the client side stream flow control window. + // Violate the client side stream/connection flow control window. var sent int - for sent <= initialWindowSize { + for sent < end { <-conn.writableChan if err := conn.framer.writeData(true, s.id, false, make([]byte, size)); err != nil { conn.writableChan <- 0 @@ -671,7 +675,8 @@ func TestClientWithMisbehavedServer(t *testing.T) { if err != nil { t.Fatalf("Failed to open stream: %v", err) } - if err := ct.Write(s, expectedRequest, &Options{Last: true, Delay: false}); err != nil { + d := make([]byte, 1) + if err := ct.Write(s, d, &Options{Last: true, Delay: false}); err != nil { t.Fatalf("Failed to write: %v", err) } // Read without window update. @@ -693,13 +698,9 @@ func TestClientWithMisbehavedServer(t *testing.T) { } // Test the logic for the violation of the connection flow control window size restriction. // - // Generate enough streams to drain the connection window. - callHdr = &CallHdr{ - Host: "localhost", - Method: "foo.MaxFrame", - } - d := make([]byte, 1) - // Make the server flood the traffic to violate flow control window size of the connection. + // Generate enough streams to drain the connection window. Make the server flood the traffic + // to violate flow control window size of the connection. + callHdr.Method = "foo.MaxFrame" for i := 0; i < int(initialConnWindowSize/initialWindowSize+10); i++ { s, err := ct.NewStream(context.Background(), callHdr) if err != nil { From 5093059db242222bf68d81f5d49258bf4d31bb25 Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Fri, 29 Jul 2016 17:36:12 -0700 Subject: [PATCH 03/10] fix a counting error in the previous commit --- transport/transport_test.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/transport/transport_test.go b/transport/transport_test.go index ec26887a..80a1b15f 100644 --- a/transport/transport_test.go +++ b/transport/transport_test.go @@ -111,24 +111,26 @@ func (h *testStreamHandler) handleStreamMisbehave(t *testing.T, s *Stream) { if !ok { t.Fatalf("Failed to convert %v to *http2Server", s.ServerTransport()) } - var end, size int - if s.Method() == "foo.MaxFrame" { - size = http2MaxFrameLen - 1 + var end int + if s.Method() == "foo.Connection" { + // Violate connection level flow control window of client but do not + // violate any stream level windows. end = initialWindowSize } else { - size = 1 + // Violate stream level flow control window of client. end = initialWindowSize + 1 } // Violate the client side stream/connection flow control window. var sent int + p := make([]byte, 1) for sent < end { <-conn.writableChan - if err := conn.framer.writeData(true, s.id, false, make([]byte, size)); err != nil { + if err := conn.framer.writeData(true, s.id, false, p); err != nil { conn.writableChan <- 0 break } conn.writableChan <- 0 - sent += size + sent += len(p) } } @@ -664,7 +666,7 @@ func TestClientWithMisbehavedServer(t *testing.T) { server, ct := setUp(t, 0, math.MaxUint32, misbehaved) callHdr := &CallHdr{ Host: "localhost", - Method: "foo", + Method: "foo.Stream", } conn, ok := ct.(*http2Client) if !ok { @@ -700,7 +702,7 @@ func TestClientWithMisbehavedServer(t *testing.T) { // // Generate enough streams to drain the connection window. Make the server flood the traffic // to violate flow control window size of the connection. - callHdr.Method = "foo.MaxFrame" + callHdr.Method = "foo.Connection" for i := 0; i < int(initialConnWindowSize/initialWindowSize+10); i++ { s, err := ct.NewStream(context.Background(), callHdr) if err != nil { From 2e3bdf55f82dc683ca1c9907a7bd1a915ee41d71 Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Fri, 29 Jul 2016 17:52:30 -0700 Subject: [PATCH 04/10] make it faster --- transport/transport_test.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/transport/transport_test.go b/transport/transport_test.go index 80a1b15f..7fecbeaf 100644 --- a/transport/transport_test.go +++ b/transport/transport_test.go @@ -111,20 +111,22 @@ func (h *testStreamHandler) handleStreamMisbehave(t *testing.T, s *Stream) { if !ok { t.Fatalf("Failed to convert %v to *http2Server", s.ServerTransport()) } - var end int - if s.Method() == "foo.Connection" { - // Violate connection level flow control window of client but do not - // violate any stream level windows. - end = initialWindowSize - } else { - // Violate stream level flow control window of client. - end = initialWindowSize + 1 - } - // Violate the client side stream/connection flow control window. var sent int - p := make([]byte, 1) - for sent < end { + p := make([]byte, http2MaxFrameLen) + for sent < initialWindowSize { <-conn.writableChan + n := initialWindowSize - sent + // The last message may be smaller than http2MaxFrameLen + if n < http2MaxFrameLen { + if s.Method() == "foo.Connection" { + // Violate connection level flow control window of client but do not + // violate any stream level windows. + p = make([]byte, n) + } else { + // Violate stream level flow control window of client. + p = make([]byte, n+1) + } + } if err := conn.framer.writeData(true, s.id, false, p); err != nil { conn.writableChan <- 0 break From 849641790e3d324194c6a02d14fe765ef1905ce8 Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Fri, 29 Jul 2016 17:58:17 -0700 Subject: [PATCH 05/10] small fix --- transport/transport_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transport/transport_test.go b/transport/transport_test.go index 7fecbeaf..54315ee7 100644 --- a/transport/transport_test.go +++ b/transport/transport_test.go @@ -117,7 +117,7 @@ func (h *testStreamHandler) handleStreamMisbehave(t *testing.T, s *Stream) { <-conn.writableChan n := initialWindowSize - sent // The last message may be smaller than http2MaxFrameLen - if n < http2MaxFrameLen { + if n <= http2MaxFrameLen { if s.Method() == "foo.Connection" { // Violate connection level flow control window of client but do not // violate any stream level windows. From 0eea2a699dc6a4c3693244963edcf66669579220 Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Sat, 30 Jul 2016 11:11:39 -0700 Subject: [PATCH 06/10] add a missing server.stop() --- transport/transport_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/transport/transport_test.go b/transport/transport_test.go index 21068ed3..54315ee7 100644 --- a/transport/transport_test.go +++ b/transport/transport_test.go @@ -561,6 +561,7 @@ func TestServerContextCanceledOnClosedConnection(t *testing.T) { case <-time.After(5 * time.Second): t.Fatalf("Failed to cancel the context of the sever side stream.") } + server.stop() } func TestServerWithMisbehavedClient(t *testing.T) { From 2f16a90cc78b24d9a43fd269e29b9fa358ddd719 Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Sat, 30 Jul 2016 11:18:58 -0700 Subject: [PATCH 07/10] tune a end2end test --- test/end2end_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/end2end_test.go b/test/end2end_test.go index a272a91c..bc03000c 100644 --- a/test/end2end_test.go +++ b/test/end2end_test.go @@ -2156,8 +2156,8 @@ func testClientRequestBodyError_Cancel(t *testing.T, e env) { te.withServerTester(func(st *serverTester) { st.writeHeadersGRPC(1, "/grpc.testing.TestService/UnaryCall") // Say we have 5 bytes coming, but cancel it instead. - st.writeData(1, false, []byte{0, 0, 0, 0, 5}) st.writeRSTStream(1, http2.ErrCodeCancel) + st.writeData(1, false, []byte{0, 0, 0, 0, 5}) // Verify we didn't a call yet. select { From fb5fcd93fd2621a8cd2735424392c37f8016562c Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Sat, 30 Jul 2016 11:26:07 -0700 Subject: [PATCH 08/10] revert some changes on the transport test --- transport/transport_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/transport/transport_test.go b/transport/transport_test.go index 54315ee7..15372f3e 100644 --- a/transport/transport_test.go +++ b/transport/transport_test.go @@ -708,10 +708,10 @@ func TestClientWithMisbehavedServer(t *testing.T) { for i := 0; i < int(initialConnWindowSize/initialWindowSize+10); i++ { s, err := ct.NewStream(context.Background(), callHdr) if err != nil { - t.Fatalf("Failed to create a new stream: %v", err) + break } if err := ct.Write(s, d, &Options{Last: true, Delay: false}); err != nil { - t.Fatalf("Failed to write data to %v: %v", s, err) + break } } // http2Client.errChan is closed due to connection flow control window size violation. From 0f38a650e625fa3293df4467d03ac0744b1a07d6 Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Sat, 30 Jul 2016 11:31:00 -0700 Subject: [PATCH 09/10] a small fix --- transport/transport_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transport/transport_test.go b/transport/transport_test.go index 15372f3e..20ae1334 100644 --- a/transport/transport_test.go +++ b/transport/transport_test.go @@ -739,7 +739,7 @@ func TestEncodingRequiredStatus(t *testing.T) { Last: true, Delay: false, } - if err := ct.Write(s, expectedRequest, &opts); err != nil || err == io.EOF { + if err := ct.Write(s, expectedRequest, &opts); err != nil || err != io.EOF { t.Fatalf("Failed to write the request: %v", err) } p := make([]byte, http2MaxFrameLen) From 405998a8362f08fee6f3d3a462e72295b441c1b1 Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Sat, 30 Jul 2016 11:32:15 -0700 Subject: [PATCH 10/10] a small fix --- transport/transport_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transport/transport_test.go b/transport/transport_test.go index 20ae1334..e57d8e4a 100644 --- a/transport/transport_test.go +++ b/transport/transport_test.go @@ -739,7 +739,7 @@ func TestEncodingRequiredStatus(t *testing.T) { Last: true, Delay: false, } - if err := ct.Write(s, expectedRequest, &opts); err != nil || err != io.EOF { + if err := ct.Write(s, expectedRequest, &opts); err != nil && err != io.EOF { t.Fatalf("Failed to write the request: %v", err) } p := make([]byte, http2MaxFrameLen)