From be858f0b3439363b6af8804f0f7a5c6ec4f47c71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Mon, 8 Jul 2024 20:50:20 +0000 Subject: [PATCH 01/15] create grpcframer package --- internal/transport/grpcframer/errors.go | 40 ++++ internal/transport/grpcframer/framer.go | 267 ++++++++++++++++++++++++ 2 files changed, 307 insertions(+) create mode 100644 internal/transport/grpcframer/errors.go create mode 100644 internal/transport/grpcframer/framer.go diff --git a/internal/transport/grpcframer/errors.go b/internal/transport/grpcframer/errors.go new file mode 100644 index 000000000000..86b0d79459e4 --- /dev/null +++ b/internal/transport/grpcframer/errors.go @@ -0,0 +1,40 @@ +/* + * + * Copyright 2024 gRPC authors. + * + * 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 grpcframer + +// ErrorCode represents an HTTP/2 Error Code. +// See https://httpwg.org/specs/rfc7540.html#ErrorCodes. +type ErrorCode uint32 + +const ( + NoError ErrorCode = 0x0 + ProtocolError ErrorCode = 0x1 + InternalError ErrorCode = 0x2 + FlowControlError ErrorCode = 0x3 + SettingsTimeout ErrorCode = 0x4 + StreamClosed ErrorCode = 0x5 + FrameSizeError ErrorCode = 0x6 + RefusedStream ErrorCode = 0x7 + Cancel ErrorCode = 0x8 + CompressionError ErrorCode = 0x9 + ConnectError ErrorCode = 0xa + EnhanceYourCalm ErrorCode = 0xb + IndaequateSecurity ErrorCode = 0xc + HTTP11Required ErrorCode = 0xd +) diff --git a/internal/transport/grpcframer/framer.go b/internal/transport/grpcframer/framer.go new file mode 100644 index 000000000000..23c1c7c895cd --- /dev/null +++ b/internal/transport/grpcframer/framer.go @@ -0,0 +1,267 @@ +/* + * + * Copyright 2024 gRPC authors. + * + * 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 grpcframer defines the interface for implementing an HTTP/2 framer, +// its required types and an implementation of said framer. +package grpcframer + +import "sync" + +// FrameType represents the type of a Frame and its value according to the +// HTTP/2 spec. +// See https://httpwg.org/specs/rfc7540.html#FrameTypes. +type FrameType uint8 + +const ( + DataFrameType FrameType = 0x0 + HeadersFrameType FrameType = 0x1 + PriorityFrameType FrameType = 0x2 + RSTStreamFrameType FrameType = 0x3 + SettingsFrameType FrameType = 0x4 + PushPromiseFrameType FrameType = 0x5 + PingFrameType FrameType = 0x6 + GoAwayFrameType FrameType = 0x7 + WindowUpdateFrameType FrameType = 0x8 + ContinuationFrameType FrameType = 0x9 +) + +// Flags represents the flags that can be set on every frame type. +type Flags uint8 + +const ( + // Data Frame + FlagDataEndStream Flags = 0x1 + FlagDataPadded Flags = 0x8 + + // Headers Frame + FlagHeadersEndStream Flags = 0x1 + FlagHeadersEndHeaders Flags = 0x4 + FlagHeadersPadded Flags = 0x8 + FlagHeadersPriority Flags = 0x20 + + // Settings Frame + FlagSettingsAck Flags = 0x1 + + // Ping Frame + FlagPingAck Flags = 0x1 + + // Continuation Frame + FlagContinuationEndHeaders Flags = 0x4 + + FlagPushPromiseEndHeaders Flags = 0x4 + FlagPushPromisePadded Flags = 0x8 +) + +// Setting represents the id and value pair of a particular HTTP/2 setting. +// See https://httpwg.org/specs/rfc7540.html#SettingFormat. +type Setting struct { + ID SettingID + Value uint32 +} + +// SettingID represents the id of a specific HTTP/2 setting. +// See https://httpwg.org/specs/rfc7540.html#SettingValues. +type SettingID uint16 + +const ( + SettingsHeaderTableSize SettingID = 0x1 + SettingsEnablePush SettingID = 0x2 + SettingsMaxConcurrentStreams SettingID = 0x3 + SettingsInitialWindowSize SettingID = 0x4 + SettingsMaxFrameSize SettingID = 0x5 + SettingsMaxHeaderListSize SettingID = 0x6 +) + +// FrameHeader is the 9 byte header of any HTTP/2 Frame. +// See https://httpwg.org/specs/rfc7540.html#FrameHeader. +type FrameHeader struct { + // Size is the size of the frame's payload without the 9 header bytes. + // As per the HTTP/2 spec, size can be up to 3 bytes, but only frames + // up to 16KB can be processed without agreement. + Size uint32 + + // Type is a byte that represents the Frame Type. The HTTP/2 spec + // defines 10 standard types but extension frames may be written. + Type FrameType + + // Flags is a byte representing the flags set for the specific frame + // type. + Flags Flags + + // StreamID is the ID for the stream which this frame is for. If the + // frame is connection specific instead of stream specific, the + // streamID is 0. + StreamID uint32 +} + +// Frame represents any kind of HTTP/2 Frame, which can be casted into +// its corresponding type using the Header method provided. +type Frame interface { + // Header returns this frame's Header. + Header() FrameHeader +} + +type DataFrame struct { + hdr FrameHeader + pool *sync.Pool + Data []byte +} + +func (f *DataFrame) Header() FrameHeader { + return f.hdr +} + +func (f *DataFrame) Free() { + if f.Data == nil { + return + } + + f.pool.Put(&f.Data) + f.Data = nil +} + +type HeadersFrame struct { + hdr FrameHeader + pool *sync.Pool + HdrBlock []byte +} + +func (f *HeadersFrame) Header() FrameHeader { + return f.hdr +} + +func (f *HeadersFrame) Free() { + if f.HdrBlock == nil { + return + } + + f.pool.Put(&f.HdrBlock) + f.HdrBlock = nil +} + +type RSTStreamFrame struct { + hdr FrameHeader + Code ErrorCode +} + +func (f *RSTStreamFrame) Header() FrameHeader { + return f.hdr +} + +type SettingsFrame struct { + hdr FrameHeader + pool *sync.Pool + settings []byte +} + +func (f *SettingsFrame) Header() FrameHeader { + return f.hdr +} + +func (f *SettingsFrame) Free() { + if f.settings == nil { + return + } + + f.pool.Put(&f.settings) + f.settings = nil +} + +type PingFrame struct { + hdr FrameHeader + pool *sync.Pool + Data []byte +} + +func (f *PingFrame) Header() FrameHeader { + return f.hdr +} + +func (f *PingFrame) Free() { + if f.Data == nil { + return + } + + f.pool.Put(&f.Data) + f.Data = nil +} + +type GoAwayFrame struct { + hdr FrameHeader + pool *sync.Pool + LastStreamID uint32 + Code ErrorCode + DebugData []byte +} + +func (f *GoAwayFrame) Header() FrameHeader { + return f.hdr +} + +func (f *GoAwayFrame) Free() { + if f.DebugData == nil { + return + } + + f.pool.Put(&f.DebugData) + f.DebugData = nil +} + +type WindowUpdateFrame struct { + hdr FrameHeader + Inc uint32 +} + +func (f *WindowUpdateFrame) Header() FrameHeader { + return f.hdr +} + +type ContinuationFrame struct { + hdr FrameHeader + pool *sync.Pool + HdrBlock []byte +} + +func (f *ContinuationFrame) Header() FrameHeader { + return f.hdr +} + +func (f *ContinuationFrame) Free() { + if f.HdrBlock == nil { + return + } + + f.pool.Put(&f.HdrBlock) + f.HdrBlock = nil +} + +// GRPCFramer represents the methods a framer must implement to be used in gRPC-Go. +type GRPCFramer interface { + // ReadFrame returns an HTTP/2 Frame. It is the caller's responsibility to + // free the frame once it is done using it. + ReadFrame() (Frame, error) + WriteData(streamID uint32, endStream bool, data ...[]byte) error + WriteHeaders(streamID uint32, endStream, endHeaders bool, headerBlock ...[]byte) error + WriteRSTStream(streamID uint32, code ErrorCode) error + WriteSettings(settings ...Setting) error + WriteSettingsAck() error + WritePing(ack bool, data [8]byte) error + WriteGoAway(maxStreamID uint32, code ErrorCode, debugData []byte) error + WriteWindowUpdate(streamID, inc uint32) error + WriteContinuation(streamID uint32, endHeaders bool, headerBlock ...[]byte) error +} From a77e5d52347c25b67cda728bf1cfd5c01352eadb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Wed, 10 Jul 2024 18:23:53 +0000 Subject: [PATCH 02/15] change file location to new package name --- .../{grpcframer => grpchttp2}/errors.go | 2 +- .../{grpcframer => grpchttp2}/framer.go | 149 +++++++----------- 2 files changed, 59 insertions(+), 92 deletions(-) rename internal/transport/{grpcframer => grpchttp2}/errors.go (98%) rename internal/transport/{grpcframer => grpchttp2}/framer.go (59%) diff --git a/internal/transport/grpcframer/errors.go b/internal/transport/grpchttp2/errors.go similarity index 98% rename from internal/transport/grpcframer/errors.go rename to internal/transport/grpchttp2/errors.go index 86b0d79459e4..bef86224ec5a 100644 --- a/internal/transport/grpcframer/errors.go +++ b/internal/transport/grpchttp2/errors.go @@ -16,7 +16,7 @@ * */ -package grpcframer +package grpchttp2 // ErrorCode represents an HTTP/2 Error Code. // See https://httpwg.org/specs/rfc7540.html#ErrorCodes. diff --git a/internal/transport/grpcframer/framer.go b/internal/transport/grpchttp2/framer.go similarity index 59% rename from internal/transport/grpcframer/framer.go rename to internal/transport/grpchttp2/framer.go index 23c1c7c895cd..0a631d9891ea 100644 --- a/internal/transport/grpcframer/framer.go +++ b/internal/transport/grpchttp2/framer.go @@ -16,66 +16,56 @@ * */ -// Package grpcframer defines the interface for implementing an HTTP/2 framer, -// its required types and an implementation of said framer. -package grpcframer +// Package grpchttp2 defines HTTP/2 types and a framer API and implementation. +package grpchttp2 -import "sync" - -// FrameType represents the type of a Frame and its value according to the -// HTTP/2 spec. -// See https://httpwg.org/specs/rfc7540.html#FrameTypes. +// FrameType represents the type of an HTTP/2 Frame. +// See [Frame Type]. +// +// [Frame Type]: https://httpwg.org/specs/rfc7540.html#FrameType type FrameType uint8 const ( - DataFrameType FrameType = 0x0 - HeadersFrameType FrameType = 0x1 - PriorityFrameType FrameType = 0x2 - RSTStreamFrameType FrameType = 0x3 - SettingsFrameType FrameType = 0x4 - PushPromiseFrameType FrameType = 0x5 - PingFrameType FrameType = 0x6 - GoAwayFrameType FrameType = 0x7 - WindowUpdateFrameType FrameType = 0x8 - ContinuationFrameType FrameType = 0x9 + FrameTypeData FrameType = 0x0 + FrameTypeHeaders FrameType = 0x1 + FrameTypePriority FrameType = 0x2 + FrameTypeRSTStream FrameType = 0x3 + FrameTypeSettings FrameType = 0x4 + FrameTypePushPromise FrameType = 0x5 + FrameTypePing FrameType = 0x6 + FrameTypeGoAway FrameType = 0x7 + FrameTypeWindowUpdate FrameType = 0x8 + FrameTypeContinuation FrameType = 0x9 ) // Flags represents the flags that can be set on every frame type. type Flags uint8 const ( - // Data Frame - FlagDataEndStream Flags = 0x1 - FlagDataPadded Flags = 0x8 - - // Headers Frame - FlagHeadersEndStream Flags = 0x1 - FlagHeadersEndHeaders Flags = 0x4 - FlagHeadersPadded Flags = 0x8 - FlagHeadersPriority Flags = 0x20 - - // Settings Frame - FlagSettingsAck Flags = 0x1 - - // Ping Frame - FlagPingAck Flags = 0x1 - - // Continuation Frame + FlagDataEndStream Flags = 0x1 + FlagDataPadded Flags = 0x8 + FlagHeadersEndStream Flags = 0x1 + FlagHeadersEndHeaders Flags = 0x4 + FlagHeadersPadded Flags = 0x8 + FlagHeadersPriority Flags = 0x20 + FlagSettingsAck Flags = 0x1 + FlagPingAck Flags = 0x1 FlagContinuationEndHeaders Flags = 0x4 - - FlagPushPromiseEndHeaders Flags = 0x4 - FlagPushPromisePadded Flags = 0x8 ) -// Setting represents the id and value pair of a particular HTTP/2 setting. -// See https://httpwg.org/specs/rfc7540.html#SettingFormat. +// Setting represents the id and value pair of an HTTP/2 setting. +// See [Setting Format]. +// +// [Setting Format]: https://httpwg.org/specs/rfc7540.html#SettingFormat type Setting struct { ID SettingID Value uint32 } -// SettingID represents the id of a specific HTTP/2 setting. -// See https://httpwg.org/specs/rfc7540.html#SettingValues. +// SettingID represents the id of an HTTP/2 setting. +// See [Setting Values]. +// +// [Setting Values]: https://httpwg.org/specs/rfc7540.html#SettingValues type SettingID uint16 const ( @@ -88,37 +78,32 @@ const ( ) // FrameHeader is the 9 byte header of any HTTP/2 Frame. -// See https://httpwg.org/specs/rfc7540.html#FrameHeader. +// See [Frame Header]. +// +// [Frame Header]: https://httpwg.org/specs/rfc7540.html#FrameHeader type FrameHeader struct { // Size is the size of the frame's payload without the 9 header bytes. // As per the HTTP/2 spec, size can be up to 3 bytes, but only frames // up to 16KB can be processed without agreement. Size uint32 - - // Type is a byte that represents the Frame Type. The HTTP/2 spec - // defines 10 standard types but extension frames may be written. + // Type is a byte that represents the Frame Type. Type FrameType - - // Flags is a byte representing the flags set for the specific frame - // type. + // Flags is a byte representing the flags set on this Frame. Flags Flags - // StreamID is the ID for the stream which this frame is for. If the // frame is connection specific instead of stream specific, the // streamID is 0. StreamID uint32 } -// Frame represents any kind of HTTP/2 Frame, which can be casted into -// its corresponding type using the Header method provided. +// Frame represents an HTTP/2 Frame. type Frame interface { - // Header returns this frame's Header. Header() FrameHeader } type DataFrame struct { hdr FrameHeader - pool *sync.Pool + free func([]byte) Data []byte } @@ -127,17 +112,14 @@ func (f *DataFrame) Header() FrameHeader { } func (f *DataFrame) Free() { - if f.Data == nil { - return + if f.free != nil { + f.free(f.Data) } - - f.pool.Put(&f.Data) - f.Data = nil } type HeadersFrame struct { hdr FrameHeader - pool *sync.Pool + free func([]byte) HdrBlock []byte } @@ -146,12 +128,9 @@ func (f *HeadersFrame) Header() FrameHeader { } func (f *HeadersFrame) Free() { - if f.HdrBlock == nil { - return + if f.free != nil { + f.free(f.HdrBlock) } - - f.pool.Put(&f.HdrBlock) - f.HdrBlock = nil } type RSTStreamFrame struct { @@ -165,7 +144,7 @@ func (f *RSTStreamFrame) Header() FrameHeader { type SettingsFrame struct { hdr FrameHeader - pool *sync.Pool + free func([]byte) settings []byte } @@ -174,17 +153,14 @@ func (f *SettingsFrame) Header() FrameHeader { } func (f *SettingsFrame) Free() { - if f.settings == nil { - return + if f.free != nil { + f.free(f.settings) } - - f.pool.Put(&f.settings) - f.settings = nil } type PingFrame struct { hdr FrameHeader - pool *sync.Pool + free func([]byte) Data []byte } @@ -193,17 +169,14 @@ func (f *PingFrame) Header() FrameHeader { } func (f *PingFrame) Free() { - if f.Data == nil { - return + if f.free != nil { + f.free(f.Data) } - - f.pool.Put(&f.Data) - f.Data = nil } type GoAwayFrame struct { hdr FrameHeader - pool *sync.Pool + free func([]byte) LastStreamID uint32 Code ErrorCode DebugData []byte @@ -214,12 +187,9 @@ func (f *GoAwayFrame) Header() FrameHeader { } func (f *GoAwayFrame) Free() { - if f.DebugData == nil { - return + if f.free != nil { + f.free(f.DebugData) } - - f.pool.Put(&f.DebugData) - f.DebugData = nil } type WindowUpdateFrame struct { @@ -233,7 +203,7 @@ func (f *WindowUpdateFrame) Header() FrameHeader { type ContinuationFrame struct { hdr FrameHeader - pool *sync.Pool + free func([]byte) HdrBlock []byte } @@ -242,16 +212,13 @@ func (f *ContinuationFrame) Header() FrameHeader { } func (f *ContinuationFrame) Free() { - if f.HdrBlock == nil { - return + if f.free != nil { + f.free(f.HdrBlock) } - - f.pool.Put(&f.HdrBlock) - f.HdrBlock = nil } -// GRPCFramer represents the methods a framer must implement to be used in gRPC-Go. -type GRPCFramer interface { +// Framer represents a Framer used in gRPC-Go. +type Framer interface { // ReadFrame returns an HTTP/2 Frame. It is the caller's responsibility to // free the frame once it is done using it. ReadFrame() (Frame, error) From 070d347c652f531033fe5758aae2d959e38d1179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Wed, 10 Jul 2024 18:25:29 +0000 Subject: [PATCH 03/15] address comments --- internal/transport/grpchttp2/errors.go | 64 +++++++++++++++++++------- internal/transport/grpchttp2/framer.go | 10 ++-- 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/internal/transport/grpchttp2/errors.go b/internal/transport/grpchttp2/errors.go index bef86224ec5a..2f20b6e79585 100644 --- a/internal/transport/grpchttp2/errors.go +++ b/internal/transport/grpchttp2/errors.go @@ -18,23 +18,53 @@ package grpchttp2 -// ErrorCode represents an HTTP/2 Error Code. -// See https://httpwg.org/specs/rfc7540.html#ErrorCodes. -type ErrorCode uint32 +// ErrCode represents an HTTP/2 Error Code. Error codes are 32-bit fields +// that are used in [RST_STREAM] and [GOAWAY] frames to convey the reasons for +// the stream or connection error. See [HTTP/2 Error Code] for definitions of +// each of the following error codes. +// +// [HTTP/2 Error Code]: https://httpwg.org/specs/rfc7540.html#ErrorCodes +// [RST_STREAM]: https://httpwg.org/specs/rfc7540.html#RST_STREAM +// [GOAWAY]: https://httpwg.org/specs/rfc7540.html#GOAWAY +type ErrCode uint32 const ( - NoError ErrorCode = 0x0 - ProtocolError ErrorCode = 0x1 - InternalError ErrorCode = 0x2 - FlowControlError ErrorCode = 0x3 - SettingsTimeout ErrorCode = 0x4 - StreamClosed ErrorCode = 0x5 - FrameSizeError ErrorCode = 0x6 - RefusedStream ErrorCode = 0x7 - Cancel ErrorCode = 0x8 - CompressionError ErrorCode = 0x9 - ConnectError ErrorCode = 0xa - EnhanceYourCalm ErrorCode = 0xb - IndaequateSecurity ErrorCode = 0xc - HTTP11Required ErrorCode = 0xd + ErrCodeNoError ErrCode = 0x0 + ErrCodeProtocol ErrCode = 0x1 + ErrCodeInternal ErrCode = 0x2 + ErrCodeFlowControl ErrCode = 0x3 + ErrCodeSettingsTimeout ErrCode = 0x4 + ErrCodeStreamClosed ErrCode = 0x5 + ErrCodeFrameSize ErrCode = 0x6 + ErrCodeRefusedStream ErrCode = 0x7 + ErrCodeCancel ErrCode = 0x8 + ErrCodeCompression ErrCode = 0x9 + ErrCodeConnect ErrCode = 0xa + ErrCodeEnhanceYourCalm ErrCode = 0xb + ErrCodeIndaequateSecurity ErrCode = 0xc + ErrCodeHTTP11Required ErrCode = 0xd ) + +var errorCodeNames = map[ErrCode]string{ + ErrCodeNoError: "NO_ERROR", + ErrCodeProtocol: "PROTOCOL_ERROR", + ErrCodeInternal: "INTERNAL_ERROR", + ErrCodeFlowControl: "FLOW_CONTROL_ERROR", + ErrCodeSettingsTimeout: "SETTINGS_TIMEOUT", + ErrCodeStreamClosed: "STREAM_CLOSED", + ErrCodeFrameSize: "FRAME_SIZE_ERROR", + ErrCodeRefusedStream: "REFUSED_STREAM", + ErrCodeCancel: "CANCEL", + ErrCodeCompression: "COMPRESSION_ERROR", + ErrCodeConnect: "CONNECT_ERROR", + ErrCodeEnhanceYourCalm: "ENHANCE_YOUR_CALM", + ErrCodeIndaequateSecurity: "INADEQUATE_SECURITY", + ErrCodeHTTP11Required: "HTTP_1_1_REQUIRED", +} + +func (err ErrCode) String() string { + if v, ok := errorCodeNames[err]; ok { + return v + } + return "INTERNAL_ERROR" +} diff --git a/internal/transport/grpchttp2/framer.go b/internal/transport/grpchttp2/framer.go index 0a631d9891ea..a6bcf2ee7db3 100644 --- a/internal/transport/grpchttp2/framer.go +++ b/internal/transport/grpchttp2/framer.go @@ -38,7 +38,7 @@ const ( FrameTypeContinuation FrameType = 0x9 ) -// Flags represents the flags that can be set on every frame type. +// Flags represents one or more flags set on an HTTP/2 Frame. type Flags uint8 const ( @@ -135,7 +135,7 @@ func (f *HeadersFrame) Free() { type RSTStreamFrame struct { hdr FrameHeader - Code ErrorCode + Code ErrCode } func (f *RSTStreamFrame) Header() FrameHeader { @@ -178,7 +178,7 @@ type GoAwayFrame struct { hdr FrameHeader free func([]byte) LastStreamID uint32 - Code ErrorCode + Code ErrCode DebugData []byte } @@ -224,11 +224,11 @@ type Framer interface { ReadFrame() (Frame, error) WriteData(streamID uint32, endStream bool, data ...[]byte) error WriteHeaders(streamID uint32, endStream, endHeaders bool, headerBlock ...[]byte) error - WriteRSTStream(streamID uint32, code ErrorCode) error + WriteRSTStream(streamID uint32, code ErrCode) error WriteSettings(settings ...Setting) error WriteSettingsAck() error WritePing(ack bool, data [8]byte) error - WriteGoAway(maxStreamID uint32, code ErrorCode, debugData []byte) error + WriteGoAway(maxStreamID uint32, code ErrCode, debugData []byte) error WriteWindowUpdate(streamID, inc uint32) error WriteContinuation(streamID uint32, endHeaders bool, headerBlock ...[]byte) error } From 78c4957dafb63e191d27ff8fbbc7bae50e423dd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Wed, 10 Jul 2024 20:05:33 +0000 Subject: [PATCH 04/15] add meta decoder to interface --- internal/transport/grpchttp2/framer.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/transport/grpchttp2/framer.go b/internal/transport/grpchttp2/framer.go index a6bcf2ee7db3..24992b760f99 100644 --- a/internal/transport/grpchttp2/framer.go +++ b/internal/transport/grpchttp2/framer.go @@ -19,6 +19,8 @@ // Package grpchttp2 defines HTTP/2 types and a framer API and implementation. package grpchttp2 +import "golang.org/x/net/http2/hpack" + // FrameType represents the type of an HTTP/2 Frame. // See [Frame Type]. // @@ -219,6 +221,10 @@ func (f *ContinuationFrame) Free() { // Framer represents a Framer used in gRPC-Go. type Framer interface { + // SetMetaDecoder will set a decoder for the framer. When the decoder is + // set, ReadFrame will parse the header values from all Headers and + // Continuation frames. + SetMetaDecoder(d *hpack.Decoder) // ReadFrame returns an HTTP/2 Frame. It is the caller's responsibility to // free the frame once it is done using it. ReadFrame() (Frame, error) From cb355827826a741ae145885a3bcb26682de98091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Wed, 10 Jul 2024 20:06:05 +0000 Subject: [PATCH 05/15] add SetMetaDecoder to interface --- internal/transport/grpchttp2/framer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/transport/grpchttp2/framer.go b/internal/transport/grpchttp2/framer.go index 24992b760f99..166f75753151 100644 --- a/internal/transport/grpchttp2/framer.go +++ b/internal/transport/grpchttp2/framer.go @@ -222,7 +222,7 @@ func (f *ContinuationFrame) Free() { // Framer represents a Framer used in gRPC-Go. type Framer interface { // SetMetaDecoder will set a decoder for the framer. When the decoder is - // set, ReadFrame will parse the header values from all Headers and + // set, ReadFrame will parse the header values, merging all Headers and // Continuation frames. SetMetaDecoder(d *hpack.Decoder) // ReadFrame returns an HTTP/2 Frame. It is the caller's responsibility to From 4a02377b2a0e13ba961ae1f42120876609f161b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Wed, 10 Jul 2024 22:24:33 +0000 Subject: [PATCH 06/15] add MetaHeadersFrame and change struct fields to pointers. --- internal/transport/grpchttp2/framer.go | 76 ++++++++++++++------------ 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/internal/transport/grpchttp2/framer.go b/internal/transport/grpchttp2/framer.go index 166f75753151..9c8837163832 100644 --- a/internal/transport/grpchttp2/framer.go +++ b/internal/transport/grpchttp2/framer.go @@ -100,125 +100,131 @@ type FrameHeader struct { // Frame represents an HTTP/2 Frame. type Frame interface { - Header() FrameHeader + Header() *FrameHeader } type DataFrame struct { - hdr FrameHeader - free func([]byte) + hdr *FrameHeader + free func() Data []byte } -func (f *DataFrame) Header() FrameHeader { +func (f *DataFrame) Header() *FrameHeader { return f.hdr } func (f *DataFrame) Free() { if f.free != nil { - f.free(f.Data) + f.free() } } type HeadersFrame struct { - hdr FrameHeader - free func([]byte) + hdr *FrameHeader + free func() HdrBlock []byte } -func (f *HeadersFrame) Header() FrameHeader { +func (f *HeadersFrame) Header() *FrameHeader { return f.hdr } func (f *HeadersFrame) Free() { if f.free != nil { - f.free(f.HdrBlock) + f.free() } } type RSTStreamFrame struct { - hdr FrameHeader + hdr *FrameHeader Code ErrCode } -func (f *RSTStreamFrame) Header() FrameHeader { +func (f *RSTStreamFrame) Header() *FrameHeader { return f.hdr } type SettingsFrame struct { - hdr FrameHeader - free func([]byte) - settings []byte + hdr *FrameHeader + free func() + settings []Setting } -func (f *SettingsFrame) Header() FrameHeader { +func (f *SettingsFrame) Header() *FrameHeader { return f.hdr } -func (f *SettingsFrame) Free() { - if f.free != nil { - f.free(f.settings) - } -} - type PingFrame struct { - hdr FrameHeader - free func([]byte) + hdr *FrameHeader + free func() Data []byte } -func (f *PingFrame) Header() FrameHeader { +func (f *PingFrame) Header() *FrameHeader { return f.hdr } func (f *PingFrame) Free() { if f.free != nil { - f.free(f.Data) + f.free() } } type GoAwayFrame struct { - hdr FrameHeader - free func([]byte) + hdr *FrameHeader + free func() LastStreamID uint32 Code ErrCode DebugData []byte } -func (f *GoAwayFrame) Header() FrameHeader { +func (f *GoAwayFrame) Header() *FrameHeader { return f.hdr } func (f *GoAwayFrame) Free() { if f.free != nil { - f.free(f.DebugData) + f.free() } } type WindowUpdateFrame struct { - hdr FrameHeader + hdr *FrameHeader Inc uint32 } -func (f *WindowUpdateFrame) Header() FrameHeader { +func (f *WindowUpdateFrame) Header() *FrameHeader { return f.hdr } type ContinuationFrame struct { - hdr FrameHeader - free func([]byte) + hdr *FrameHeader + free func() HdrBlock []byte } -func (f *ContinuationFrame) Header() FrameHeader { +func (f *ContinuationFrame) Header() *FrameHeader { return f.hdr } func (f *ContinuationFrame) Free() { if f.free != nil { - f.free(f.HdrBlock) + f.free() } } +// MetaHeadersFrame is not a Frame Type that appears on the HTTP/2 Spec. It is +// a representation of the merging and decoding of all the Headers and +// Continuation frames on a Stream. +type MetaHeadersFrame struct { + hdr *FrameHeader + Fields []hpack.HeaderField +} + +func (f *MetaHeadersFrame) Header() *FrameHeader { + return f.hdr +} + // Framer represents a Framer used in gRPC-Go. type Framer interface { // SetMetaDecoder will set a decoder for the framer. When the decoder is From 10207e355855120056586fce778a6841a1086cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Thu, 11 Jul 2024 00:16:29 +0000 Subject: [PATCH 07/15] address comments --- internal/transport/grpchttp2/errors.go | 4 +++- internal/transport/grpchttp2/framer.go | 14 ++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/internal/transport/grpchttp2/errors.go b/internal/transport/grpchttp2/errors.go index 2f20b6e79585..e61e19c2b4ab 100644 --- a/internal/transport/grpchttp2/errors.go +++ b/internal/transport/grpchttp2/errors.go @@ -18,6 +18,8 @@ package grpchttp2 +import "fmt" + // ErrCode represents an HTTP/2 Error Code. Error codes are 32-bit fields // that are used in [RST_STREAM] and [GOAWAY] frames to convey the reasons for // the stream or connection error. See [HTTP/2 Error Code] for definitions of @@ -66,5 +68,5 @@ func (err ErrCode) String() string { if v, ok := errorCodeNames[err]; ok { return v } - return "INTERNAL_ERROR" + return fmt.Sprintf("unknown error code 0x%x", uint32(err)) } diff --git a/internal/transport/grpchttp2/framer.go b/internal/transport/grpchttp2/framer.go index 9c8837163832..e9e70527a0db 100644 --- a/internal/transport/grpchttp2/framer.go +++ b/internal/transport/grpchttp2/framer.go @@ -30,10 +30,8 @@ type FrameType uint8 const ( FrameTypeData FrameType = 0x0 FrameTypeHeaders FrameType = 0x1 - FrameTypePriority FrameType = 0x2 FrameTypeRSTStream FrameType = 0x3 FrameTypeSettings FrameType = 0x4 - FrameTypePushPromise FrameType = 0x5 FrameTypePing FrameType = 0x6 FrameTypeGoAway FrameType = 0x7 FrameTypeWindowUpdate FrameType = 0x8 @@ -101,6 +99,9 @@ type FrameHeader struct { // Frame represents an HTTP/2 Frame. type Frame interface { Header() *FrameHeader + // Free frees the underlying buffer if present so it can be reused by the + // framer. + Free() } type DataFrame struct { @@ -144,9 +145,10 @@ func (f *RSTStreamFrame) Header() *FrameHeader { return f.hdr } +func (f *RSTStreamFrame) Free() {} + type SettingsFrame struct { hdr *FrameHeader - free func() settings []Setting } @@ -154,6 +156,8 @@ func (f *SettingsFrame) Header() *FrameHeader { return f.hdr } +func (f *SettingsFrame) Free() {} + type PingFrame struct { hdr *FrameHeader free func() @@ -225,7 +229,9 @@ func (f *MetaHeadersFrame) Header() *FrameHeader { return f.hdr } -// Framer represents a Framer used in gRPC-Go. +func (f *MetaHeadersFrame) Free() {} + +// Framer encapsulates the functionality to read and write HTTP/2 frames. type Framer interface { // SetMetaDecoder will set a decoder for the framer. When the decoder is // set, ReadFrame will parse the header values, merging all Headers and From 5990250908b09056da1d5b7cbf48da6efe25a349 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Thu, 11 Jul 2024 18:00:45 +0000 Subject: [PATCH 08/15] change flags to flag --- internal/transport/grpchttp2/framer.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/transport/grpchttp2/framer.go b/internal/transport/grpchttp2/framer.go index e9e70527a0db..662f4b9e4a24 100644 --- a/internal/transport/grpchttp2/framer.go +++ b/internal/transport/grpchttp2/framer.go @@ -38,19 +38,19 @@ const ( FrameTypeContinuation FrameType = 0x9 ) -// Flags represents one or more flags set on an HTTP/2 Frame. -type Flags uint8 +// Flag represents one or more flags set on an HTTP/2 Frame. +type Flag uint8 const ( - FlagDataEndStream Flags = 0x1 - FlagDataPadded Flags = 0x8 - FlagHeadersEndStream Flags = 0x1 - FlagHeadersEndHeaders Flags = 0x4 - FlagHeadersPadded Flags = 0x8 - FlagHeadersPriority Flags = 0x20 - FlagSettingsAck Flags = 0x1 - FlagPingAck Flags = 0x1 - FlagContinuationEndHeaders Flags = 0x4 + FlagDataEndStream Flag = 0x1 + FlagDataPadded Flag = 0x8 + FlagHeadersEndStream Flag = 0x1 + FlagHeadersEndHeaders Flag = 0x4 + FlagHeadersPadded Flag = 0x8 + FlagHeadersPriority Flag = 0x20 + FlagSettingsAck Flag = 0x1 + FlagPingAck Flag = 0x1 + FlagContinuationEndHeaders Flag = 0x4 ) // Setting represents the id and value pair of an HTTP/2 setting. @@ -89,7 +89,7 @@ type FrameHeader struct { // Type is a byte that represents the Frame Type. Type FrameType // Flags is a byte representing the flags set on this Frame. - Flags Flags + Flags Flag // StreamID is the ID for the stream which this frame is for. If the // frame is connection specific instead of stream specific, the // streamID is 0. From 67e36ff6b55fe1033466b903e4711d6193a69fbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Mon, 15 Jul 2024 21:37:32 +0000 Subject: [PATCH 09/15] make settings an exported field --- internal/transport/grpchttp2/framer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/transport/grpchttp2/framer.go b/internal/transport/grpchttp2/framer.go index 662f4b9e4a24..c1283bd24806 100644 --- a/internal/transport/grpchttp2/framer.go +++ b/internal/transport/grpchttp2/framer.go @@ -149,7 +149,7 @@ func (f *RSTStreamFrame) Free() {} type SettingsFrame struct { hdr *FrameHeader - settings []Setting + Settings []Setting } func (f *SettingsFrame) Header() *FrameHeader { From 610bf633a7d7a71427668c36aafb54c4b2fdff6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Tue, 16 Jul 2024 20:23:34 +0000 Subject: [PATCH 10/15] add documentation to frames --- internal/transport/grpchttp2/framer.go | 69 ++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/internal/transport/grpchttp2/framer.go b/internal/transport/grpchttp2/framer.go index c1283bd24806..c4ccefe95942 100644 --- a/internal/transport/grpchttp2/framer.go +++ b/internal/transport/grpchttp2/framer.go @@ -104,6 +104,11 @@ type Frame interface { Free() } +// DataFrame is the representation of a [DATA frame]. It implements the Frame +// interface. DATA frames convey arbitrary, variable-length sequences of octets +// associated with a stream. +// +// [DATA frame]: https://httpwg.org/specs/rfc7540.html#DATA type DataFrame struct { hdr *FrameHeader free func() @@ -120,6 +125,11 @@ func (f *DataFrame) Free() { } } +// HeadersFrame is the representation of a [HEADERS Frame]. It implements +// the Frame interface. The HEADERS frame is used to open a stream, and +// additionally carries a header block fragment. +// +// [HEADERS Frame]: https://httpwg.org/specs/rfc7540.html#HEADERS type HeadersFrame struct { hdr *FrameHeader free func() @@ -136,6 +146,13 @@ func (f *HeadersFrame) Free() { } } +// RSTStreamFrame is the representation of a [RST_STREAM Frame]. It implements +// the Frame interface. There is no underlying byte array in this frame, so +// Free() is a no-op. +// +// The RST_STREAM frame allows for immediate termination of a stream +// +// [RST_STREAM Frame]: https://httpwg.org/specs/rfc7540.html#RST_STREAM type RSTStreamFrame struct { hdr *FrameHeader Code ErrCode @@ -147,6 +164,14 @@ func (f *RSTStreamFrame) Header() *FrameHeader { func (f *RSTStreamFrame) Free() {} +// SettingsFrame is the representation of a [SETTINGS Frame]. It implements +// the Frame interface. There is no underlying byte array in this frame, so +// Free() is a no-op. +// +// The SETTINGS frame conveys configuration parameters that affect how +// endpoints communicate, such as preferences and constraints on peer behavior. +// +// [SETTINGS Frame]: https://httpwg.org/specs/rfc7540.html#SETTINGS type SettingsFrame struct { hdr *FrameHeader Settings []Setting @@ -158,6 +183,12 @@ func (f *SettingsFrame) Header() *FrameHeader { func (f *SettingsFrame) Free() {} +// PingFrame is the representation of a [PING Frame]. It implements the Frame +// interface. The PING frame is a mechanism for measuring a minimal round-trip +// time from the sender, as well as determining whether an idle connection is +// still functional. +// +// [PING Frame]: https://httpwg.org/specs/rfc7540.html#PING type PingFrame struct { hdr *FrameHeader free func() @@ -174,6 +205,11 @@ func (f *PingFrame) Free() { } } +// GoAwayFrame is the representation of a [GOAWAY Frame]. It implements the +// Frame interface. The GOAWAY frame is used to initiate shutdown of a +// connection or to signal serious error conditions. +// +// [GOAWAY Frame]: https://httpwg.org/specs/rfc7540.html#GOAWAY type GoAwayFrame struct { hdr *FrameHeader free func() @@ -192,6 +228,11 @@ func (f *GoAwayFrame) Free() { } } +// WindowUpdateFrame is the representation of a [WINDOW_UPDATE Frame]. It +// implements the Frame interface. The WINDOW_UPDATE frame is used to implement +// flow control. +// +// [WINDOW_UPDATE Frame]: https://httpwg.org/specs/rfc7540.html#WINDOW_UPDATE type WindowUpdateFrame struct { hdr *FrameHeader Inc uint32 @@ -201,6 +242,11 @@ func (f *WindowUpdateFrame) Header() *FrameHeader { return f.hdr } +// ContinuationFrame is the representation of a [CONTINUATION Frame]. It +// implements the Frame interface. The CONTINUATION frame (type=0x9) is used to +// continue a sequence of header block fragments. +// +// [CONTINUATION Frame]: https://httpwg.org/specs/rfc7540.html#CONTINUATION type ContinuationFrame struct { hdr *FrameHeader free func() @@ -217,12 +263,16 @@ func (f *ContinuationFrame) Free() { } } -// MetaHeadersFrame is not a Frame Type that appears on the HTTP/2 Spec. It is -// a representation of the merging and decoding of all the Headers and -// Continuation frames on a Stream. +// MetaHeadersFrame is the representation of one HEADERS frame and zero or more +// contiguous CONTINUATION frames and the decoding of their HPACK-encoded +// contents. This frame type is not transmitted over the network and is only +// generated by the ReadFrame() function. type MetaHeadersFrame struct { hdr *FrameHeader Fields []hpack.HeaderField + // Truncated indicates whether the MetaHeadersFrame has been truncated due + // to being longer than the MaxHeaderListSize. + Truncated bool } func (f *MetaHeadersFrame) Header() *FrameHeader { @@ -237,16 +287,25 @@ type Framer interface { // set, ReadFrame will parse the header values, merging all Headers and // Continuation frames. SetMetaDecoder(d *hpack.Decoder) - // ReadFrame returns an HTTP/2 Frame. It is the caller's responsibility to - // free the frame once it is done using it. + // ReadFrame returns grpchttp2.Frame. It is the caller's responsibility to + // call Frame.Free() once it is done using it. ReadFrame() (Frame, error) + // WriteData writes an HTTP/2 DATA frame to the stream. WriteData(streamID uint32, endStream bool, data ...[]byte) error + // WriteData writes an HTTP/2 HEADERS frame to the stream. WriteHeaders(streamID uint32, endStream, endHeaders bool, headerBlock ...[]byte) error + // WriteData writes an HTTP/2 RST_STREAM frame to the stream. WriteRSTStream(streamID uint32, code ErrCode) error + // WriteSettings writes an HTTP/2 SETTINGS frame to the connection. WriteSettings(settings ...Setting) error + // WriteSettingsAck writes an HTTP/2 SETTINGS frame with the ACK flag set. WriteSettingsAck() error + // WritePing writes an HTTP/2 PING frame to the connection. WritePing(ack bool, data [8]byte) error + // WriteGoAway writes an HTTP/2 GOAWAY frame to the connection. WriteGoAway(maxStreamID uint32, code ErrCode, debugData []byte) error + // WriteWindowUpdate writes an HTTP/2 WINDOW_UPDATE frame to the stream. WriteWindowUpdate(streamID, inc uint32) error + // WriteContinuation writes an HTTP/2 CONTINUATION frame to the stream. WriteContinuation(streamID uint32, endHeaders bool, headerBlock ...[]byte) error } From 4c2028334d1094b96af96b8a781e4657a3079d30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Wed, 17 Jul 2024 20:34:41 +0000 Subject: [PATCH 11/15] add test and more documentation to frame types --- internal/transport/grpchttp2/errors_test.go | 33 +++++++++ internal/transport/grpchttp2/framer.go | 76 +++++++++++---------- 2 files changed, 74 insertions(+), 35 deletions(-) create mode 100644 internal/transport/grpchttp2/errors_test.go diff --git a/internal/transport/grpchttp2/errors_test.go b/internal/transport/grpchttp2/errors_test.go new file mode 100644 index 000000000000..a02081d0c20a --- /dev/null +++ b/internal/transport/grpchttp2/errors_test.go @@ -0,0 +1,33 @@ +package grpchttp2_test + +import ( + "testing" + + "google.golang.org/grpc/internal/grpctest" + "google.golang.org/grpc/internal/transport/grpchttp2" +) + +type s struct { + grpctest.Tester +} + +func Test(t *testing.T) { + grpctest.RunSubTests(t, s{}) +} + +func (s) TestErrorCodeString(t *testing.T) { + err := grpchttp2.ErrCodeNoError + if err.String() != "NO_ERROR" { + t.Errorf("got %q, want %q", err.String(), "NO_ERROR") + } + + err = grpchttp2.ErrCode(0x1) + if err.String() != "PROTOCOL_ERROR" { + t.Errorf("got %q, want %q", err.String(), "PROTOCOL_ERROR") + } + + err = grpchttp2.ErrCode(0xf) + if err.String() != "unknown error code 0xf" { + t.Errorf("got %q, want %q", err.String(), "unknown error code 0xf") + } +} diff --git a/internal/transport/grpchttp2/framer.go b/internal/transport/grpchttp2/framer.go index c4ccefe95942..b12ae3c7971d 100644 --- a/internal/transport/grpchttp2/framer.go +++ b/internal/transport/grpchttp2/framer.go @@ -96,17 +96,23 @@ type FrameHeader struct { StreamID uint32 } -// Frame represents an HTTP/2 Frame. +// Frame represents an HTTP/2 Frame. This interface struct is only to be used +// on the read path of the Framer. The writing path expects the data to be +// passed individually, not using this type. +// +// Each concrete Frame type defined below implements the Frame interface. type Frame interface { Header() *FrameHeader // Free frees the underlying buffer if present so it can be reused by the // framer. + // + // TODO: Remove method from the interface once the mem package gets merged. + // Free will be called on each mem.Buffer individually. Free() } -// DataFrame is the representation of a [DATA frame]. It implements the Frame -// interface. DATA frames convey arbitrary, variable-length sequences of octets -// associated with a stream. +// DataFrame is the representation of a [DATA frame]. DATA frames convey +// arbitrary, variable-length sequences of octets associated with a stream. // // [DATA frame]: https://httpwg.org/specs/rfc7540.html#DATA type DataFrame struct { @@ -125,9 +131,8 @@ func (f *DataFrame) Free() { } } -// HeadersFrame is the representation of a [HEADERS Frame]. It implements -// the Frame interface. The HEADERS frame is used to open a stream, and -// additionally carries a header block fragment. +// HeadersFrame is the representation of a [HEADERS Frame]. The HEADERS frame +// is used to open a stream, and additionally carries a header block fragment. // // [HEADERS Frame]: https://httpwg.org/specs/rfc7540.html#HEADERS type HeadersFrame struct { @@ -146,11 +151,9 @@ func (f *HeadersFrame) Free() { } } -// RSTStreamFrame is the representation of a [RST_STREAM Frame]. It implements -// the Frame interface. There is no underlying byte array in this frame, so -// Free() is a no-op. -// -// The RST_STREAM frame allows for immediate termination of a stream +// RSTStreamFrame is the representation of a [RST_STREAM Frame]. There is no +// underlying byte array in this frame, so Free() is a no-op. The RST_STREAM +// frame allows for immediate termination of a stream // // [RST_STREAM Frame]: https://httpwg.org/specs/rfc7540.html#RST_STREAM type RSTStreamFrame struct { @@ -164,9 +167,8 @@ func (f *RSTStreamFrame) Header() *FrameHeader { func (f *RSTStreamFrame) Free() {} -// SettingsFrame is the representation of a [SETTINGS Frame]. It implements -// the Frame interface. There is no underlying byte array in this frame, so -// Free() is a no-op. +// SettingsFrame is the representation of a [SETTINGS Frame]. There is no +// underlying byte array in this frame, so Free() is a no-op. // // The SETTINGS frame conveys configuration parameters that affect how // endpoints communicate, such as preferences and constraints on peer behavior. @@ -183,10 +185,9 @@ func (f *SettingsFrame) Header() *FrameHeader { func (f *SettingsFrame) Free() {} -// PingFrame is the representation of a [PING Frame]. It implements the Frame -// interface. The PING frame is a mechanism for measuring a minimal round-trip -// time from the sender, as well as determining whether an idle connection is -// still functional. +// PingFrame is the representation of a [PING Frame]. The PING frame is a +// mechanism for measuring a minimal round-trip time from the sender, as well +// as determining whether an idle connection is still functional. // // [PING Frame]: https://httpwg.org/specs/rfc7540.html#PING type PingFrame struct { @@ -205,9 +206,9 @@ func (f *PingFrame) Free() { } } -// GoAwayFrame is the representation of a [GOAWAY Frame]. It implements the -// Frame interface. The GOAWAY frame is used to initiate shutdown of a -// connection or to signal serious error conditions. +// GoAwayFrame is the representation of a [GOAWAY Frame]. The GOAWAY frame is +// used to initiate shutdown of a connection or to signal serious error +// conditions. // // [GOAWAY Frame]: https://httpwg.org/specs/rfc7540.html#GOAWAY type GoAwayFrame struct { @@ -228,9 +229,8 @@ func (f *GoAwayFrame) Free() { } } -// WindowUpdateFrame is the representation of a [WINDOW_UPDATE Frame]. It -// implements the Frame interface. The WINDOW_UPDATE frame is used to implement -// flow control. +// WindowUpdateFrame is the representation of a [WINDOW_UPDATE Frame]. The +// WINDOW_UPDATE frame is used to implement flow control. // // [WINDOW_UPDATE Frame]: https://httpwg.org/specs/rfc7540.html#WINDOW_UPDATE type WindowUpdateFrame struct { @@ -242,9 +242,8 @@ func (f *WindowUpdateFrame) Header() *FrameHeader { return f.hdr } -// ContinuationFrame is the representation of a [CONTINUATION Frame]. It -// implements the Frame interface. The CONTINUATION frame (type=0x9) is used to -// continue a sequence of header block fragments. +// ContinuationFrame is the representation of a [CONTINUATION Frame]. The +// CONTINUATION frame is used to continue a sequence of header block fragments. // // [CONTINUATION Frame]: https://httpwg.org/specs/rfc7540.html#CONTINUATION type ContinuationFrame struct { @@ -267,6 +266,8 @@ func (f *ContinuationFrame) Free() { // contiguous CONTINUATION frames and the decoding of their HPACK-encoded // contents. This frame type is not transmitted over the network and is only // generated by the ReadFrame() function. +// +// Since there is no underlying buffer in this Frame, Free() is a no-op. type MetaHeadersFrame struct { hdr *FrameHeader Fields []hpack.HeaderField @@ -283,17 +284,18 @@ func (f *MetaHeadersFrame) Free() {} // Framer encapsulates the functionality to read and write HTTP/2 frames. type Framer interface { - // SetMetaDecoder will set a decoder for the framer. When the decoder is - // set, ReadFrame will parse the header values, merging all Headers and - // Continuation frames. - SetMetaDecoder(d *hpack.Decoder) // ReadFrame returns grpchttp2.Frame. It is the caller's responsibility to - // call Frame.Free() once it is done using it. + // call Frame.Free() once it is done using it. Note that once the mem + // package gets merged, this API will change in favor of Buffer.Free(). ReadFrame() (Frame, error) // WriteData writes an HTTP/2 DATA frame to the stream. + // TODO: Once the mem package gets merged, data will change type to + // mem.BufferSlice. WriteData(streamID uint32, endStream bool, data ...[]byte) error // WriteData writes an HTTP/2 HEADERS frame to the stream. - WriteHeaders(streamID uint32, endStream, endHeaders bool, headerBlock ...[]byte) error + // TODO: Once the mem package gets merged, headerBlock will change type to + // mem.Buffer. + WriteHeaders(streamID uint32, endStream, endHeaders bool, headerBlocks []byte) error // WriteData writes an HTTP/2 RST_STREAM frame to the stream. WriteRSTStream(streamID uint32, code ErrCode) error // WriteSettings writes an HTTP/2 SETTINGS frame to the connection. @@ -303,9 +305,13 @@ type Framer interface { // WritePing writes an HTTP/2 PING frame to the connection. WritePing(ack bool, data [8]byte) error // WriteGoAway writes an HTTP/2 GOAWAY frame to the connection. + // TODO: Once the mem package gets merged, debugData will change type to + // mem.Buffer. WriteGoAway(maxStreamID uint32, code ErrCode, debugData []byte) error // WriteWindowUpdate writes an HTTP/2 WINDOW_UPDATE frame to the stream. WriteWindowUpdate(streamID, inc uint32) error // WriteContinuation writes an HTTP/2 CONTINUATION frame to the stream. - WriteContinuation(streamID uint32, endHeaders bool, headerBlock ...[]byte) error + // TODO: Once the mem package gets merged, data will change type to + // mem.Buffer. + WriteContinuation(streamID uint32, endHeaders bool, headerBlock []byte) error } From 6a18aed2452efae7d88687cf4c05d7e4bb21d274 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Wed, 17 Jul 2024 20:37:16 +0000 Subject: [PATCH 12/15] add missing copyright notice --- internal/transport/grpchttp2/errors_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/internal/transport/grpchttp2/errors_test.go b/internal/transport/grpchttp2/errors_test.go index a02081d0c20a..63dfeadd958f 100644 --- a/internal/transport/grpchttp2/errors_test.go +++ b/internal/transport/grpchttp2/errors_test.go @@ -1,3 +1,21 @@ +/* + * + * Copyright 2024 gRPC authors. + * + * 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 grpchttp2_test import ( From cc8bc6b740827a97dd0783d07785c5ed6753d8dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Wed, 17 Jul 2024 21:06:49 +0000 Subject: [PATCH 13/15] change test to include all values --- internal/transport/grpchttp2/errors_test.go | 35 ++++++++++++++------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/internal/transport/grpchttp2/errors_test.go b/internal/transport/grpchttp2/errors_test.go index 63dfeadd958f..3215cae8e3a7 100644 --- a/internal/transport/grpchttp2/errors_test.go +++ b/internal/transport/grpchttp2/errors_test.go @@ -34,18 +34,31 @@ func Test(t *testing.T) { } func (s) TestErrorCodeString(t *testing.T) { - err := grpchttp2.ErrCodeNoError - if err.String() != "NO_ERROR" { - t.Errorf("got %q, want %q", err.String(), "NO_ERROR") + errCodesTest := []struct { + err grpchttp2.ErrCode + want string + }{ + {grpchttp2.ErrCodeNoError, "NO_ERROR"}, + {grpchttp2.ErrCodeProtocol, "PROTOCOL_ERROR"}, + {grpchttp2.ErrCodeInternal, "INTERNAL_ERROR"}, + {grpchttp2.ErrCodeFlowControl, "FLOW_CONTROL_ERROR"}, + {grpchttp2.ErrCodeSettingsTimeout, "SETTINGS_TIMEOUT"}, + {grpchttp2.ErrCodeStreamClosed, "STREAM_CLOSED"}, + {grpchttp2.ErrCodeFrameSize, "FRAME_SIZE_ERROR"}, + {grpchttp2.ErrCodeRefusedStream, "REFUSED_STREAM"}, + {grpchttp2.ErrCodeCancel, "CANCEL"}, + {grpchttp2.ErrCodeCompression, "COMPRESSION_ERROR"}, + {grpchttp2.ErrCodeConnect, "CONNECT_ERROR"}, + {grpchttp2.ErrCodeEnhanceYourCalm, "ENHANCE_YOUR_CALM"}, + {grpchttp2.ErrCodeIndaequateSecurity, "INADEQUATE_SECURITY"}, + {grpchttp2.ErrCodeHTTP11Required, "HTTP_1_1_REQUIRED"}, + {grpchttp2.ErrCode(0x1), "PROTOCOL_ERROR"}, + {grpchttp2.ErrCode(0xf), "unknown error code 0xf"}, } - err = grpchttp2.ErrCode(0x1) - if err.String() != "PROTOCOL_ERROR" { - t.Errorf("got %q, want %q", err.String(), "PROTOCOL_ERROR") - } - - err = grpchttp2.ErrCode(0xf) - if err.String() != "unknown error code 0xf" { - t.Errorf("got %q, want %q", err.String(), "unknown error code 0xf") + for _, errTest := range errCodesTest { + if errTest.err.String() != errTest.want { + t.Errorf("got %q, want %q", errTest.err.String(), errTest.want) + } } } From 93fc8669ae7f0fb1240570a2332cf9cf7443279b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Wed, 17 Jul 2024 21:15:48 +0000 Subject: [PATCH 14/15] move test to same package --- internal/transport/grpchttp2/errors_test.go | 37 ++++++++++----------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/internal/transport/grpchttp2/errors_test.go b/internal/transport/grpchttp2/errors_test.go index 3215cae8e3a7..3ab78823329d 100644 --- a/internal/transport/grpchttp2/errors_test.go +++ b/internal/transport/grpchttp2/errors_test.go @@ -16,13 +16,12 @@ * */ -package grpchttp2_test +package grpchttp2 import ( "testing" "google.golang.org/grpc/internal/grpctest" - "google.golang.org/grpc/internal/transport/grpchttp2" ) type s struct { @@ -35,25 +34,25 @@ func Test(t *testing.T) { func (s) TestErrorCodeString(t *testing.T) { errCodesTest := []struct { - err grpchttp2.ErrCode + err ErrCode want string }{ - {grpchttp2.ErrCodeNoError, "NO_ERROR"}, - {grpchttp2.ErrCodeProtocol, "PROTOCOL_ERROR"}, - {grpchttp2.ErrCodeInternal, "INTERNAL_ERROR"}, - {grpchttp2.ErrCodeFlowControl, "FLOW_CONTROL_ERROR"}, - {grpchttp2.ErrCodeSettingsTimeout, "SETTINGS_TIMEOUT"}, - {grpchttp2.ErrCodeStreamClosed, "STREAM_CLOSED"}, - {grpchttp2.ErrCodeFrameSize, "FRAME_SIZE_ERROR"}, - {grpchttp2.ErrCodeRefusedStream, "REFUSED_STREAM"}, - {grpchttp2.ErrCodeCancel, "CANCEL"}, - {grpchttp2.ErrCodeCompression, "COMPRESSION_ERROR"}, - {grpchttp2.ErrCodeConnect, "CONNECT_ERROR"}, - {grpchttp2.ErrCodeEnhanceYourCalm, "ENHANCE_YOUR_CALM"}, - {grpchttp2.ErrCodeIndaequateSecurity, "INADEQUATE_SECURITY"}, - {grpchttp2.ErrCodeHTTP11Required, "HTTP_1_1_REQUIRED"}, - {grpchttp2.ErrCode(0x1), "PROTOCOL_ERROR"}, - {grpchttp2.ErrCode(0xf), "unknown error code 0xf"}, + {ErrCodeNoError, "NO_ERROR"}, + {ErrCodeProtocol, "PROTOCOL_ERROR"}, + {ErrCodeInternal, "INTERNAL_ERROR"}, + {ErrCodeFlowControl, "FLOW_CONTROL_ERROR"}, + {ErrCodeSettingsTimeout, "SETTINGS_TIMEOUT"}, + {ErrCodeStreamClosed, "STREAM_CLOSED"}, + {ErrCodeFrameSize, "FRAME_SIZE_ERROR"}, + {ErrCodeRefusedStream, "REFUSED_STREAM"}, + {ErrCodeCancel, "CANCEL"}, + {ErrCodeCompression, "COMPRESSION_ERROR"}, + {ErrCodeConnect, "CONNECT_ERROR"}, + {ErrCodeEnhanceYourCalm, "ENHANCE_YOUR_CALM"}, + {ErrCodeIndaequateSecurity, "INADEQUATE_SECURITY"}, + {ErrCodeHTTP11Required, "HTTP_1_1_REQUIRED"}, + {ErrCode(0x1), "PROTOCOL_ERROR"}, + {ErrCode(0xf), "unknown error code 0xf"}, } for _, errTest := range errCodesTest { From e450516a43375279c063badc521223153466fba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Fern=C3=A1ndez=20Alvarado?= Date: Thu, 18 Jul 2024 17:40:29 +0000 Subject: [PATCH 15/15] change tests to comply with style guide --- internal/transport/grpchttp2/errors.go | 2 +- internal/transport/grpchttp2/errors_test.go | 44 +++++++++++---------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/internal/transport/grpchttp2/errors.go b/internal/transport/grpchttp2/errors.go index e61e19c2b4ab..c2aac6bd4f5a 100644 --- a/internal/transport/grpchttp2/errors.go +++ b/internal/transport/grpchttp2/errors.go @@ -68,5 +68,5 @@ func (err ErrCode) String() string { if v, ok := errorCodeNames[err]; ok { return v } - return fmt.Sprintf("unknown error code 0x%x", uint32(err)) + return fmt.Sprintf("unknown error code %#x", uint32(err)) } diff --git a/internal/transport/grpchttp2/errors_test.go b/internal/transport/grpchttp2/errors_test.go index 3ab78823329d..133efdd2ffa8 100644 --- a/internal/transport/grpchttp2/errors_test.go +++ b/internal/transport/grpchttp2/errors_test.go @@ -33,31 +33,35 @@ func Test(t *testing.T) { } func (s) TestErrorCodeString(t *testing.T) { - errCodesTest := []struct { + tests := []struct { err ErrCode want string }{ - {ErrCodeNoError, "NO_ERROR"}, - {ErrCodeProtocol, "PROTOCOL_ERROR"}, - {ErrCodeInternal, "INTERNAL_ERROR"}, - {ErrCodeFlowControl, "FLOW_CONTROL_ERROR"}, - {ErrCodeSettingsTimeout, "SETTINGS_TIMEOUT"}, - {ErrCodeStreamClosed, "STREAM_CLOSED"}, - {ErrCodeFrameSize, "FRAME_SIZE_ERROR"}, - {ErrCodeRefusedStream, "REFUSED_STREAM"}, - {ErrCodeCancel, "CANCEL"}, - {ErrCodeCompression, "COMPRESSION_ERROR"}, - {ErrCodeConnect, "CONNECT_ERROR"}, - {ErrCodeEnhanceYourCalm, "ENHANCE_YOUR_CALM"}, - {ErrCodeIndaequateSecurity, "INADEQUATE_SECURITY"}, - {ErrCodeHTTP11Required, "HTTP_1_1_REQUIRED"}, - {ErrCode(0x1), "PROTOCOL_ERROR"}, - {ErrCode(0xf), "unknown error code 0xf"}, + // Known error cases + {err: ErrCodeNoError, want: "NO_ERROR"}, + {err: ErrCodeProtocol, want: "PROTOCOL_ERROR"}, + {err: ErrCodeInternal, want: "INTERNAL_ERROR"}, + {err: ErrCodeFlowControl, want: "FLOW_CONTROL_ERROR"}, + {err: ErrCodeSettingsTimeout, want: "SETTINGS_TIMEOUT"}, + {err: ErrCodeStreamClosed, want: "STREAM_CLOSED"}, + {err: ErrCodeFrameSize, want: "FRAME_SIZE_ERROR"}, + {err: ErrCodeRefusedStream, want: "REFUSED_STREAM"}, + {err: ErrCodeCancel, want: "CANCEL"}, + {err: ErrCodeCompression, want: "COMPRESSION_ERROR"}, + {err: ErrCodeConnect, want: "CONNECT_ERROR"}, + {err: ErrCodeEnhanceYourCalm, want: "ENHANCE_YOUR_CALM"}, + {err: ErrCodeIndaequateSecurity, want: "INADEQUATE_SECURITY"}, + {err: ErrCodeHTTP11Required, want: "HTTP_1_1_REQUIRED"}, + // Type casting known error case + {err: ErrCode(0x1), want: "PROTOCOL_ERROR"}, + // Unknown error case + {err: ErrCode(0xf), want: "unknown error code 0xf"}, } - for _, errTest := range errCodesTest { - if errTest.err.String() != errTest.want { - t.Errorf("got %q, want %q", errTest.err.String(), errTest.want) + for _, test := range tests { + got := test.err.String() + if got != test.want { + t.Errorf("ErrCode.String(%#x) = %q, want %q", int(test.err), got, test.want) } } }