Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split trailers from headers #242

Merged
merged 5 commits into from
Sep 13, 2023
Merged

Split trailers from headers #242

merged 5 commits into from
Sep 13, 2023

Conversation

wojtekmach
Copy link
Contributor

@wojtekmach wojtekmach commented Aug 29, 2023

Ref wojtekmach/req#230 (comment)

I couldn't write an integration test for http2 given Plug has no support for trailers and it is not easy to write a fake server.

I verified it manually:

$ openssl req -newkey rsa:2048 -nodes -keyout server.key -x509 -days 365 -out server.crt
$ cat main.go
package main

import (
  "log"
  "net/http"
)

func main() {
  srv := &http.Server{Addr: ":8080", Handler: http.HandlerFunc(handle)}
  log.Printf("Serving on https://0.0.0.0:8080")
  log.Fatal(srv.ListenAndServeTLS("server.crt", "server.key"))
}

func handle(w http.ResponseWriter, r *http.Request) {
  log.Printf("Got connection: %s", r.Proto)
  w.Header().Set("trailer", "x-foo")
  w.Write([]byte("Hello"))
  w.Header().Set("x-foo", "bar")
}
$ cat run.exs
{:ok, _} =
  Finch.start_link(
    name: MyFinch,
    pools: %{
      default: [
        protocol: :http2,
        conn_opts: [
          transport_opts: [verify: :verify_none]
        ]
      ]
    }
  )

req = Finch.build(:get, "https://localhost:8080")
IO.inspect(Finch.request(req, MyFinch))
$ go run main.go
$ mix run run.exs
{:ok,
 %Finch.Response{
   status: 200,
   body: "Hello",
   headers: [
     {"trailer", "x-foo"},
     {"content-type", "text/plain; charset=utf-8"},
     {"content-length", "5"},
     {"date", "Tue, 29 Aug 2023 09:37:39 GMT"}
   ],
   trailers: [{"x-foo", "bar"}]
 }}

I couldn't write an integration test for http2 given Plug has no support for trailers and it is not easy to write a fake server.

I verified it manually:

```
$ openssl req -newkey rsa:2048 -nodes -keyout server.key -x509 -days 365 -out server.crt
$ cat main.go
```

```go
package main

import (
  "log"
  "net/http"
)

func main() {
  srv := &http.Server{Addr: ":8080", Handler: http.HandlerFunc(handle)}
  log.Printf("Serving on https://0.0.0.0:8080")
  log.Fatal(srv.ListenAndServeTLS("server.crt", "server.key"))
}

func handle(w http.ResponseWriter, r *http.Request) {
  log.Printf("Got connection: %s", r.Proto)
  w.Header().Set("trailer", "x-foo")
  w.Write([]byte("Hello"))
  w.Header().Set("x-foo", "bar")
}
```

```
$ cat run.exs
```

```elixir
{:ok, _} =
  Finch.start_link(
    name: MyFinch,
    pools: %{
      default: [
        protocol: :http2,
        conn_opts: [
          transport_opts: [verify: :verify_none]
        ]
      ]
    }
  )

req = Finch.build(:get, "https://localhost:8080")
IO.inspect(Finch.request(req, MyFinch))
```

```
$ go run main.go
$ mix run run.exs
```

```elixir
{:ok,
 %Finch.Response{
   status: 200,
   body: "Hello",
   headers: [
     {"trailer", "x-foo"},
     {"content-type", "text/plain; charset=utf-8"},
     {"content-length", "5"},
     {"date", "Tue, 29 Aug 2023 09:37:39 GMT"}
   ],
   trailers: [{"x-foo", "bar"}]
 }}
```
lib/finch/http1/conn.ex Outdated Show resolved Hide resolved
Copy link
Owner

@sneako sneako left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks @wojtekmach !

@sneako sneako merged commit 08085ca into sneako:main Sep 13, 2023
2 checks passed
@wojtekmach wojtekmach deleted the wm-trailers branch September 13, 2023 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants