This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko-grpc.git
The following commit(s) were added to refs/heads/main by this push:
new 02a70d2f Report malformed response headers as a gRPC status (#870)
02a70d2f is described below
commit 02a70d2f064e808c2bea4916da72d6f585a661b9
Author: PJ Fanning <[email protected]>
AuthorDate: Tue Sep 1 11:21:24 2026 +0100
Report malformed response headers as a gRPC status (#870)
Motivation:
Two spots in the pekko-http client turned a malformed response from the peer
into a raw JVM exception rather than a `StatusRuntimeException`, so a caller
matching on gRPC status saw something it could not handle.
`Status.fromCodeValue(statusCode.toInt)` threw `NumberFormatException` for a
server sending `grpc-status: abc`. Building the response metadata
base64-decodes every `-bin` header, so a malformed one threw
`IllegalArgumentException` - and `mapToStatusException` builds that
metadata for
every error response, so an invalid binary header replaced the error
actually
being reported with an exception about itself.
Both are reachable from any peer. The netty backend rejects these at the
transport layer, so this was pekko-http only.
Modification:
- A non-numeric `grpc-status` is reported as `INTERNAL`, quoting the
offending
value. `parseResponseStatus` already matches on `Some("0")`, so such a
response was on the failure path regardless; it now fails with a status.
- Metadata construction falls back to empty metadata if a header cannot be
represented, so the status and message still reach the caller.
Result:
A broken or hostile peer produces a `StatusRuntimeException` like any other
failure, instead of an exception the caller is not expecting.
Tests:
- 2 cases in `PekkoHttpClientUtilsSpec`: a non-numeric `grpc-status`, and a
malformed `-bin` header alongside a real error that must still be reported
- Confirmed both guards bite: restoring `toInt` fails the first, removing
the
metadata fallback fails the second
- sbt "runtime/testOnly ...PekkoHttpClientUtilsSpec" - 8 passed
- sbt "runtime/mimaReportBinaryIssues" - passed
- sbt scalafmtAll scalafmtSbt - applied
- sbt "runtime/test" - not run locally, left to CI
Note: `HeaderMetadataImpl.getBinary` and `asList` still throw on a malformed
binary header when a user calls them directly on a successful response.
That is
left as is - an explicit accessor call is a different contract from the
error
path, where the exception displaces an unrelated failure.
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../pekko/grpc/internal/PekkoHttpClientUtils.scala | 33 ++++++++++++++++++++--
.../grpc/internal/PekkoHttpClientUtilsSpec.scala | 32 +++++++++++++++++++++
2 files changed, 63 insertions(+), 2 deletions(-)
diff --git
a/runtime/src/main/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtils.scala
b/runtime/src/main/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtils.scala
index f513f72d..a5c7eb65 100644
---
a/runtime/src/main/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtils.scala
+++
b/runtime/src/main/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtils.scala
@@ -42,6 +42,7 @@ import scala.concurrent.{ ExecutionContext, Future, Promise }
import scala.concurrent.duration.DurationLong
import scala.jdk.FutureConverters._
import scala.util.{ Failure, Success }
+import scala.util.control.NonFatal
/**
* INTERNAL API
@@ -400,7 +401,7 @@ object PekkoHttpClientUtils {
private def mapToStatusException(response: HttpResponse, trailers:
Seq[HttpHeader]): StatusRuntimeException = {
val allHeaders = response.headers ++ trailers
- val metadata: io.grpc.Metadata = new MetadataImpl(new
HeaderMetadataImpl(allHeaders).asList).toGoogleGrpcMetadata()
+ val metadata: io.grpc.Metadata = metadataOf(allHeaders)
allHeaders.find(_.name == "grpc-status").map(_.value) match {
case None =>
new StatusRuntimeException(mapHttpStatus(response).withDescription("No
grpc-status found"), metadata)
@@ -409,10 +410,38 @@ object PekkoHttpClientUtils {
// before it reaches the caller. The server side encodes it in
`Status-Message`.
val description =
allHeaders.find(_.name == "grpc-message").map(h =>
PercentEncoding.Decoder.decode(h.value))
- new
StatusRuntimeException(Status.fromCodeValue(statusCode.toInt).withDescription(description.orNull),
metadata)
+ statusCodeOf(statusCode) match {
+ case Some(code) =>
+ new
StatusRuntimeException(Status.fromCodeValue(code).withDescription(description.orNull),
metadata)
+ case None =>
+ // a peer that sends a non-numeric grpc-status is broken, but that
is a protocol
+ // error to report, not an exception to throw at the caller
+ new StatusRuntimeException(
+ Status.INTERNAL.withDescription(s"Invalid grpc-status
[$statusCode] in response"),
+ metadata)
+ }
}
}
+ /**
+ * The response metadata, or empty metadata if a header cannot be
represented.
+ *
+ * Binary headers are base64-decoded here, which throws on a malformed
value. That is
+ * reachable from any peer, and it happens while building the metadata for
an error that is
+ * already being reported, so it must not replace that error with an
exception of its own.
+ */
+ private def metadataOf(allHeaders: Seq[HttpHeader]): io.grpc.Metadata =
+ try new MetadataImpl(new
HeaderMetadataImpl(allHeaders).asList).toGoogleGrpcMetadata()
+ catch {
+ case NonFatal(_) => new io.grpc.Metadata()
+ }
+
+ private def statusCodeOf(value: String): Option[Int] =
+ try Some(value.toInt)
+ catch {
+ case _: NumberFormatException => None
+ }
+
/**
* See
https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md
*/
diff --git
a/runtime/src/test/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtilsSpec.scala
b/runtime/src/test/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtilsSpec.scala
index b3dfe42c..c5e5486f 100644
---
a/runtime/src/test/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtilsSpec.scala
+++
b/runtime/src/test/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtilsSpec.scala
@@ -131,6 +131,38 @@ class PekkoHttpClientUtilsSpec extends
TestKit(ActorSystem()) with AnyWordSpecLi
failure.asInstanceOf[StatusRuntimeException].getStatus.getDescription
should be("broken %ZZ escape")
}
+ "report a non-numeric grpc-status as INTERNAL rather than throwing" in {
+ // a broken peer must not surface as a NumberFormatException to the
caller
+ val responseHeaders = RawHeader("grpc-status", "abc") :: Nil
+ val response =
+ Future.successful(HttpResponse(OK, responseHeaders,
Strict(GrpcProtocolNative.contentType, ByteString.empty)))
+
+ val failure = PekkoHttpClientUtils.responseToSource(response,
null).run().failed.futureValue
+
+ failure shouldBe a[StatusRuntimeException]
+ val status = failure.asInstanceOf[StatusRuntimeException].getStatus
+ status.getCode should be(Status.Code.INTERNAL)
+ status.getDescription should include("abc")
+ }
+
+ "still report the gRPC status when a binary header is not valid base64" in
{
+ // building the metadata base64-decodes `-bin` headers, which throws on
a malformed value.
+ // That must not replace the error already being reported with an
exception of its own.
+ val responseHeaders = RawHeader("grpc-status", "9") ::
+ RawHeader("grpc-message", "the real failure") ::
+ RawHeader("custom-key-bin", "!!!not valid base64!!!") ::
+ Nil
+ val response =
+ Future.successful(HttpResponse(OK, responseHeaders,
Strict(GrpcProtocolNative.contentType, ByteString.empty)))
+
+ val failure = PekkoHttpClientUtils.responseToSource(response,
null).run().failed.futureValue
+
+ failure shouldBe a[StatusRuntimeException]
+ val status = failure.asInstanceOf[StatusRuntimeException].getStatus
+ status.getCode should be(Status.Code.FAILED_PRECONDITION)
+ status.getDescription should be("the real failure")
+ }
+
"map a strict 200 response with non-0 gRPC error code with a trailer to a
failed stream with trailer metadata" in {
val responseHeaders = List(RawHeader("grpc-status", "9"))
val responseTrailers = Trailer(
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
