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)
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.