puma: Streaming bodies, PR #2740, WebSocket

Describe the bug

PR #2740 added support for streaming bodies. Still reviewing, but somewhat confused. Code below after the PR:

if res_body && !res_body.respond_to?(:each)
  response_hijack = res_body
else
  response_hijack = resp_info[:response_hijack]
end

cork_socket socket

# ** removed existing 'no_body code', doesn't affect this issue

if content_length
  io_buffer.append CONTENT_LENGTH_S, content_length.to_s, line_ending
  chunked = false
elsif !response_hijack and resp_info[:allow_chunked]
  io_buffer << TRANSFER_ENCODING_CHUNKED
  chunked = true
end

io_buffer << line_ending

if response_hijack
  fast_write_str socket, io_buffer.to_s
  response_hijack.call socket
  return :async
end

Note the line elsif !response_hijack and resp_info[:allow_chunked]. This means that if Content-Length is specified, we will write the header, but if resp_info[:allow_chunked] is set, there will be no transfer-eoncoding header written. Hence, the added test in test_puma_server.rb test_streaming_body is an invalid response. So, should Puma be writing the header? This could be changed by removing !response_hijack from the elsif conditional.

Also, I’m not sure if Puma should be looking for headers returned from the app that would indicate a WebSocket upgrade and changing the response headers accordingly…

@ioquatix ? Hope.this.made.sense.

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Comments: 24 (24 by maintainers)

Most upvoted comments

Yup, it’s about time for one.

Can you let me know when it’s done so I can rerun the tests on my end! Just mention me in the PR/commit/whatever 😃

@ioquatix Sorry, busy tearing apart some old code. Yes, I think it should be merged soon.

I tried running it locally and it was passing, so I need to check a little bit more.