[PR] Percent-decode grpc-message on the pekko-http client [pekko-grpc]

["pjfanning (via GitHub)" <gi...@apache.org>]

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: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org

Re: [PR] Percent-decode grpc-message on the pekko-http client [pekko-grpc]

["raboof (via GitHub)" <gi...@apache.org>]

raboof merged PR #869:
URL: https://github.com/apache/pekko-grpc/pull/869


-- 
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: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org