100% test coverage + minor bug fixes. (#12)

Author: ioquatix

fixtures/frame_examples.rb

@@ -9,15 +9,19 @@
 	let(:stream) {StringIO.new}
 	let(:frame) {subject.new}
 
-	it "can write frame" do
+	it "is a valid frame type" do
+		expect(frame).to be(:valid_type?)
+	end
+	
+	it "can write the frame" do
 		frame.write(stream)
 
 		expect(stream.string).not.to be(:empty?)
 	end
 
 	let(:framer) {Protocol::HTTP2::Framer.new(stream, {subject::TYPE => subject})}
 
-	it "can read frame using framer" do
+	it "can read the frame using framer" do
 		frame.write(stream)
 		stream.seek(0)
 

lib/protocol/http2/client.rb

@@ -53,10 +53,6 @@ def receive_push_promise(frame)
 					# This is almost certainly invalid:
 					promised_stream, request_headers = stream.receive_push_promise(frame)
 
-					if stream.closed?
-						@streams.delete(stream.id)
-					end
-					
 					return promised_stream, request_headers
 				end
 			end

lib/protocol/http2/connection.rb

@@ -41,7 +41,7 @@ def initialize(framer, local_stream_id)
 				@decoder = HPACK::Context.new
 				@encoder = HPACK::Context.new
 
-				@local_window = LocalWindow.new
+				@local_window = LocalWindow.new()
 				@remote_window = Window.new
 			end
 
@@ -287,9 +287,10 @@ def send_ping(data)
 
 			def receive_ping(frame)
 				if @state != :closed
-					if frame.stream_id != 0
-						raise ProtocolError, "Ping received for non-zero stream!"
-					end
+					# This is handled in `read_payload`:
+					# if frame.stream_id != 0
+					# 	raise ProtocolError, "Ping received for non-zero stream!"
+					# end
 
 					unless frame.acknowledgement?
 						reply = frame.acknowledge
@@ -313,7 +314,7 @@ def receive_data(frame)
 				end
 			end
 
-			def valid_remote_stream_id?
+			def valid_remote_stream_id?(stream_id)
 				false
 			end
 
@@ -338,6 +339,10 @@ def accept_push_promise_stream(stream_id, &block)
 			# On the client side, we create requests.
 			# @return [Stream] the created stream.
 			def create_stream(id = next_stream_id, &block)
+				if @streams.key?(id)
+					raise ProtocolError, "Cannot create stream with id #{id}, already exists!"
+				end
+				
 				if block_given?
 					return yield(self, id)
 				else
@@ -382,24 +387,6 @@ def send_priority(stream_id, priority)
 				write_frame(frame)
 			end
 
-			def idle_stream_id?(id)
-				if id.even?
-					# Server-initiated streams are even.
-					if @local_stream_id.even?
-						id >= @local_stream_id
-					else
-						id > @remote_stream_id
-					end
-				elsif id.odd?
-					# Client-initiated streams are odd.
-					if @local_stream_id.odd?
-						id >= @local_stream_id
-					else
-						id > @remote_stream_id
-					end
-				end
-			end
-			
 			# Sets the priority for an incoming stream.
 			def receive_priority(frame)
 				if dependency = @dependencies[frame.stream_id]
@@ -421,27 +408,34 @@ def server_stream_id?(id)
 				id.even?
 			end
 
-			def closed_stream_id?(id)
-				if id.zero?
-					# The connection "stream id" can never be closed:
-					false
-				elsif id.even?
+			def idle_stream_id?(id)
+				if id.even?
 					# Server-initiated streams are even.
 					if @local_stream_id.even?
-						id < @local_stream_id
+						id >= @local_stream_id
 					else
-						id <= @remote_stream_id
+						id > @remote_stream_id
 					end
 				elsif id.odd?
 					# Client-initiated streams are odd.
 					if @local_stream_id.odd?
-						id < @local_stream_id
+						id >= @local_stream_id
 					else
-						id <= @remote_stream_id
+						id > @remote_stream_id
 					end
 				end
 			end
 
+			# This is only valid if the stream doesn't exist in `@streams`.
+			def closed_stream_id?(id)
+				if id.zero?
+					# The connection "stream id" can never be closed:
+					false
+				else
+					!idle_stream_id?(id)
+				end
+			end
+			
 			def receive_reset_stream(frame)
 				if frame.connection?
 					raise ProtocolError, "Cannot reset connection!"

lib/protocol/http2/continuation_frame.rb

@@ -14,6 +14,10 @@ def initialize(*)
 				@continuation = nil
 			end
 
+			def continued?
+				!!@continuation
+			end
+			
 			def end_headers?
 				flag_set?(END_HEADERS)
 			end
@@ -97,7 +101,7 @@ def apply(connection)
 			end
 
 			def inspect
-				"\#<#{self.class} stream_id=#{@stream_id} flags=#{@flags} #{@length}b>"
+				"\#<#{self.class} stream_id=#{@stream_id} flags=#{@flags} length=#{@length || 0}b>"
 			end
 		end
 	end

lib/protocol/http2/data_frame.rb

@@ -43,7 +43,7 @@ def apply(connection)
 			end
 
 			def inspect
-				"\#<#{self.class} stream_id=#{@stream_id} flags=#{@flags} #{@length}b>"
+				"\#<#{self.class} stream_id=#{@stream_id} flags=#{@flags} #{@length || 0}b>"
 			end
 		end
 	end

lib/protocol/http2/dependency.rb

@@ -170,7 +170,7 @@ def receive_priority(frame)
 			end
 
 			def total_weight
-				self.orderd_children
+				self.ordered_children
 
 				return @total_weight
 			end
@@ -203,14 +203,14 @@ def consume_window(size)
 				end
 			end
 
-			def to_s
+			def inspect
 				"\#<#{self.class} id=#{@id} parent id=#{@parent&.id} weight=#{@weight} #{@children&.size || 0} children>"
 			end
 
-			def print_hierarchy(buffer, indent: 0)
-				buffer.puts "#{" " * indent}#{self}"
+			def print_hierarchy(output = $stderr, indent: 0)
+				output.puts "#{"\t" * indent}#{self.inspect}"
 				@children&.each_value do |child|
-					child.print_hierarchy(buffer, indent: indent+1)
+					child.print_hierarchy(output, indent: indent+1)
 				end
 			end
 		end

lib/protocol/http2/error.rb

@@ -80,12 +80,6 @@ def initialize(message)
 			end
 		end
 
-		class HeaderError < StreamClosed
-			def initialize(message)
-				super(message)
-			end
-		end
-		
 		class GoawayError < ProtocolError
 		end
 
@@ -96,6 +90,12 @@ def initialize(message)
 			end
 		end
 
+		class HeaderError < StreamClosed
+			def initialize(message)
+				super(message)
+			end
+		end
+		
 		# Raised on invalid flow control frame or command.
 		class FlowControlError < ProtocolError
 			def initialize(message)

lib/protocol/http2/frame.rb

@@ -31,12 +31,15 @@ class Frame
 			LENGTH_HISHIFT = 16
 			LENGTH_LOMASK  = 0xFFFF
 
+			# The base class does not have any specific type index:
+			TYPE = nil
+			
 			# @param length [Integer] the length of the payload, or nil if the header has not been read yet.
 			def initialize(stream_id = 0, flags = 0, type = self.class::TYPE, length = nil, payload = nil)
-				@length = length
-				@type = type
-				@flags = flags
 				@stream_id = stream_id
+				@flags = flags
+				@type = type
+				@length = length
 				@payload = payload
 			end
 
@@ -79,7 +82,7 @@ def pack(payload, maximum_size: nil)
 				@length = payload.bytesize
 
 				if maximum_size and @length > maximum_size
-					raise ProtocolError, "Frame length #{@length} bigger than maximum allowed: #{maximum_size}"
+					raise ProtocolError, "Frame length bigger than maximum allowed: #{@length} > #{maximum_size}"
 				end
 			end
 
@@ -112,7 +115,7 @@ def connection?
 			# @return [String]
 			def header
 				unless VALID_LENGTH.include? @length
-					raise ProtocolError, "Invalid frame size: #{@length.inspect}"
+					raise ProtocolError, "Invalid frame length: #{@length.inspect}"
 				end
 
 				unless VALID_STREAM_ID.include? @stream_id
@@ -178,8 +181,15 @@ def write_payload(stream)
 			end
 
 			def write(stream)
-				if @payload and @length != @payload.bytesize
-					raise ProtocolError, "Invalid payload size: #{@length} != #{@payload.bytesize}"
+				# Validate the payload size:
+				if @payload.nil?
+					if @length != 0
+						raise ProtocolError, "Invalid frame length: #{@length} != 0"
+					end
+				else
+					if @length != @payload.bytesize
+						raise ProtocolError, "Invalid payload size: #{@length} != #{@payload.bytesize}"
+					end
 				end
 
 				self.write_header(stream)
@@ -191,7 +201,7 @@ def apply(connection)
 			end
 
 			def inspect
-				"\#<#{self.class} stream_id=#{@stream_id} flags=#{@flags} #{self.unpack}>"
+				"\#<#{self.class} stream_id=#{@stream_id} flags=#{@flags} payload=#{self.unpack}>"
 			end
 		end
 	end

lib/protocol/http2/framer.rb

@@ -33,7 +33,7 @@ module HTTP2
 		].freeze
 
 		# Default connection "fast-fail" preamble string as defined by the spec.
-		CONNECTION_PREFACE_MAGIC = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n".freeze
+		CONNECTION_PREFACE = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n".freeze
 
 		class Framer
 			def initialize(stream, frames = FRAMES)
@@ -50,13 +50,13 @@ def closed?
 			end
 
 			def write_connection_preface
-				@stream.write(CONNECTION_PREFACE_MAGIC)
+				@stream.write(CONNECTION_PREFACE)
 			end
 
 			def read_connection_preface
-				string = @stream.read(CONNECTION_PREFACE_MAGIC.bytesize)
+				string = @stream.read(CONNECTION_PREFACE.bytesize)
 
-				unless string == CONNECTION_PREFACE_MAGIC
+				unless string == CONNECTION_PREFACE
 					raise HandshakeError, "Invalid connection preface: #{string.inspect}"
 				end
 

lib/protocol/http2/headers_frame.rb

@@ -68,7 +68,7 @@ def apply(connection)
 			end
 
 			def inspect
-				"\#<#{self.class} stream_id=#{@stream_id} flags=#{@flags} #{@length}b>"
+				"\#<#{self.class} stream_id=#{@stream_id} flags=#{@flags} #{@length || 0}b>"
 			end
 		end
 	end