From 3cf4c7bdb5617364d8b9dece265a256b2ad32886 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Mon, 18 Aug 2025 08:00:27 +0000 Subject: [PATCH 1/6] Fixed the Handle 1xx HTTP status HEADERS frames correctly --- internal/transport/http2_client.go | 22 ++++++--- internal/transport/transport_test.go | 68 ++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/internal/transport/http2_client.go b/internal/transport/http2_client.go index 5467fe9715a3..2cbb111269a4 100644 --- a/internal/transport/http2_client.go +++ b/internal/transport/http2_client.go @@ -1499,13 +1499,6 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) { case "grpc-message": grpcMessage = decodeGrpcMessage(hf.Value) case ":status": - if hf.Value == "200" { - httpStatusErr = "" - statusCode := 200 - httpStatusCode = &statusCode - break - } - c, err := strconv.ParseInt(hf.Value, 10, 32) if err != nil { se := status.New(codes.Internal, fmt.Sprintf("transport: malformed http-status: %v", err)) @@ -1513,7 +1506,22 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) { return } statusCode := int(c) + if statusCode >= 100 && statusCode < 200 { + if endStream { + se := status.New(codes.Internal, fmt.Sprintf( + "protocol error: received 1xx informational header with END_STREAM set: %d", statusCode)) + t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream) + return + } + return + } httpStatusCode = &statusCode + if hf.Value == "200" { + httpStatusErr = "" + statusCode := 200 + httpStatusCode = &statusCode + break + } httpStatusErr = fmt.Sprintf( "unexpected HTTP status code received from server: %d (%s)", diff --git a/internal/transport/transport_test.go b/internal/transport/transport_test.go index b8f97c3c9464..a43715933cac 100644 --- a/internal/transport/transport_test.go +++ b/internal/transport/transport_test.go @@ -3125,3 +3125,71 @@ func (s) TestServerSendsRSTAfterDeadlineToMisbehavedClient(t *testing.T) { t.Fatalf("RST frame received earlier than expected by duration: %v", want-got) } } + +// TestClientTransport_Handle1xxHeaders validates that 1xx HTTP status +// headers are ignored unless END_STREAM is also set, which results in a +// protocol error. +func (s) TestClientTransport_Handle1xxHeaders(t *testing.T) { + testStream := func() *ClientStream { + return &ClientStream{ + Stream: &Stream{ + buf: &recvBuffer{ + c: make(chan recvMsg), + mu: sync.Mutex{}, + }, + }, + done: make(chan struct{}), + headerChan: make(chan struct{}), + } + } + + testClient := func(ts *ClientStream) *http2Client { + return &http2Client{ + mu: sync.Mutex{}, + activeStreams: map[uint32]*ClientStream{ + 0: ts, + }, + controlBuf: newControlBuffer(make(<-chan struct{})), + } + } + + for _, test := range []struct { + name string + metaHeaderFrame *http2.MetaHeadersFrame + wantStatus *status.Status + }{ + { + name: "1xx with END_STREAM is error", + metaHeaderFrame: &http2.MetaHeadersFrame{ + Fields: []hpack.HeaderField{ + {Name: ":status", Value: "100"}, + }, + }, + wantStatus: status.New( + codes.Internal, + "protocol error: received 1xx informational header with END_STREAM set: 100", + ), + }, + } { + t.Run(test.name, func(t *testing.T) { + ts := testStream() + s := testClient(ts) + + test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{ + FrameHeader: http2.FrameHeader{ + StreamID: 0, + Flags: http2.FlagHeadersEndStream, + }, + } + + s.operateHeaders(test.metaHeaderFrame) + + got := ts.status + want := test.wantStatus + + if got.Code() != want.Code() || got.Message() != want.Message() { + t.Fatalf("operateHeaders(%v); status = \ngot: %v\nwant: %v", test.metaHeaderFrame, got, want) + } + }) + } +} From 3a682d69f95174664e872409af8f3572b8521cf3 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Mon, 18 Aug 2025 14:02:15 +0000 Subject: [PATCH 2/6] small tweaks --- internal/transport/http2_client.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/transport/http2_client.go b/internal/transport/http2_client.go index 2cbb111269a4..4f9e011623d7 100644 --- a/internal/transport/http2_client.go +++ b/internal/transport/http2_client.go @@ -1513,7 +1513,6 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) { t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream) return } - return } httpStatusCode = &statusCode if hf.Value == "200" { From 4aa736c0ae56a750c1bbf9dcfbf3964ec66ef595 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Mon, 18 Aug 2025 14:04:22 +0000 Subject: [PATCH 3/6] small tweaks --- internal/transport/http2_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/transport/http2_client.go b/internal/transport/http2_client.go index 4f9e011623d7..28bf9413b289 100644 --- a/internal/transport/http2_client.go +++ b/internal/transport/http2_client.go @@ -1511,8 +1511,8 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) { se := status.New(codes.Internal, fmt.Sprintf( "protocol error: received 1xx informational header with END_STREAM set: %d", statusCode)) t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream) - return } + return } httpStatusCode = &statusCode if hf.Value == "200" { From 99675c3586312b8b9598071e0c43300befe4231a Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Mon, 18 Aug 2025 14:13:54 +0000 Subject: [PATCH 4/6] Docs updates --- internal/transport/transport_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/transport/transport_test.go b/internal/transport/transport_test.go index a43715933cac..be60622285f2 100644 --- a/internal/transport/transport_test.go +++ b/internal/transport/transport_test.go @@ -3126,9 +3126,8 @@ func (s) TestServerSendsRSTAfterDeadlineToMisbehavedClient(t *testing.T) { } } -// TestClientTransport_Handle1xxHeaders validates that 1xx HTTP status -// headers are ignored unless END_STREAM is also set, which results in a -// protocol error. +// TestClientTransport_Handle1xxHeaders validates that 1xx HTTP status headers +// are ignored and treated as a protocol error if END_STREAM is set. func (s) TestClientTransport_Handle1xxHeaders(t *testing.T) { testStream := func() *ClientStream { return &ClientStream{ From 7dac9ddc0155846a00264bec31afeff265b48926 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Tue, 26 Aug 2025 17:03:55 +0000 Subject: [PATCH 5/6] Fixed the review changes --- internal/transport/http2_client.go | 6 ++---- internal/transport/transport_test.go | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/internal/transport/http2_client.go b/internal/transport/http2_client.go index 28bf9413b289..c31ed87b1676 100644 --- a/internal/transport/http2_client.go +++ b/internal/transport/http2_client.go @@ -1509,16 +1509,14 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) { if statusCode >= 100 && statusCode < 200 { if endStream { se := status.New(codes.Internal, fmt.Sprintf( - "protocol error: received 1xx informational header with END_STREAM set: %d", statusCode)) + "protocol error: informational header with status code %d must not have END_STREAM set", statusCode)) t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream) } return } httpStatusCode = &statusCode - if hf.Value == "200" { + if statusCode == 200 { httpStatusErr = "" - statusCode := 200 - httpStatusCode = &statusCode break } diff --git a/internal/transport/transport_test.go b/internal/transport/transport_test.go index be60622285f2..d5b96fa60866 100644 --- a/internal/transport/transport_test.go +++ b/internal/transport/transport_test.go @@ -3155,6 +3155,7 @@ func (s) TestClientTransport_Handle1xxHeaders(t *testing.T) { for _, test := range []struct { name string metaHeaderFrame *http2.MetaHeadersFrame + endStreamFlag http2.Flags wantStatus *status.Status }{ { @@ -3164,11 +3165,22 @@ func (s) TestClientTransport_Handle1xxHeaders(t *testing.T) { {Name: ":status", Value: "100"}, }, }, + endStreamFlag: http2.FlagHeadersEndStream, wantStatus: status.New( codes.Internal, - "protocol error: received 1xx informational header with END_STREAM set: 100", + "protocol error: informational header with status code 100 must not have END_STREAM set", ), }, + { + name: "1xx without END_STREAM is ignored", + metaHeaderFrame: &http2.MetaHeadersFrame{ + Fields: []hpack.HeaderField{ + {Name: ":status", Value: "100"}, + }, + }, + endStreamFlag: 0, + wantStatus: nil, + }, } { t.Run(test.name, func(t *testing.T) { ts := testStream() @@ -3177,7 +3189,7 @@ func (s) TestClientTransport_Handle1xxHeaders(t *testing.T) { test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{ FrameHeader: http2.FrameHeader{ StreamID: 0, - Flags: http2.FlagHeadersEndStream, + Flags: test.endStreamFlag, }, } From dc3b201309593b805a1db29a7a0bcc533ad2b28f Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Fri, 29 Aug 2025 07:42:13 +0000 Subject: [PATCH 6/6] small tweaks --- internal/transport/transport_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/transport/transport_test.go b/internal/transport/transport_test.go index d5b96fa60866..d0c8a88d6abf 100644 --- a/internal/transport/transport_test.go +++ b/internal/transport/transport_test.go @@ -3155,7 +3155,7 @@ func (s) TestClientTransport_Handle1xxHeaders(t *testing.T) { for _, test := range []struct { name string metaHeaderFrame *http2.MetaHeadersFrame - endStreamFlag http2.Flags + httpFlags http2.Flags wantStatus *status.Status }{ { @@ -3165,7 +3165,7 @@ func (s) TestClientTransport_Handle1xxHeaders(t *testing.T) { {Name: ":status", Value: "100"}, }, }, - endStreamFlag: http2.FlagHeadersEndStream, + httpFlags: http2.FlagHeadersEndStream, wantStatus: status.New( codes.Internal, "protocol error: informational header with status code 100 must not have END_STREAM set", @@ -3178,8 +3178,8 @@ func (s) TestClientTransport_Handle1xxHeaders(t *testing.T) { {Name: ":status", Value: "100"}, }, }, - endStreamFlag: 0, - wantStatus: nil, + httpFlags: 0, + wantStatus: nil, }, } { t.Run(test.name, func(t *testing.T) { @@ -3189,7 +3189,7 @@ func (s) TestClientTransport_Handle1xxHeaders(t *testing.T) { test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{ FrameHeader: http2.FrameHeader{ StreamID: 0, - Flags: test.endStreamFlag, + Flags: test.httpFlags, }, } @@ -3199,7 +3199,7 @@ func (s) TestClientTransport_Handle1xxHeaders(t *testing.T) { want := test.wantStatus if got.Code() != want.Code() || got.Message() != want.Message() { - t.Fatalf("operateHeaders(%v); status = \ngot: %v\nwant: %v", test.metaHeaderFrame, got, want) + t.Fatalf("operateHeaders(%v); status = %v, want %v", test.metaHeaderFrame, got, want) } }) }