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

Critical: HTTP2 stream will load whole response into memory #282

Open
hissssst opened this issue Aug 3, 2024 · 2 comments
Open

Critical: HTTP2 stream will load whole response into memory #282

hissssst opened this issue Aug 3, 2024 · 2 comments

Comments

@hissssst
Copy link

hissssst commented Aug 3, 2024

Mix.install [:plug, :bandit, :finch]

defmodule P do
  @behaviour Plug
  import Plug.Conn

  def init(_ \\ []) do
    {:ok, []}
  end

  def call(conn, _) do
    conn = send_chunked(conn, 200)
    Enum.reduce(1..1_000_000_000, conn, fn _, conn ->
      {:ok, conn} = chunk(conn, String.duplicate("0", 1_000_000))
      conn
    end)
  end
end

Bandit.start_link(ip: {0, 0, 0, 0}, plug: P, port: 8000, http_1_options: [enabled: true])

Finch.start_link(name: TestFinch, pools: %{default: [size: 100, count: 1, protocols: [:http2]]})

r = Finch.build("GET", "http://localhost:8000/")
Finch.stream(r, TestFinch, [], fn _, _ ->
  IO.inspect Process.info(self(), :message_queue_len)
  Process.sleep(1_000)
end)

Run this and see in logs

{:message_queue_len, 1}
{:message_queue_len, 15902}
{:message_queue_len, 33221}
{:message_queue_len, 50260}
{:message_queue_len, 67273}
{:message_queue_len, 84020}

Current implementation of streaming in HTTP2 is extremely dangerous since it may just OOM the caller, which is a bad thing to do. I'd mark http2 adapter as not ready for production use and raise in any client using it, until this gets fixed

@hissssst
Copy link
Author

hissssst commented Aug 3, 2024

I've reproduced it until the program got killed by OOM-killer, consuming 16gb (max on my machine) total

@hissssst
Copy link
Author

hissssst commented Aug 5, 2024

@sneako , I don't think that it is a good thing to consider this issue as completed when there's no solution, but only a warning

@sneako sneako reopened this Aug 6, 2024
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

No branches or pull requests

2 participants