pjfanning opened a new pull request, #1280:
URL: https://github.com/apache/pekko-http/pull/1280
### Motivation
Refs #1273.
That issue asks for a way to send a request whose path renders exactly as
provided, because `Uri` cannot round-trip a percent-encoded path — it decodes
segments when parsing and re-encodes them with the `pchar-base` keep-set, which
leaves sub-delims raw, so `%2B` renders back as `+`. AWS SigV4 signs the
encoded path, so an S3 object key containing `+` or `=` is rejected with
`SignatureDoesNotMatch` after the round trip.
While investigating I found the issue's premise is only half right. The
escape hatch it proposes (option 2, "a pre-rendered request target that the
client renderer honors") **already exists for HTTP/1.1**:
`HttpRequestRendererFactory` special-cases a `Raw-Request-URI` header and emits
it verbatim as the request target. It is a `SyntheticHeader`, so it is not also
rendered as a header line; it is covered by tests in `RequestRendererSpec` and
documented under "synthetic headers" in `http-model.md`, whose example is
literally an S3 path.
What is genuinely missing is HTTP/2. `RequestRendering.initialHeaderPairs`
built `:path` from `request.uri.toHttpRequestTargetOriginForm` unconditionally
and ignored the header, so the same `HttpRequest` produced a different request
target depending on which protocol was negotiated.
### Modification
Take `Raw-Request-URI` into account when building `:path`, mirroring
HTTP/1.1.
The header needs no new suppression logic: being a `SyntheticHeader` it was
already excluded from the rendered header block by the existing
`renderInRequests` filter, so it was silently dropped rather than mis-sent. As
in HTTP/1.1, the value is used exactly as given — supplying a valid origin-form
target is the caller's responsibility, and this is stated in the docs.
Also documents, in the existing synthetic-headers section of
`http-model.md`, that the header is honoured by both clients, why it is needed
(the `Uri` round-trip loss), and the SigV4 case.
### Result
`aws-spi-pekko-http`, and any proxy or pass-through use case that must not
alter the request target, can pass `SdkHttpRequest.encodedPath()` through
unchanged over HTTP/2 as well as HTTP/1.1.
This does not change the `Uri` model, so the round-trip limitation described
in #1273 remains as such; it makes the existing escape hatch work uniformly
across protocols. I'd suggest #1273 stays open if a raw `Uri.Path`
representation is still wanted — that is a much larger change, since `Path` is
a sealed ADT matched exhaustively across the codebase and in user code.
### Tests
Two cases added to `Http2ClientSpec`:
- a request carrying `Raw-Request-URI("/a%2Bb%20c")` sends that value as
`:path`. The assertion compares the whole pseudo/header set, so it also pins
that the header itself is not rendered. Verified this fails on `main` — `:path`
comes out as `/a+b%20c` — and passes with the change.
- without the header, the path is still re-encoded from the `Uri`. This one
passes both before and after; it is a characterization test that pins the
round-trip loss (`/a%2Bb%20c` in, `/a+b%20c` out) which motivates the escape
hatch.
`sbt "http2-tests/testOnly *Http2ClientSpec *Http2ClientServerSpec
*Http2PersistentClientSpec *Http2ServerSpec"` — 183 pass. `sbt
http-core/mimaReportBinaryIssues` — clean (the change is confined to an
`@InternalApi private[http2]` class). Native `scalafmt` clean.
### References
Refs #1273.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
