apache / apache/pekko-http

HTTP/2 error when parsing percent-encoded query string

Open
#59 16 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.