Skip to content

Commit dcb7af8

Browse files
committed
make sure we don't try to read past end of response body
1 parent e983700 commit dcb7af8

File tree

2 files changed

+22
-0
lines changed

2 files changed

+22
-0
lines changed

lib/sse_client/streaming_http.rb

+6
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ def initialize(socket, read_timeout)
9191
@read_timeout = read_timeout
9292
@parser = HTTPTools::Parser.new
9393
@buffer = ""
94+
@done = false
9495
@lock = Mutex.new
9596

9697
# Provide callbacks for the Parser to give us the headers and body. This has to be done
@@ -102,6 +103,9 @@ def initialize(socket, read_timeout)
102103
@parser.on(:stream) do |data|
103104
@lock.synchronize { @buffer << data } # synchronize because we're called from another thread in Socketry
104105
end
106+
@parser.on(:finish) do
107+
@lock.synchronize { @done = true }
108+
end
105109

106110
# Block until the status code and headers have been successfully read.
107111
while !have_headers
@@ -132,6 +136,8 @@ def read_all
132136
# Attempt to read some more data from the socket. Return true if successful, false if EOF.
133137
# A read timeout will result in an exception from Socketry's readpartial method.
134138
def read_chunk_into_buffer
139+
# If @done is set, it means the Parser has signaled end of response body
140+
@lock.synchronize { return false if @done }
135141
data = @socket.readpartial(DEFAULT_CHUNK_SIZE, timeout: @read_timeout)
136142
return false if data == :eof
137143
@parser << data

spec/sse_client/streaming_http_spec.rb

+16
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,22 @@ def with_connection(cxn)
7373
end
7474
end
7575

76+
it "can read entire response body" do
77+
body = <<-EOT
78+
This is
79+
a response
80+
EOT
81+
with_server do |server|
82+
server.setup_response("/foo") do |req,res|
83+
res.body = body
84+
end
85+
with_connection(subject.new(server.base_uri.merge("/foo"), nil, {}, 30, 30)) do |cxn|
86+
read_body = cxn.read_all
87+
expect(read_body).to eq("This is\na response\n")
88+
end
89+
end
90+
end
91+
7692
it "enforces read timeout" do
7793
with_server do |server|
7894
server.setup_response("/") do |req,res|

0 commit comments

Comments
 (0)