This is an automated email from the ASF dual-hosted git repository.
raboof 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 8c11091f Percent-decode grpc-message on the pekko-http client (#869)
8c11091f is described below
commit 8c11091f8a97ae1e4a26302d1dd0598f33c39bfa
Author: PJ Fanning <[email protected]>
AuthorDate: Sun Aug 30 08:37:39 2026 +0100
Percent-decode grpc-message on the pekko-http client (#869)
Motivation:
`grpc-message` travels UTF-8 percent-encoded on the wire. The server side
encodes it, and `Status-Message.parse` exists to decode it, but the
pekko-http
client's `mapToStatusException` read the header value raw. A non-ASCII error
description reached the caller as `%D0%BF...` rather than text. The netty
backend decodes it in grpc-java, so this was pekko-http only.
Fixing that exposed a second bug underneath.
`PercentEncoding.Decoder.decode`
converted its input with `getBytes(US_ASCII)`, which replaces every
non-ASCII
character with '?'. A conforming grpc-message is pure ASCII so this never
bit on
valid input, but the protocol requires that an invalid value is neither
errored
on nor thrown away, and this silently destroyed it. Only values containing a
percent reach the slow path, which is why the existing round-trip tests
missed
it: they always encode first, so the non-ASCII is already escaped by then.
grpc-java does not have this bug. Its `parseAsciiString` takes the wire
bytes
directly and uses US-ASCII only for the two hex digits after a percent,
which
are ASCII by definition. The pekko port takes a String, because
`ModeledCustomHeaderCompanion.parse` hands it one, so it needs a
String-to-bytes
step that grpc-java does not have - and that step picked US-ASCII,
following the
name `parseAsciiString` rather than its data flow.
Modification:
- `mapToStatusException` decodes the header through
`PercentEncoding.Decoder`.
- `Decoder.TransferEncoding` is UTF-8. It is byte-identical to US-ASCII for
conforming input, and preserves the characters of a non-conforming one.
`decodeSlow` already copies payload bytes through one at a time and
decodes
once as UTF-8 at the end, so multi-byte sequences reassemble correctly.
Result:
A percent-encoded `grpc-message` reaches the caller as text on both
backends,
and a non-conforming one is no longer mangled.
Tests:
- 3 cases in `PekkoHttpClientUtilsSpec`: an encoded message, a plain one
that
must pass through untouched, and a broken `%ZZ` escape that must not be
thrown away
- 1 case in `HeadersSpec` for the decoder, covering non-ASCII alongside a
percent - the shape the existing round-trip tests could not reach
- Confirmed both guards bite: reverting the client-side decode fails the
encoded-message case, and restoring US-ASCII fails the decoder case
- sbt "runtime/testOnly ...PekkoHttpClientUtilsSpec ...HeadersSpec" - 14
passed
- sbt "runtime/mimaReportBinaryIssues" - passed
- sbt scalafmtAll scalafmtSbt - applied
- sbt "runtime/test" - not run locally, left to CI
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../pekko/grpc/internal/PekkoHttpClientUtils.scala | 6 +++-
.../grpc/scaladsl/headers/PercentEncoding.scala | 7 +++-
.../grpc/internal/PekkoHttpClientUtilsSpec.scala | 39 ++++++++++++++++++++++
.../pekko/grpc/scaladsl/headers/HeadersSpec.scala | 17 ++++++++++
4 files changed, 67 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 e864f3ab..91b8ab15 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
@@ -23,6 +23,7 @@ import pekko.actor.ClassicActorSystemProvider
import pekko.annotation.InternalApi
import pekko.event.LoggingAdapter
import pekko.grpc.GrpcProtocol.GrpcProtocolReader
+import pekko.grpc.scaladsl.headers.PercentEncoding
import pekko.grpc.{ GrpcClientSettings, GrpcResponseMetadata,
GrpcSingleResponse, ProtobufSerializer }
import pekko.http.scaladsl.model.HttpEntity.{ Chunk, Chunked, LastChunk,
Strict }
import pekko.http.scaladsl.{ ClientTransport, ConnectionContext, Http,
HttpsConnectionContext }
@@ -380,7 +381,10 @@ object PekkoHttpClientUtils {
case None =>
new StatusRuntimeException(mapHttpStatus(response).withDescription("No
grpc-status found"), metadata)
case Some(statusCode) =>
- val description = allHeaders.find(_.name ==
"grpc-message").map(_.value)
+ // grpc-message travels UTF-8 percent-encoded on the wire, so it has
to be decoded
+ // 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)
}
}
diff --git
a/runtime/src/main/scala/org/apache/pekko/grpc/scaladsl/headers/PercentEncoding.scala
b/runtime/src/main/scala/org/apache/pekko/grpc/scaladsl/headers/PercentEncoding.scala
index 5ad342ab..d1f6185a 100644
---
a/runtime/src/main/scala/org/apache/pekko/grpc/scaladsl/headers/PercentEncoding.scala
+++
b/runtime/src/main/scala/org/apache/pekko/grpc/scaladsl/headers/PercentEncoding.scala
@@ -79,7 +79,12 @@ private[grpc] object PercentEncoding {
// Copied with slight adaptations from
https://github.com/grpc/grpc-java/blob/79e75bace40cea7e4be72e7dcd1f41c3ad6ee857/api/src/main/java/io/grpc/Status.java#L626
object Decoder {
- private val TransferEncoding = StandardCharsets.US_ASCII
+ // A conforming value is pure ASCII, for which this is identical to
US-ASCII. It matters for
+ // a non-conforming one: `getBytes(US_ASCII)` turns every non-ASCII
character into '?', and
+ // the protocol requires that an invalid value is neither errored on nor
thrown away.
+ // grpc-java decodes straight off the wire, where the bytes are ASCII by
construction; here
+ // the input is already a String, so it can carry characters that ASCII
cannot represent.
+ private val TransferEncoding = StandardCharsets.UTF_8
def decode(value: String): String =
if (value.indexOf('%') > -1)
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 36db8f6a..26c15539 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
@@ -19,6 +19,7 @@ import scala.concurrent.duration._
import org.apache.pekko
import pekko.actor.ActorSystem
import pekko.grpc.GrpcResponseMetadata
+import pekko.grpc.scaladsl.headers.PercentEncoding
import pekko.http.scaladsl.model.HttpEntity.Strict
import pekko.http.scaladsl.model._
import pekko.http.scaladsl.model.StatusCodes._
@@ -64,6 +65,44 @@ class PekkoHttpClientUtilsSpec extends
TestKit(ActorSystem()) with AnyWordSpecLi
failure.asInstanceOf[StatusRuntimeException].getTrailers.get(key) should
be("custom-value-in-header")
}
+ "percent-decode the grpc-message of a failed response" in {
+ // grpc-message travels UTF-8 percent-encoded on the wire; the server
side encodes it with
+ // `PercentEncoding.Encoder`, so an undecoded client hands the caller
the raw escapes
+ val message = "quota exceeded: 100% of 5 µs — café"
+ val encoded = PercentEncoding.Encoder.encode(message)
+ encoded should not be message
+
+ val responseHeaders = RawHeader("grpc-status", "9") ::
RawHeader("grpc-message", encoded) :: Nil
+ val response =
+ Future.successful(HttpResponse(OK, responseHeaders,
Strict(GrpcProtocolNative.contentType, ByteString.empty)))
+
+ val failure = PekkoHttpClientUtils.responseToSource(response,
null).run().failed.futureValue
+
+ failure.asInstanceOf[StatusRuntimeException].getStatus.getDescription
should be(message)
+ }
+
+ "leave an unencoded grpc-message alone" in {
+ val message = "plain ascii failure"
+ val responseHeaders = RawHeader("grpc-status", "9") ::
RawHeader("grpc-message", message) :: Nil
+ val response =
+ Future.successful(HttpResponse(OK, responseHeaders,
Strict(GrpcProtocolNative.contentType, ByteString.empty)))
+
+ val failure = PekkoHttpClientUtils.responseToSource(response,
null).run().failed.futureValue
+
+ failure.asInstanceOf[StatusRuntimeException].getStatus.getDescription
should be(message)
+ }
+
+ "not throw away a grpc-message with a broken escape" in {
+ // the spec requires implementations not to error on invalid values
+ val responseHeaders = RawHeader("grpc-status", "9") ::
RawHeader("grpc-message", "broken %ZZ escape") :: Nil
+ val response =
+ Future.successful(HttpResponse(OK, responseHeaders,
Strict(GrpcProtocolNative.contentType, ByteString.empty)))
+
+ val failure = PekkoHttpClientUtils.responseToSource(response,
null).run().failed.futureValue
+
+ failure.asInstanceOf[StatusRuntimeException].getStatus.getDescription
should be("broken %ZZ escape")
+ }
+
"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(
diff --git
a/runtime/src/test/scala/org/apache/pekko/grpc/scaladsl/headers/HeadersSpec.scala
b/runtime/src/test/scala/org/apache/pekko/grpc/scaladsl/headers/HeadersSpec.scala
index 9fdffe71..1bd761d7 100644
---
a/runtime/src/test/scala/org/apache/pekko/grpc/scaladsl/headers/HeadersSpec.scala
+++
b/runtime/src/test/scala/org/apache/pekko/grpc/scaladsl/headers/HeadersSpec.scala
@@ -72,6 +72,23 @@ class HeadersSpec extends AnyWordSpec with Matchers {
actual.get.unencodedValue should equal(expected)
}
}
+
+ "not replace non-ASCII characters when the value also contains a percent"
in {
+ // A conforming grpc-message is pure ASCII, so this only arises for a
non-conforming value.
+ // The protocol requires that such a value is neither errored on nor
thrown away, but
+ // decoding through US-ASCII turned every non-ASCII character into '?'.
Without a percent
+ // the value is returned as is, so a percent is what it takes to reach
the slow path.
+ val inAndExpectedOut = Table(
+ ("raw input", "expected decoded value"),
+ ("100% café", "100% café"),
+ ("50% µs", "50% µs"),
+ ("%41 café", "A café"),
+ ("Καλημέρα 100%", "Καλημέρα 100%"))
+
+ forAll(inAndExpectedOut) { (in, expected) =>
+ `Status-Message`.parse(in).get.unencodedValue should equal(expected)
+ }
+ }
}
"Status-Message.value() and Status-Message.parse()" should {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
