Skip to content

Commit

Permalink
Add state tracking for safer connection reuse.
Browse files Browse the repository at this point in the history
  • Loading branch information
ioquatix committed Sep 18, 2024
1 parent 4dae367 commit e88da02
Show file tree
Hide file tree
Showing 11 changed files with 234 additions and 59 deletions.
39 changes: 16 additions & 23 deletions lib/protocol/http1/body/chunked.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ module Body
class Chunked < HTTP::Body::Readable
CRLF = "\r\n"

def initialize(stream, headers)
@stream = stream
def initialize(connection, headers)
@connection = connection
@finished = false

@headers = headers
Expand All @@ -23,16 +23,16 @@ def initialize(stream, headers)
end

def empty?
@stream.nil?
@connection.nil?
end

def discard
if stream = @stream
@stream = nil
if connection = @connection
@connection = nil

# We only close the connection if we haven't completed reading the entire body:
unless @finished
stream.close_read
connection.close_read
end
end
end
Expand All @@ -48,8 +48,8 @@ def close(error = nil)
# Follows the procedure outlined in https://tools.ietf.org/html/rfc7230#section-4.1.3
def read
if !@finished
if @stream
length, _extensions = read_line.split(";", 2)
if @connection
length, _extensions = @connection.read_line.split(";", 2)

unless length =~ VALID_CHUNK_LENGTH
raise BadRequest, "Invalid chunk length: #{length.inspect}"
Expand All @@ -61,15 +61,16 @@ def read
if length == 0
read_trailer

# The final chunk has been read and the stream is now closed:
@stream = nil
# The final chunk has been read and the connection is now closed:
@connection.receive_end_stream!
@connection = nil
@finished = true

return nil
end

# Read trailing CRLF:
chunk = @stream.read(length + 2)
chunk = @connection.read(length + 2)

if chunk.bytesize == length + 2
# ...and chomp it off:
Expand All @@ -80,13 +81,13 @@ def read

return chunk
else
# The stream has been closed before we have read the requested length:
# The connection has been closed before we have read the requested length:
self.discard
end
end

# If the stream has been closed before we have read the final chunk, raise an error:
raise EOFError, "Stream closed before expected length was read!"
# If the connection has been closed before we have read the final chunk, raise an error:
raise EOFError, "connection closed before expected length was read!"
end
end

Expand All @@ -96,16 +97,8 @@ def inspect

private

def read_line?
@stream.gets(CRLF, chomp: true)
end

def read_line
read_line? or raise EOFError
end

def read_trailer
while line = read_line?
while line = @connection.read_line?
# Empty line indicates end of trailer:
break if line.empty?

Expand Down
31 changes: 18 additions & 13 deletions lib/protocol/http1/body/fixed.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ module Protocol
module HTTP1
module Body
class Fixed < HTTP::Body::Readable
def initialize(stream, length)
@stream = stream
def initialize(connection, length)
@connection = connection

@length = length
@remaining = length
Expand All @@ -20,15 +20,15 @@ def initialize(stream, length)
attr :remaining

def empty?
@stream.nil? or @remaining == 0
@connection.nil? or @remaining == 0
end

def discard
if stream = @stream
@stream = nil
if connection = @connection
@connection = nil

if @remaining != 0
stream.close_read
connection.close_read
end
end
end
Expand All @@ -39,25 +39,30 @@ def close(error = nil)
super
end

# @raises EOFError if the stream is closed before the expected length is read.
# @raises EOFError if the connection is closed before the expected length is read.
def read
if @remaining > 0
if @stream
# `readpartial` will raise `EOFError` if the stream is finished, or `IOError` if the stream is closed.
chunk = @stream.readpartial(@remaining)
if @connection
# `readpartial` will raise `EOFError` if the connection is finished, or `IOError` if the connection is closed.
chunk = @connection.readpartial(@remaining)

@remaining -= chunk.bytesize

if @remaining == 0
@connection.receive_end_stream!
@connection = nil
end

return chunk
end

# If the stream has been closed before we have read the expected length, raise an error:
raise EOFError, "Stream closed before expected length was read!"
# If the connection has been closed before we have read the expected length, raise an error:
raise EOFError, "connection closed before expected length was read!"
end
end

def inspect
"\#<#{self.class} length=#{@length} remaining=#{@remaining} state=#{@stream ? 'open' : 'closed'}>"
"\#<#{self.class} length=#{@length} remaining=#{@remaining} state=#{@connection ? 'open' : 'closed'}>"
end
end
end
Expand Down
29 changes: 15 additions & 14 deletions lib/protocol/http1/body/remainder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,25 @@
module Protocol
module HTTP1
module Body
# A body that reads all remaining data from the stream.
# A body that reads all remaining data from the connection.
class Remainder < HTTP::Body::Readable
BLOCK_SIZE = 1024 * 64

# block_size may be removed in the future. It is better managed by stream.
def initialize(stream)
@stream = stream
# block_size may be removed in the future. It is better managed by connection.
def initialize(connection)
@connection = connection
end

def empty?
@stream.nil?
@connection.nil?
end

def discard
if stream = @stream
@stream = nil
stream.close_read
if connection = @connection
@connection = nil

# Ensure no further requests can be read from the connection, as we are discarding the body which may not be fully read:
connection.close_read
end
end

Expand All @@ -35,15 +37,14 @@ def close(error = nil)
end

def read
@stream&.readpartial(BLOCK_SIZE)
rescue EOFError, IOError
@stream = nil
# I noticed that in some cases you will get EOFError, and in other cases IOError!?
return nil
@connection&.readpartial(BLOCK_SIZE)
rescue EOFError
@connection.receive_end_stream!
@connection = nil
end

def inspect
"\#<#{self.class} state=#{@stream ? 'open' : 'closed'}>"
"\#<#{self.class} state=#{@connection ? 'open' : 'closed'}>"
end
end
end
Expand Down
Loading

0 comments on commit e88da02

Please sign in to comment.