HTTP/2 error when parsing percent-encoded query string
- Dominant language
- Scala
- Stars
- 196
- Forks
- 55
- Avg merge
- 4d 52m
- Merged PRs (30d)
- 74
Description
Very simple reproducer with explanation: https://github.com/mkurz/akka-http-percent-encoding-bug (that reproducer is still based on akka-http but should work 1:1 when applied to pekko-http)
Copy and pasted README (in case I ever delete that repo):
pekko-http's (experimental?) http2 support does not correctly handle URL parsing errors.
Specially when passing an invalid percent-encoded character in the path and/or query string (here `%_D`):
As reproducer the example from the akka docs can be used, but with HTTP/2 enabled:
* https://doc.akka.io/docs/akka-http/current/introduction.html#low-level-http-server-apis
* https://doc.akka.io/docs/akka-http/current/server-side/http2.html#enable-http-2-support
```sh
# Start the server
sbt run
```
```sh
# HTTP/1 works:
$ curl --http1.1 -v http://localhost:8080/?param=%_D
* Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /?param=%_D HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.87.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 400 Bad Request
< Server: akka-http/10.2.10
< Date: Wed, 15 Feb 2023 16:29:18 GMT
< Connection: close
< Content-Type: text/plain; charset=UTF-8
< Content-Length: 78
<
* Closing connection 0
Illegal request-target: Invalid input '_', expected HEXDIG (line 1, column 10)
```
```sh
# HTTP/2 gives error:
$ curl --http2-prior-knowledge -v http://localhost:8080/?param=%_D
* Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
* Using HTTP2, server supports multiplexing
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* h2h3 [:method: GET]
* h2h3 [:path: /?param=%_D]
* h2h3 [:scheme: http]
* h2h3 [:authority: localhost:8080]
* h2h3 [user-agent: curl/7.87.0]
* h2h3 [accept: */*]
* Using Stream ID: 1 (easy handle 0xaaabd90f0dc0)
> GET /?param=%_D HTTP/2
> Host: localhost:8080
> user-agent: curl/7.87.0
> accept: */*
>
* Connection state changed (MAX_CONCURRENT_STREAMS == 256)!
* Closing connection 0
curl: (16) Error in the HTTP2 framing layer
```
At some point, no matter if using HTTP/1 or HTTP/2, the parser ends up here
* https://github.com/apache/incubator-pekko-http/blob/6624fa98f43849f791bccd14c1ec3d80d091a994/http-core/src/main/scala/org/apache/pekko/http/impl/model/parser/UriParser.scala#L253-L256
As you can see it will (also) be looked for `pct-encoded` characters, which expect a `%` sign, followed by two `HEXDIG` signs:
* https://github.com/apache/incubator-pekko-http/blob/6624fa98f43849f791bccd14c1ec3d80d091a994/http-core/src/main/scala/org/apache/pekko/http/impl/model/parser/UriParser.scala#L300-L305
Now if that fails...
* **...when using HTTP/1...**
...then the method [`parseHttpRequestTarget`](https://github.com/apache/incubator-pekko-http/blob/6624fa98f43849f791bccd14c1ec3d80d091a994/http-core/src/main/scala/org/apache/pekko/http/impl/model/parser/UriParser.scala#L320-L326) fails with (=throws) an `IllegalUriException`, which will later be handled so a 400 Bad Request with following body will be send:
```
Illegal request-target: Invalid input '_', expected HEXDIG (line 1, column 10)
```
(There are various places in the http-core module where `IllegalUriException`s and ` ParsingException`s are caught and handled)
* **...when using HTTP/2...**
`PathAndQuery.parse` in following line throws an `ParsingException` which is never handled (this is in the `org.apache.pekko.http.impl.engine.http2` package, so not used by HTTP/1):
* https://github.com/apache/incubator-pekko-http/blob/6624fa98f43849f791bccd14c1ec3d80d091a994/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala#L71
A couple of lines later only an `IOException` gets caught:
* https://github.com/apache/incubator-pekko-http/blob/6624fa98f43849f791bccd14c1ec3d80d091a994/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala#L86-L96
If you see where the containing method `parseAndEmit` gets called, the `ParsingException` never gets handled.
# Expected Result
When using HTTP/2 I would expect to also receive a 400 Bad Request with the body:
```
Illegal request-target: Invalid input '_', expected HEXDIG (line 1, column 10)
```
Contributor guide
Research direction
Start with http-core/src/main/scala/org/apache/pekko/http/impl/model/parser/UriParser.scala and the HTTP/2 flow in http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala, including parseAndEmit. Reproduce the invalid `%_D` query with curl over HTTP/1.1 and HTTP/2, then trace how ParsingException is handled. Done means HTTP/2 returns 400 Bad Request with the documented error body instead of closing with an HTTP/2 framing error.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- scala
- Domain
- backend, backend-api-design, networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100