pjfanning opened a new pull request, #869:
URL: https://github.com/apache/pekko-grpc/pull/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:
```scala
val description = allHeaders.find(_.name == "grpc-message").map(_.value)
```
So an error description containing anything outside the unescaped set reached
the caller as `caf%C3%A9` rather than `café`. The netty backend decodes it
inside
grpc-java, so this affected the pekko-http backend 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 is explicit that an invalid value must not
be
errored on or thrown away, and this silently destroyed it. Only values
containing a `%` reach the slow path, which is why the existing round-trip
tests
missed it — they always encode first, so any non-ASCII is already escaped by
the
time `parse` sees it.
**grpc-java does not have this bug.** Its `parseAsciiString` takes the wire
`byte[]` directly and uses `US_ASCII` only for the two hex digits following a
`%`, which are ASCII by definition; payload bytes are copied through
untouched
and decoded once as UTF-8 at the end. The pekko port takes a `String`,
because
`ModeledCustomHeaderCompanion.parse` hands it one, so it needs a
String-to-bytes
step that grpc-java simply does not have — and that step picked `US_ASCII`,
following the name `parseAsciiString` rather than its actual data flow.
### Modification
- `mapToStatusException` decodes the header through
`PercentEncoding.Decoder`.
- `Decoder.TransferEncoding` is now `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 cannot reach
Confirmed both guards bite, by reverting each fix in turn:
- client-side decode removed → the encoded-message case fails
- `TransferEncoding` back to `US_ASCII` → the decoder case fails
- `sbt "runtime/testOnly ...PekkoHttpClientUtilsSpec ...HeadersSpec"` — 14
passed
- `sbt "runtime/mimaReportBinaryIssues"` — passed
- `sbt scalafmtAll scalafmtSbt` — applied
- `sbt "runtime/test"` (full suite) — not run locally, left to CI
### Notes
One divergence from grpc-java left as is: its fast-path guard requires
`i + 2 < value.length` before taking the slow path, so a trailing `%`
short-circuits, whereas `decode` only checks `indexOf('%') > -1`. Both
produce
the same output, just via different paths.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
