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

Unable to send request with body larger than 64Kb #265

Open
GRoguelon opened this issue Feb 20, 2024 · 8 comments
Open

Unable to send request with body larger than 64Kb #265

GRoguelon opened this issue Feb 20, 2024 · 8 comments

Comments

@GRoguelon
Copy link

Hi,

I am not totally sure if this is a bug but I found a stranged behaviour in Req when I'm sending a body larger than 64Kb and it seems to be related to Finch protocols option.

It seems a change in the protocols provides different behaviours:

With protocols: [:http1]:

iex(1)> {:ok, pid} = Finch.start_link(name: MyFinch, pools: %{ default: [ protocols: [:http1] ] })
{:ok, #PID<0.648.0>}
iex(2)> Finch.build(:post, "https://nghttp2.org/httpbin/post", [], :crypto.strong_rand_bytes(65538)) |> Finch.request(MyFinch)
{:ok,
 %Finch.Response{
   status: 200,
   body: "{\n  \"args\": {}, \n  \"data\": \"data:application/octet-stream;base64,TRUNCATED",
   headers: [
     {"date", "Tue, 20 Feb 2024 18:31:19 GMT"},
     {"content-type", "application/json"},
     {"content-length", "87690"},
     {"access-control-allow-origin", "*"},
     {"access-control-allow-credentials", "true"},
     {"x-backend-header-rtt", "0.156761"},
     {"strict-transport-security", "max-age=31536000"},
     {"connection", "close"},
     {"alt-svc", "h3=\":443\"; ma=3600, h3-29=\":443\"; ma=3600"},
     {"server", "nghttpx"},
     {"via", "1.1 nghttpx"},
     {"x-frame-options", "SAMEORIGIN"},
     {"x-xss-protection", "1; mode=block"},
     {"x-content-type-options", "nosniff"}
   ],
   trailers: []
 }}

With protocols: [:http2]:

iex(1)> {:ok, pid} = Finch.start_link(name: MyFinch, pools: %{ default: [ protocols: [:http2] ] })
{:ok, #PID<0.648.0>}
iex(2)> Finch.build(:post, "https://nghttp2.org/httpbin/post", [], :crypto.strong_rand_bytes(65538)) |> Finch.request(MyFinch)
{:ok,
 %Finch.Response{
   status: 200,
   body: "{\n  \"args\": {}, \n  \"data\": \"data:application/octet-stream;base64,TRUNCATED",
   headers: [
     {"date", "Tue, 20 Feb 2024 18:27:43 GMT"},
     {"content-type", "application/json"},
     {"content-length", "87695"},
     {"access-control-allow-origin", "*"},
     {"access-control-allow-credentials", "true"},
     {"x-backend-header-rtt", "0.508807"},
     {"strict-transport-security", "max-age=31536000"},
     {"server", "nghttpx"},
     {"alt-svc", "h3=\":443\"; ma=3600, h3-29=\":443\"; ma=3600"},
     {"via", "1.1 nghttpx"},
     {"x-frame-options", "SAMEORIGIN"},
     {"x-xss-protection", "1; mode=block"},
     {"x-content-type-options", "nosniff"}
   ],
   trailers: []
 }}

With protocols: [:http1, :http2]:

iex(1)> {:ok, pid} = Finch.start_link(name: MyFinch, pools: %{ default: [ protocols: [:http1, :http2] ] })
{:ok, #PID<0.648.0>}
iex(2)> Finch.build(:post, "https://nghttp2.org/httpbin/post", [], :crypto.strong_rand_bytes(65538)) |> Finch.request(MyFinch)
{:error,
 %Mint.HTTPError{
   reason: {:exceeds_window_size, :request, 65535},
   module: Mint.HTTP2
 }}

Is it related to the lack of multiplexing when :http1 is provided?

Regards,

@wojtekmach
Copy link
Contributor

wojtekmach commented Feb 20, 2024

Oh man, I was able to reproduce it in Mint. I don't understand why it works through Finch protocols: [:http2].

Mix.install([
  {:mint, github: "elixir-mint/mint"},
  :castore
])

url = URI.new!("https://nghttp2.org/httpbin/post")

scheme = String.to_existing_atom(url.scheme)
options = [protocols: [:http2]]
{:ok, conn} = Mint.HTTP.connect(scheme, url.host, url.port, options)

{:error, _conn, error} =
  Mint.HTTP.request(conn, "POST", url.path, [], :crypto.strong_rand_bytes(65538))

IO.inspect(error)
%Mint.HTTPError{
  reason: {:exceeds_window_size, :request, 65535},
  module: Mint.HTTP2
}

@wojtekmach
Copy link
Contributor

Briefly looking at Finch.HTTP2.Pool, it implements flow control as described in https://hexdocs.pm/mint/Mint.HTTP2.html#get_window_size/2. The protocols: [:http1, :http2] option, talking HTTP/2 over HTTP/1 connection (Finch.HTTP1.Conn) does not as it's using just the Mint.HTTP API.

We might have to port HTTP/2 flow control in HTTP/1 connections or give up on the http1/http2 alpn feature. I can look into this.

@ericmj
Copy link
Contributor

ericmj commented Feb 21, 2024

You can check if it's a HTTP/2 connection after ALPN upgrade is done and call the Mint.HTTP2 functions directly.

@whatyouhide
Copy link
Contributor

So ok let me back up a sec to understand this better. Mint itself is behaving correctly it seems, as it's yelling when protocols: [:http2] and it's not yelling when protocols: [:http1]. So far so good.

Finch, instead, is behaving incorrectly because the issue only shows up with protocols: [:http1, :http2], but not with protocols: [:http2] (where it should show up). Is that correct @sneako @wojtekmach?

What's happening with protocols: [:http2] that makes this not show up in Finch?

Sorry for backtracking here a bit, but I don't have that much time to dig into Finch's codebase right now and I’m trying to help out with this as best as I can 😄

@sneako
Copy link
Owner

sneako commented May 2, 2024

No worries Andrea, I appreciate your attention here.

My understanding is that this issue pops up when we end up using Finch's HTTP1 pool but the connections in that pool are using HTTP2 due to ALPN

@whatyouhide
Copy link
Contributor

Aaah okay, I see. One thing I still don't understand: we use protocols: [:http1, :http2] with Finch, so we use the HTTP/1 pool but the connections have upgraded to HTTP/2 through ALPN.

Okay, but why does the Finch HTTP/2 pool handle large req payloads correctly? If the Finch HTTP/2 pool uses HTTP/2 conns, does it do anything to payloads to prevent this error?

@sneako
Copy link
Owner

sneako commented May 2, 2024

That's because our H2 pool implements flow control as Wojtek pointed out above

@wojtekmach
Copy link
Contributor

So to be more specific it's perhaps because H2 always seems to set request body to :stream, https://github.com/sneako/finch/blob/v0.18.0/lib/finch/http2/pool.ex#L547, and perhaps then Mint does all of the bookkeeping under the hood after all?

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

5 participants