From c7aec4defbfbc6581367bb2fbb77ba387fecf75c Mon Sep 17 00:00:00 2001
From: Evan Jones <evan.jones@datadoghq.com>
Date: Wed, 7 May 2025 14:20:46 -0400
Subject: [PATCH] transport: skip Status.Proto() without details in writeStatus
 (#8282)

---
 internal/status/status.go          |  8 ++++++
 internal/status/status_test.go     | 40 ++++++++++++++++++++++++++++++
 internal/transport/http2_server.go |  5 ++--
 status/status_test.go              |  5 +++-
 4 files changed, 55 insertions(+), 3 deletions(-)
 create mode 100644 internal/status/status_test.go

diff --git a/internal/status/status.go b/internal/status/status.go
index 1186f1e9a..aad171cd0 100644
--- a/internal/status/status.go
+++ b/internal/status/status.go
@@ -236,3 +236,11 @@ func IsRestrictedControlPlaneCode(s *Status) bool {
 	}
 	return false
 }
+
+// RawStatusProto returns the internal protobuf message for use by gRPC itself.
+func RawStatusProto(s *Status) *spb.Status {
+	if s == nil {
+		return nil
+	}
+	return s.s
+}
diff --git a/internal/status/status_test.go b/internal/status/status_test.go
new file mode 100644
index 000000000..1d3fc4c41
--- /dev/null
+++ b/internal/status/status_test.go
@@ -0,0 +1,40 @@
+/*
+ *
+ * Copyright 2025 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 status_test
+
+import (
+	"testing"
+
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/internal/status"
+)
+
+func TestRawStatusProto(t *testing.T) {
+	spb := status.RawStatusProto(nil)
+	if spb != nil {
+		t.Errorf("RawStatusProto(nil) = %v; must return nil", spb)
+	}
+	s := status.New(codes.Internal, "test internal error")
+	spb1 := status.RawStatusProto(s)
+	spb2 := status.RawStatusProto(s)
+	// spb1 and spb2 should be the same pointer: no copies
+	if spb1 != spb2 {
+		t.Errorf("RawStatusProto(s)=%p then %p; must return the same pointer", spb1, spb2)
+	}
+}
diff --git a/internal/transport/http2_server.go b/internal/transport/http2_server.go
index 7e53eb173..e4c3731bd 100644
--- a/internal/transport/http2_server.go
+++ b/internal/transport/http2_server.go
@@ -39,6 +39,7 @@ import (
 	"google.golang.org/grpc/internal/grpclog"
 	"google.golang.org/grpc/internal/grpcutil"
 	"google.golang.org/grpc/internal/pretty"
+	istatus "google.golang.org/grpc/internal/status"
 	"google.golang.org/grpc/internal/syscall"
 	"google.golang.org/grpc/mem"
 	"google.golang.org/protobuf/proto"
@@ -1055,7 +1056,7 @@ func (t *http2Server) writeHeaderLocked(s *ServerStream) error {
 	return nil
 }
 
-// WriteStatus sends stream status to the client and terminates the stream.
+// writeStatus sends stream status to the client and terminates the stream.
 // There is no further I/O operations being able to perform on this stream.
 // TODO(zhaoq): Now it indicates the end of entire stream. Revisit if early
 // OK is adopted.
@@ -1083,7 +1084,7 @@ func (t *http2Server) writeStatus(s *ServerStream, st *status.Status) error {
 	headerFields = append(headerFields, hpack.HeaderField{Name: "grpc-status", Value: strconv.Itoa(int(st.Code()))})
 	headerFields = append(headerFields, hpack.HeaderField{Name: "grpc-message", Value: encodeGrpcMessage(st.Message())})
 
-	if p := st.Proto(); p != nil && len(p.Details) > 0 {
+	if p := istatus.RawStatusProto(st); len(p.GetDetails()) > 0 {
 		// Do not use the user's grpc-status-details-bin (if present) if we are
 		// even attempting to set our own.
 		delete(s.trailer, grpcStatusDetailsBinHeader)
diff --git a/status/status_test.go b/status/status_test.go
index 03085d97d..b0f1f4e72 100644
--- a/status/status_test.go
+++ b/status/status_test.go
@@ -354,6 +354,9 @@ func (s) TestStatus_ErrorDetails(t *testing.T) {
 			t.Fatalf("(%v).WithDetails(%+v) failed: %v", str(s), tc.details, err)
 		}
 		details := s.Details()
+		if len(details) != len(tc.details) {
+			t.Fatalf("len(s.Details()) = %d, want = %d.", len(details), len(tc.details))
+		}
 		for i := range details {
 			if !proto.Equal(details[i].(protoreflect.ProtoMessage), tc.details[i].(protoreflect.ProtoMessage)) {
 				t.Fatalf("(%v).Details()[%d] = %+v, want %+v", str(s), i, details[i], tc.details[i])
@@ -420,7 +423,7 @@ func (s) TestStatus_ErrorDetails_Fail(t *testing.T) {
 	for _, tc := range tests {
 		details := tc.s.Details()
 		if len(details) != len(tc.want) {
-			t.Fatalf("len(s.Details()) = %v, want = %v.", len(details), len(tc.want))
+			t.Fatalf("len(s.Details()) = %d, want = %d.", len(details), len(tc.want))
 		}
 		for i, d := range details {
 			// s.Details can either contain an error or a proto message.  We