Skip to content

Commit 41d5c22

Browse files
committed
address arjun's comment
1 parent d569359 commit 41d5c22

2 files changed

Lines changed: 48 additions & 106 deletions

File tree

internal/transport/controlbuf.go

Lines changed: 4 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,13 @@ import (
2424
"fmt"
2525
"net"
2626
"runtime"
27-
"strconv"
2827
"sync"
2928
"sync/atomic"
3029

3130
"golang.org/x/net/http2"
3231
"golang.org/x/net/http2/hpack"
3332
"google.golang.org/grpc/internal/grpclog"
34-
"google.golang.org/grpc/internal/grpcutil"
35-
"google.golang.org/grpc/internal/pretty"
36-
istatus "google.golang.org/grpc/internal/status"
3733
"google.golang.org/grpc/mem"
38-
"google.golang.org/grpc/status"
39-
"google.golang.org/protobuf/proto"
4034
)
4135

4236
var updateHeaderTblSize = func(e *hpack.Encoder, v uint32) {
@@ -150,12 +144,9 @@ type cleanupStream struct {
150144
func (c *cleanupStream) isTransportResponseFrame() bool { return c.rst } // Results in a RST_STREAM
151145

152146
type earlyAbortStream struct {
153-
httpStatus uint32
154-
streamID uint32
155-
contentSubtype string
156-
status *status.Status
157-
rst bool
158-
maxSendHeaderListSize *uint32
147+
streamID uint32
148+
rst bool
149+
hf []hpack.HeaderField // Pre-built header fields
159150
}
160151

161152
func (*earlyAbortStream) isTransportResponseFrame() bool { return false }
@@ -847,34 +838,7 @@ func (l *loopyWriter) earlyAbortStreamHandler(eas *earlyAbortStream) error {
847838
if l.side == clientSide {
848839
return errors.New("earlyAbortStream not handled on client")
849840
}
850-
// In case the caller forgets to set the http status, default to 200.
851-
if eas.httpStatus == 0 {
852-
eas.httpStatus = 200
853-
}
854-
headerFields := []hpack.HeaderField{
855-
{Name: ":status", Value: strconv.Itoa(int(eas.httpStatus))},
856-
{Name: "content-type", Value: grpcutil.ContentType(eas.contentSubtype)},
857-
{Name: "grpc-status", Value: strconv.Itoa(int(eas.status.Code()))},
858-
{Name: "grpc-message", Value: encodeGrpcMessage(eas.status.Message())},
859-
}
860-
861-
if p := istatus.RawStatusProto(eas.status); len(p.GetDetails()) > 0 {
862-
stBytes, err := proto.Marshal(p)
863-
if err != nil {
864-
l.logger.Errorf("Failed to marshal rpc status: %s, error: %v", pretty.ToJSON(p), err)
865-
} else {
866-
headerFields = append(headerFields, hpack.HeaderField{Name: grpcStatusDetailsBinHeader, Value: encodeBinHeader(stBytes)})
867-
}
868-
}
869-
870-
if !checkForHeaderListSize(headerFields, eas.maxSendHeaderListSize) {
871-
if l.logger.V(logLevel) {
872-
l.logger.Infof("Header list size to send violates the maximum size (%d bytes) set by client", *eas.maxSendHeaderListSize)
873-
}
874-
return l.framer.fr.WriteRSTStream(eas.streamID, http2.ErrCodeInternal)
875-
}
876-
877-
if err := l.writeHeader(eas.streamID, true, headerFields, nil); err != nil {
841+
if err := l.writeHeader(eas.streamID, true, eas.hf, nil); err != nil {
878842
return err
879843
}
880844
if eas.rst {

internal/transport/http2_server.go

Lines changed: 44 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -479,17 +479,7 @@ func (t *http2Server) operateHeaders(ctx context.Context, frame *http2.MetaHeade
479479
if t.logger.V(logLevel) {
480480
t.logger.Infof("Aborting the stream early: %v", errMsg)
481481
}
482-
eas := &earlyAbortStream{
483-
httpStatus: http.StatusBadRequest,
484-
streamID: streamID,
485-
contentSubtype: s.contentSubtype,
486-
status: status.New(codes.Internal, errMsg),
487-
rst: !frame.StreamEnded(),
488-
}
489-
t.controlBuf.executeAndPut(func() bool {
490-
eas.maxSendHeaderListSize = t.maxSendHeaderListSize
491-
return true
492-
}, eas)
482+
t.writeEarlyAbort(streamID, s.contentSubtype, status.New(codes.Internal, errMsg), http.StatusBadRequest, !frame.StreamEnded())
493483
return nil
494484
}
495485

@@ -503,31 +493,11 @@ func (t *http2Server) operateHeaders(ctx context.Context, frame *http2.MetaHeade
503493
return nil
504494
}
505495
if !isGRPC {
506-
eas := &earlyAbortStream{
507-
httpStatus: http.StatusUnsupportedMediaType,
508-
streamID: streamID,
509-
contentSubtype: s.contentSubtype,
510-
status: status.Newf(codes.InvalidArgument, "invalid gRPC request content-type %q", contentType),
511-
rst: !frame.StreamEnded(),
512-
}
513-
t.controlBuf.executeAndPut(func() bool {
514-
eas.maxSendHeaderListSize = t.maxSendHeaderListSize
515-
return true
516-
}, eas)
496+
t.writeEarlyAbort(streamID, s.contentSubtype, status.Newf(codes.InvalidArgument, "invalid gRPC request content-type %q", contentType), http.StatusUnsupportedMediaType, !frame.StreamEnded())
517497
return nil
518498
}
519499
if headerError != nil {
520-
eas := &earlyAbortStream{
521-
httpStatus: http.StatusBadRequest,
522-
streamID: streamID,
523-
contentSubtype: s.contentSubtype,
524-
status: headerError,
525-
rst: !frame.StreamEnded(),
526-
}
527-
t.controlBuf.executeAndPut(func() bool {
528-
eas.maxSendHeaderListSize = t.maxSendHeaderListSize
529-
return true
530-
}, eas)
500+
t.writeEarlyAbort(streamID, s.contentSubtype, headerError, http.StatusBadRequest, !frame.StreamEnded())
531501
return nil
532502
}
533503

@@ -581,17 +551,7 @@ func (t *http2Server) operateHeaders(ctx context.Context, frame *http2.MetaHeade
581551
if t.logger.V(logLevel) {
582552
t.logger.Infof("Aborting the stream early: %v", errMsg)
583553
}
584-
eas := &earlyAbortStream{
585-
httpStatus: http.StatusMethodNotAllowed,
586-
streamID: streamID,
587-
contentSubtype: s.contentSubtype,
588-
status: status.New(codes.Internal, errMsg),
589-
rst: !frame.StreamEnded(),
590-
}
591-
t.controlBuf.executeAndPut(func() bool {
592-
eas.maxSendHeaderListSize = t.maxSendHeaderListSize
593-
return true
594-
}, eas)
554+
t.writeEarlyAbort(streamID, s.contentSubtype, status.New(codes.Internal, errMsg), http.StatusMethodNotAllowed, !frame.StreamEnded())
595555
s.cancel()
596556
return nil
597557
}
@@ -606,35 +566,15 @@ func (t *http2Server) operateHeaders(ctx context.Context, frame *http2.MetaHeade
606566
if !ok {
607567
stat = status.New(codes.PermissionDenied, err.Error())
608568
}
609-
eas := &earlyAbortStream{
610-
httpStatus: http.StatusOK,
611-
streamID: s.id,
612-
contentSubtype: s.contentSubtype,
613-
status: stat,
614-
rst: !frame.StreamEnded(),
615-
}
616-
t.controlBuf.executeAndPut(func() bool {
617-
eas.maxSendHeaderListSize = t.maxSendHeaderListSize
618-
return true
619-
}, eas)
569+
t.writeEarlyAbort(s.id, s.contentSubtype, stat, http.StatusOK, !frame.StreamEnded())
620570
return nil
621571
}
622572
}
623573

624574
if s.ctx.Err() != nil {
625575
t.mu.Unlock()
626576
// Early abort in case the timeout was zero or so low it already fired.
627-
eas := &earlyAbortStream{
628-
httpStatus: http.StatusOK,
629-
streamID: s.id,
630-
contentSubtype: s.contentSubtype,
631-
status: status.New(codes.DeadlineExceeded, context.DeadlineExceeded.Error()),
632-
rst: !frame.StreamEnded(),
633-
}
634-
t.controlBuf.executeAndPut(func() bool {
635-
eas.maxSendHeaderListSize = t.maxSendHeaderListSize
636-
return true
637-
}, eas)
577+
t.writeEarlyAbort(s.id, s.contentSubtype, status.New(codes.DeadlineExceeded, context.DeadlineExceeded.Error()), http.StatusOK, !frame.StreamEnded())
638578
return nil
639579
}
640580

@@ -1019,6 +959,44 @@ func (t *http2Server) checkForHeaderListSize(it any) bool {
1019959
return true
1020960
}
1021961

962+
// buildEarlyAbortHF builds the header fields for an early abort response.
963+
func buildEarlyAbortHF(httpStatus uint32, contentSubtype string, stat *status.Status) []hpack.HeaderField {
964+
hf := []hpack.HeaderField{
965+
{Name: ":status", Value: strconv.Itoa(int(httpStatus))},
966+
{Name: "content-type", Value: grpcutil.ContentType(contentSubtype)},
967+
{Name: "grpc-status", Value: strconv.Itoa(int(stat.Code()))},
968+
{Name: "grpc-message", Value: encodeGrpcMessage(stat.Message())},
969+
}
970+
if p := istatus.RawStatusProto(stat); len(p.GetDetails()) > 0 {
971+
stBytes, err := proto.Marshal(p)
972+
if err == nil {
973+
hf = append(hf, hpack.HeaderField{Name: grpcStatusDetailsBinHeader, Value: encodeBinHeader(stBytes)})
974+
}
975+
}
976+
return hf
977+
}
978+
979+
// writeEarlyAbort sends an early abort response with the given HTTP status and gRPC status.
980+
// If the header list size exceeds the peer's limit, it sends a RST_STREAM instead.
981+
func (t *http2Server) writeEarlyAbort(streamID uint32, contentSubtype string, stat *status.Status, httpStatus uint32, rst bool) {
982+
hf := buildEarlyAbortHF(httpStatus, contentSubtype, stat)
983+
success, _ := t.controlBuf.executeAndPut(func() bool {
984+
return checkForHeaderListSize(hf, t.maxSendHeaderListSize)
985+
}, &earlyAbortStream{
986+
streamID: streamID,
987+
rst: rst,
988+
hf: hf,
989+
})
990+
if !success {
991+
t.controlBuf.put(&cleanupStream{
992+
streamID: streamID,
993+
rst: true,
994+
rstCode: http2.ErrCodeInternal,
995+
onWrite: func() {},
996+
})
997+
}
998+
}
999+
10221000
func (t *http2Server) streamContextErr(s *ServerStream) error {
10231001
select {
10241002
case <-t.done:

0 commit comments

Comments
 (0)