Skip to content

Commit a95a5c3

Browse files
authored
transport: remove decodeState from client to reduce allocations (#3313)
1 parent 62adda2 commit a95a5c3

File tree

2 files changed

+130
-21
lines changed

2 files changed

+130
-21
lines changed

internal/transport/http2_client.go

Lines changed: 108 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,11 +1254,97 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
12541254
return
12551255
}
12561256

1257-
state := &decodeState{}
1258-
// Initialize isGRPC value to be !initialHeader, since if a gRPC Response-Headers has already been received, then it means that the peer is speaking gRPC and we are in gRPC mode.
1259-
state.data.isGRPC = !initialHeader
1260-
if h2code, err := state.decodeHeader(frame); err != nil {
1261-
t.closeStream(s, err, true, h2code, status.Convert(err), nil, endStream)
1257+
// frame.Truncated is set to true when framer detects that the current header
1258+
// list size hits MaxHeaderListSize limit.
1259+
if frame.Truncated {
1260+
se := status.New(codes.Internal, "peer header list size exceeded limit")
1261+
t.closeStream(s, se.Err(), true, http2.ErrCodeFrameSize, se, nil, endStream)
1262+
return
1263+
}
1264+
1265+
var (
1266+
// If a gRPC Response-Headers has already been received, then it means
1267+
// that the peer is speaking gRPC and we are in gRPC mode.
1268+
isGRPC = !initialHeader
1269+
mdata = make(map[string][]string)
1270+
contentTypeErr string
1271+
grpcMessage string
1272+
statusGen *status.Status
1273+
1274+
httpStatus string
1275+
rawStatus string
1276+
// headerError is set if an error is encountered while parsing the headers
1277+
headerError string
1278+
)
1279+
1280+
for _, hf := range frame.Fields {
1281+
switch hf.Name {
1282+
case "content-type":
1283+
if _, validContentType := grpcutil.ContentSubtype(hf.Value); !validContentType {
1284+
contentTypeErr = fmt.Sprintf("transport: received the unexpected content-type %q", hf.Value)
1285+
break
1286+
}
1287+
mdata[hf.Name] = append(mdata[hf.Name], hf.Value)
1288+
isGRPC = true
1289+
case "grpc-encoding":
1290+
s.recvCompress = hf.Value
1291+
case "grpc-status":
1292+
rawStatus = hf.Value
1293+
case "grpc-message":
1294+
grpcMessage = decodeGrpcMessage(hf.Value)
1295+
case "grpc-status-details-bin":
1296+
var err error
1297+
statusGen, err = decodeGRPCStatusDetails(hf.Value)
1298+
if err != nil {
1299+
headerError = fmt.Sprintf("transport: malformed grpc-status-details-bin: %v", err)
1300+
}
1301+
case ":status":
1302+
httpStatus = hf.Value
1303+
default:
1304+
if isReservedHeader(hf.Name) && !isWhitelistedHeader(hf.Name) {
1305+
break
1306+
}
1307+
v, err := decodeMetadataHeader(hf.Name, hf.Value)
1308+
if err != nil {
1309+
headerError = fmt.Sprintf("transport: malformed %s: %v", hf.Name, err)
1310+
logger.Warningf("Failed to decode metadata header (%q, %q): %v", hf.Name, hf.Value, err)
1311+
break
1312+
}
1313+
mdata[hf.Name] = append(mdata[hf.Name], v)
1314+
}
1315+
}
1316+
1317+
if !isGRPC {
1318+
var (
1319+
code = codes.Internal // when header does not include HTTP status, return INTERNAL
1320+
httpStatusCode int
1321+
)
1322+
1323+
if httpStatus != "" {
1324+
c, err := strconv.ParseInt(httpStatus, 10, 32)
1325+
if err != nil {
1326+
se := status.New(codes.Internal, fmt.Sprintf("transport: malformed http-status: %v", err))
1327+
t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
1328+
return
1329+
}
1330+
httpStatusCode = int(c)
1331+
1332+
var ok bool
1333+
code, ok = HTTPStatusConvTab[httpStatusCode]
1334+
if !ok {
1335+
code = codes.Unknown
1336+
}
1337+
}
1338+
1339+
// Verify the HTTP response is a 200.
1340+
se := status.New(code, constructHTTPErrMsg(&httpStatusCode, contentTypeErr))
1341+
t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
1342+
return
1343+
}
1344+
1345+
if headerError != "" {
1346+
se := status.New(codes.Internal, headerError)
1347+
t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
12621348
return
12631349
}
12641350

@@ -1293,9 +1379,8 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
12931379
// These values can be set without any synchronization because
12941380
// stream goroutine will read it only after seeing a closed
12951381
// headerChan which we'll close after setting this.
1296-
s.recvCompress = state.data.encoding
1297-
if len(state.data.mdata) > 0 {
1298-
s.header = state.data.mdata
1382+
if len(mdata) > 0 {
1383+
s.header = mdata
12991384
}
13001385
} else {
13011386
// HEADERS frame block carries a Trailers-Only.
@@ -1308,9 +1393,23 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
13081393
return
13091394
}
13101395

1396+
if statusGen == nil {
1397+
rawStatusCode := codes.Unknown
1398+
if rawStatus != "" {
1399+
code, err := strconv.ParseInt(rawStatus, 10, 32)
1400+
if err != nil {
1401+
se := status.New(codes.Internal, fmt.Sprintf("transport: malformed grpc-status: %v", err))
1402+
t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
1403+
return
1404+
}
1405+
rawStatusCode = codes.Code(uint32(code))
1406+
}
1407+
statusGen = status.New(rawStatusCode, grpcMessage)
1408+
}
1409+
13111410
// if client received END_STREAM from server while stream was still active, send RST_STREAM
13121411
rst := s.getState() == streamActive
1313-
t.closeStream(s, io.EOF, rst, http2.ErrCodeNo, state.status(), state.data.mdata, true)
1412+
t.closeStream(s, io.EOF, rst, http2.ErrCodeNo, statusGen, mdata, true)
13141413
}
13151414

13161415
// reader runs as a separate goroutine in charge of reading data from network

internal/transport/http_util.go

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -180,14 +180,6 @@ func isWhitelistedHeader(hdr string) bool {
180180
}
181181
}
182182

183-
func (d *decodeState) status() *status.Status {
184-
if d.data.statusGen == nil {
185-
// No status-details were provided; generate status using code/msg.
186-
d.data.statusGen = status.New(codes.Code(int32(*(d.data.rawStatusCode))), d.data.rawStatusMsg)
187-
}
188-
return d.data.statusGen
189-
}
190-
191183
const binHdrSuffix = "-bin"
192184

193185
func encodeBinHeader(v []byte) string {
@@ -217,6 +209,18 @@ func decodeMetadataHeader(k, v string) (string, error) {
217209
return v, nil
218210
}
219211

212+
func decodeGRPCStatusDetails(rawDetails string) (*status.Status, error) {
213+
v, err := decodeBinHeader(rawDetails)
214+
if err != nil {
215+
return nil, err
216+
}
217+
st := &spb.Status{}
218+
if err = proto.Unmarshal(v, st); err != nil {
219+
return nil, err
220+
}
221+
return status.FromProto(st), nil
222+
}
223+
220224
func (d *decodeState) decodeHeader(frame *http2.MetaHeadersFrame) (http2.ErrCode, error) {
221225
// frame.Truncated is set to true when framer detects that the current header
222226
// list size hits MaxHeaderListSize limit.
@@ -271,18 +275,24 @@ func (d *decodeState) decodeHeader(frame *http2.MetaHeadersFrame) (http2.ErrCode
271275
// constructErrMsg constructs error message to be returned in HTTP fallback mode.
272276
// Format: HTTP status code and its corresponding message + content-type error message.
273277
func (d *decodeState) constructHTTPErrMsg() string {
278+
return constructHTTPErrMsg(d.data.httpStatus, d.data.contentTypeErr)
279+
}
280+
281+
// constructErrMsg constructs error message to be returned in HTTP fallback mode.
282+
// Format: HTTP status code and its corresponding message + content-type error message.
283+
func constructHTTPErrMsg(httpStatus *int, contentTypeErr string) string {
274284
var errMsgs []string
275285

276-
if d.data.httpStatus == nil {
286+
if httpStatus == nil {
277287
errMsgs = append(errMsgs, "malformed header: missing HTTP status")
278288
} else {
279-
errMsgs = append(errMsgs, fmt.Sprintf("%s: HTTP status code %d", http.StatusText(*(d.data.httpStatus)), *d.data.httpStatus))
289+
errMsgs = append(errMsgs, fmt.Sprintf("%s: HTTP status code %d", http.StatusText(*(httpStatus)), *httpStatus))
280290
}
281291

282-
if d.data.contentTypeErr == "" {
292+
if contentTypeErr == "" {
283293
errMsgs = append(errMsgs, "transport: missing content-type field")
284294
} else {
285-
errMsgs = append(errMsgs, d.data.contentTypeErr)
295+
errMsgs = append(errMsgs, contentTypeErr)
286296
}
287297

288298
return strings.Join(errMsgs, "; ")

0 commit comments

Comments
 (0)